Skip to content

Pass user code that precedes the Argbash macros through verbatim - #216

Open
gdevenyi wants to merge 1 commit into
matejak:masterfrom
gdevenyi:fix/preserve-leading-user-code
Open

gdevenyi wants to merge 1 commit into
matejak:masterfrom
gdevenyi:fix/preserve-leading-user-code

Conversation

@gdevenyi

Copy link
Copy Markdown
Contributor

Fixes #208

Square brackets are m4 quotes, so one level of them was stripped from whatever user code preceded the first Argbash macro: "${arr[@]}" became "${arr@}", [[ -n $x ]] became [ -n $x ], and every regeneration made it worse.

The argbash script now escapes square brackets on the lines that precede the first Argbash macro using m4sugar quadrigraphs (@<:@, @:>@), which autom4te turns back into square brackets at the end of the run, so that code comes out unchanged and does not have to have balanced brackets. Lines that contain m4 macros (and lines inside m4 quotes those lines open) are left alone, so constructs such as the m4_ignore of argbash-init templates and the m4_define traps of the test templates keep working.

  • The same treatment is applied where argbash scans the template for ARGBASH_WRAP and INCLUDE_PARSING_CODE arguments.
  • Documentation (template layout, limitations) updated accordingly.
  • New regression tests test-leading-code and stability-leading-code.

Tested with make check (the only failures are the pre-existing shellcheck SC2154 ones in the wrapping tests that #207 addresses).

🤖 Generated with Claude Code

https://claude.ai/code/session_017DxhC3HmTM7TQUzkBZnyH2

Copilot AI lite review requested due to automatic review settings September 11, 2026 21:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Address the directive-matching issues and add the missing stability-test regeneration rule.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates Argbash to preserve square brackets in user code before the first macro.

Changes:

  • Adds quadrigraph protection for leading code and macro scans.
  • Adds regression and stability tests.
  • Updates documentation and release metadata.
File summaries
File Summary Final review comments
tests/regressiontests/test-leading-code.m4 Leading-code regression fixture None
tests/regressiontests/Makefile Test targets None
tests/regressiontests/make/tests/tests-base.m4 Test definitions Critical (1 vote): Missing regeneration rule for test-leading-code2.sh.
src/argbash.m4 Core protection logic Moderate (3 votes): Restrict directive matching to actual prefixes.
doc/usage.rst Template behavior documentation None
doc/index.rst Limitation updates None
ChangeLog Release note None
bin/argbash Generated executable update Moderate (3 votes): Restrict directive matching to actual prefixes.
AUTHORS Contributor attribution None
Review details
  • Files reviewed: 8/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

grep -q '^_before=("\[one\]" "\[two\]")$$' $<
]])

ADD_SCRIPT([test-leading-code2])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The file is produced by the generic %2.sh: %.sh pattern rule in resources/Makefile, which is how the existing stability test gets basic2.sh as well, so a clean build works (the suite runs green with this test). No change needed.

Comment thread src/argbash.m4 Outdated
protect_leading_user_code()
{
awk '
!seen_directive && /^#[[:space:]]*(ARG|DEFINE_|INCLUDE_)/ { seen_directive = 1 }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed: the detector now matches only the real macro prefixes (ARG_, ARGBASH_, DEFINE_, INCLUDE_), and the test template gained an # ARGUMENTS comment line before its bracketed code.

Square brackets are m4 quotes, so one level of them was stripped from
whatever user code preceded the first Argbash macro: "${arr[@]}" became
"${arr@}", "[[ -n $x ]]" became "[ -n $x ]", and every regeneration
made it worse.

The argbash script now escapes square brackets on the lines that precede
the first Argbash macro using m4sugar quadrigraphs, which autom4te turns
back into square brackets at the end of the run, so that code comes out
unchanged. Lines that contain m4 macros (and lines inside m4 quotes those
lines open) are left alone, so constructs such as the m4_ignore of
argbash-init templates keep working.

Fixes matejak#208

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017DxhC3HmTM7TQUzkBZnyH2
@gdevenyi
gdevenyi force-pushed the fix/preserve-leading-user-code branch from b68175f to c8b3239 Compare October 12, 2026 18:59
@matejak

matejak commented Oct 12, 2026

Copy link
Copy Markdown
Owner

This is a valuable issue to address, but I am quite uneasy about the "magic" being done here that is accomplished in your awk script.

First of all, I would not consider m4sugar or other m4 macros in the script, as the property of argbash is the ability to regenerate at any time, and the idea of using m4 inside of the script goes against this idea. Moreover, what if you m4_define a new macro? That one wouldn't get protected. So I would simplify the approach here.

Next, what about simply quoting the start of the script? One can insert a leading quote manually into the definition of input, and the only trick would be insertion of the terminating quote right before the first macro invocation, or at the end of the script if there are no macros. I am not sure, but given that awk searches for the start of Argbash macros, it should be relatively easy to supply this ending quote and still simplify the awk code. Is it the case, or am I missing something? What do you think?

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.

Generator strips square brackets from code before Argbash directives

3 participants