Skip to content

fix: escape % in class-variable comments so argparse help formatting does not crash (#183) - #185

Merged
martinjm97 merged 1 commit into
swansonk14:mainfrom
patchwright:fix/escape-percent-in-help-comment
Aug 29, 2026
Merged

martinjm97 merged 1 commit into
swansonk14:mainfrom
patchwright:fix/escape-percent-in-help-comment

Conversation

@patchwright

Copy link
Copy Markdown
Contributor

Problem

Fixes #183. A % in a class-variable comment crashes Tap (TypeError: %c requires int or char on Python <3.14; ValueError from _check_help on 3.14+). Confirmed on main HEAD fd2e8416.

Root cause

Tap._add_argument appends the extracted comment to the argparse help string verbatim (kwargs["help"] += " " + comment). argparse %-formats every help string (help_string % params), so a literal % in the comment is read as a format spec and crashes.

Fix

Escape % to %% (the standard argparse escape for a literal percent) when appending the comment. One source line in src/tap/tap.py; no API changes.

How to test

# saved as repro.py
from tap import Tap

class MyTap(Tap):
    percent: int = 100  # The % chance this will break

MyTap().format_help()
$ python repro.py
Traceback (most recent call last):
  ...
  File ".../argparse.py", line 640, in _expand_help
    return self._get_help_string(action) % params
TypeError: %c requires int or char

With this PR it returns the help text with the comment intact. Added test_comment_with_percent in tests/test_integration.py, verified to fail on unpatched main and pass with the fix.

Backward compatibility

No breaking changes. Comments render identically; %% is collapsed back to % by argparse's own formatting.


Assisted-by: Claude (code generation, reviewed and tested locally)

Comment thread tests/test_integration.py

@martinjm97 martinjm97 Jul 11, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test would benefit from a full string match to ensure the parse is actually "The % chance this will break" and not mangled.

@martinjm97
martinjm97 merged commit 88f4c2d into swansonk14:main Aug 29, 2026
15 checks passed
@martinjm97

Copy link
Copy Markdown
Collaborator

Thank you for your contribution to Tap!

--JK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Including "%" in a comment breaks Tap

2 participants