fix: consume the -- separator in every addon argument source - #1
Merged
kfkonrad merged 2 commits intoSep 3, 2026
Conversation
plaintextaccounting#2699 applies breakAtFirstSeparator to the command line only, so a "--" written in a config file section or a command alias is passed on to the addon. main stripped every "--", so it never was. [argvdump] alpha -- beta $ hledger --conf CONF argvdump gamma main argv: [alpha] [beta] [gamma] plaintextaccounting#2699 argv: [alpha] [--] [beta] [gamma] this commit argv: [alpha] [beta] [gamma] Splitting the concatenation once, rather than per source, does not work. The sources are joined as conf <> alias <> cli, so a "--" from a config section or alias falls before the command line args and moves them all into the "after" partition, where hledger's own options are deliberately not stripped. That leaks them to the addon: argv: [alpha] [beta] [--conf] [FILE] [gamma] So each source consumes its own first "--" instead. That also makes the documented "-- -n" escape hatch reachable from a config section and from an alias, not only from the command line. Cases 14-16 in addons.test cover a config file section, a command alias, and the escape hatch. addons.test 16/16, full functional suite 1746/1746. Applies on top of plaintextaccounting#2699, which introduced this. AI usage: Claude Opus 5, ~15k output tokens
Passing addon arguments as an argv list fixes arguments containing an
apostrophe, which did not reach the addon before this PR.
Case 13 ("Arguments needing shell quoting are preserved") uses 'a b' and
'$x', but both of those are delivered literally on main as well, so it
passes either way. Nothing in the suite currently fails without the argv
change; an apostrophe is the input that distinguishes them.
addons.test 17/17.
AI usage: Claude Opus 5, ~2k output tokens
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
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.
Applies on top of plaintextaccounting#2699, against that branch.
plaintextaccounting#2699 applies
breakAtFirstSeparatorto the command line only, so a--writtenin a config file section or a command alias was not properly processed
It applies to command aliases as well as
[COMMAND]sections:Per source
The sources are joined as
conf <> alias <> cli, so a--from a config section or alias falls beforethe command line args and moves them all into the "after" partition, where
hledger's own options are deliberately not stripped.
So each source consumes its own first
--:This also makes the documented
-- -nescape hatch reachable from a configsection and from an alias, not only from the command line. Before it, there was
no way to get
-nto an addon except by typing it on the command line.Tests
Three new cases in
addons.testcover a config file section, a command alias,and the escape hatch. A fourth covers an argument containing an apostrophe:
case 13 ("Arguments needing shell quoting are preserved") uses
a band$x,both of which are delivered literally on main too, so nothing in the suite
currently fails without plaintextaccounting#2699's argv change. An apostrophe is the input that
distinguishes them.
addons.test17/17, full functional suite 1746/1746, builds warning-freeunder
-Werror.Note these cases only run locally for now — all six CI workflows exclude
/addons(see plaintextaccounting#2710).@kfkonrad not sure if you have notifications on...
AI usage: Claude Opus 5, ~15k output tokens