Repository navigation
Follow gawk's backslash rules in sub()/gsub() replacement text - #616
Conversation
In the replacement text of sub() and gsub(), Jawk applied the POSIX backslash rules (\\ is a literal backslash) in every mode. gawk's default mode follows its own documented rules instead: \\\& is a literal \&, \\\\ a literal \\, \\& a literal backslash followed by the match, and any other backslash is kept as is, so \\ stays \\. sub() and gsub() now follow gawk's rules by default and the POSIX ones under --posix, which is what gawk --posix, mawk and One True Awk do. The POSIX switch reaches the runtime through JRT.setPosix(), set by AVM from the settings. gensub() keeps its own gawk rules. JRT.prepareReplacement() is split into prepareSubReplacement(), which takes the POSIX switch, and prepareGensubReplacement(), which takes the highest capture group number, instead of one method whose boolean and int overloads meant different things. The gawk backgsub compatibility test now passes. Closes #614 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b5425b8d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The runtime took the sub()/gsub() backslash rules from the settings of the AVM that runs the program. A program compiled with --posix -K and loaded with -L, which cannot be combined with --posix, would therefore silently switch to gawk's rules. AwkProgram now records whether it was compiled in POSIX mode, and AVM takes the rules from the program when it executes it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex please review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4229382713
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Expressions compiled with POSIX settings and evaluated through Awk.eval() never reached the execute() paths that set the runtime mode, so sub()/gsub() in them used gawk's rules. The mode now lives on AwkTuples, the base of both AwkProgram and AwkExpression, is recorded by both compile paths, and is applied by AVM.eval() as well. The loaded-program test now runs through AwkTestSupport. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Closes #614
Problem
In the replacement text of
sub()andgsub(), Jawk applied the POSIX backslash rules (\\is a single literal backslash) in every mode. gawk's default mode follows its own documented rules ("Gory details" in the gawk manual):\\\&is a literal\&,\\\\a literal\\,\\&a literal backslash followed by the match, and any other backslash is kept as is, so\\stays\\. The gawkbackgsubcompatibility test encodes that rule and failed inmvn verify.Verified against gawk 5,
gawk --posixand mawk in WSL on 15 replacement strings;gawk --posixand mawk agree with each other and with Jawk's previous behavior.Change
sub()andgsub()follow gawk's rules by default and the POSIX ones under--posix, in line with the project policy of emulating gawk's documented behavior.JRT.setPosix(), set byAVMfrom the settings, next to the existing sink and stream setters.JRT.prepareReplacement()is split intoprepareSubReplacement(String, boolean posix)andprepareGensubReplacement(String, int maxGroup): the old boolean and int overloads meant different things.gensub()keeps gawk's own rules (\Ngroup,\\backslash,\&ersand, any other\ca plainc).backgsubnow passes; the failsafe baseline is otherwise unchanged.Tests
PrepareReplacementTest: full gawk table, POSIX table, and gensub table.AwkTest: one replacement string exercising all five gawk cases, run throughAwkTestSupportin default mode and with--posix.mvn verify: 889 unit tests pass; checkstyle, PMD and SpotBugs clean.Docs
compatibility.md.vm: new bullet under "Where Jawk follows gawk".cli-reference.md:--posixnow mentions the replacement-text rules.behavior-changes.md: two bullets underUnreleased(behavior, and theJRTAPI rename for embedders).🤖 Generated with Claude Code