Fix empty string piece leaking into first_list for comma-only input#217
Merged
Conversation
HumanName(',') (no-comma path after collapse_whitespace strips the
trailing comma) and HumanName('Doe,, Jr.') (lastname-comma path) both
appended '' to first_list. Filter tokens that strip to nothing in
parse_pieces, the common source feeding all three parse paths, so no
empty piece reaches the parse loops or the public *_list attributes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Add the missing `if part` guard to the suffix-comma parts[1:] loop:
"John Doe, Jr.,," leaked '' into suffix_list via
expand_suffix_delimiter('') returning [''], the same bug class this
branch fixes elsewhere (regression test added)
- Pin the setter-path filtering as intended behavior: whitespace-only
and empty tokens assigned via attributes, lists, or __setitem__ no
longer become *_list members
- Pin the lastname_pieces call site: ", John" leaves last_list empty
- Assert sibling lists for HumanName(',') so a relocated (not removed)
empty piece can't pass
- State the no-empty-pieces guarantee in parse_pieces()'s docstring and
broaden the release log entry to cover last_list/suffix_list and the
setter behavior
- Align the suffix-comma "final piece" comment with the no-comma site's
wording (the PR 216 replace_all missed this deeper-indented copy)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Degenerate comma inputs left silent empty-string members in the public
*_listattributes. Two fixes:1.
parse_pieces()drops tokens that strip to nothing (the common tokenizer feeding all parse paths and the setters):HumanName(',')—collapse_whitespacestrips the trailing comma, so this flows through the no-comma path with a single''piece;first_listwas['']HumanName('Doe,, Jr.')— the empty middle segment flows through the lastname-comma path;first_listwas['']HumanName(', John')— emptyparts[0]put['']inlast_listhn.first = ' 'now yields an empty first (was the truthy string' '), andhn.middle = ['a', '', 'b']/hn['last'] = ['', 'Smith']drop the empty member. As a side benefit this repairs conjunction joining for double-spaced setter input ("Mr. and Mrs. Smith"previously produced malformed pieces).2. The suffix-comma path's
parts[1:]loop gets the sameif part:guard itsparts[2:]sibling got in #216 — found in review:HumanName('John Doe, Jr.,,')leaked''intosuffix_listbecause that loop bypassesparse_piecesviaexpand_suffix_delimiter('')→[''].The empty members were harmless in
str()output (the joined accessors are falsy), but the*_listattributes are public API.Notes for review
empty_attribute_defaultconfig params (test_degenerate_comma_input_leaves_no_empty_pieces,test_assignment_filters_empty_tokensintests/test_python_api.py;test_suffix_comma_empty_segment_not_added_to_suffix_listintests/test_suffixes.py).parse_pieces()'s docstring now states the no-empty-pieces guarantee; the release log entry covers all three affected lists and the setter behavior.replace_allin Fix bugs hidden in untested parser paths; remove vestigial unparsable #216 missed this deeper-indented copy).Follow-up to #216, which surfaced the original bug during its silent-failure review.
🤖 Generated with Claude Code