Print numeric-looking strings verbatim - #541
Conversation
Per POSIX, print must output string values unchanged; only actual numbers are formatted with OFMT (or as integers). AwkSink used to coerce any operand whose text parsed as a number into a double, so print "0100" output 100 and input fields like $1 lost their original text. Remove the coercion, fix the OFMT-vs-CONVFMT expectation that relied on it, and add POSIX conformance tests. Fixes #529. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: c8cca65f57
ℹ️ 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".
| */ | ||
| protected final String formatPrintArgument(Object value, String ofmt) { | ||
| return formatOutputValue(normalizePrintArgument(value), ofmt, locale); | ||
| return formatOutputValue(value, ofmt, locale); |
There was a problem hiding this comment.
This changes user-visible AWK output for numeric-looking strings and for values converted with CONVFMT, but the commit updates neither README.md nor the site documentation. Add the new behavior to both documentation surfaces as required for end-user changes.
AGENTS.md reference: AGENTS.md:L34-L34
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Documented in 006b58e: added an "Output conversion in print" section to the compatibility page's Detailed Behavior Notes (covering verbatim string output, OFMT applying to numbers only, and CONVFMT vs OFMT). README.md intentionally unchanged: it stays high-level and already links to the compatibility page for detailed behavior notes, and this fix makes the behavior match the POSIX AWK support the README already claims.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. 🚀 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". |
Replaces the print-specific compatibility note with a dedicated site page that documents the user-visible behavior changes of each Jawk release, from newest to oldest, distilled from the GitHub release notes. Co-Authored-By: Claude Fable 5 <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: 68cfd3ffd3
ℹ️ 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".
| protected final String formatPrintArgument(Object value, String ofmt) { | ||
| return formatOutputValue(normalizePrintArgument(value), ofmt, locale); | ||
| return formatOutputValue(value, ofmt, locale); |
There was a problem hiding this comment.
When a custom AwkSink compiled against Jawk 7.0 calls the protected normalizePrintArgument helper—whose Javadoc explicitly invited structured sinks to reuse it—removing the method makes an upgrade fail with NoSuchMethodError at runtime, while recompiling the sink also fails. Preserve the signature for binary/source compatibility, potentially as a deprecated identity implementation, while bypassing it in the built-in text sinks.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Restored in 910e088 as a @deprecated compatibility shim with its original conversion behavior, so sinks compiled against 7.0 keep working unchanged; the built-in text sinks bypass it.
Restores the protected helper removed earlier in this branch so custom AwkSink subclasses compiled against 7.0 keep working; the built-in text sinks no longer call it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. Breezy! 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". |
The release workflow now renames the "Unreleased" section of behavior-changes.md to the released version (and inserts a fresh stub) before maven-release-plugin tags the release, so the page can never be forgotten. AGENTS.md requires behavior-changing PRs to record their change under "Unreleased", which Codex enforces in review. Co-Authored-By: Claude Fable 5 <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: 9598204d4b
ℹ️ 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".
| print "Unreleased" | ||
| print "----------" | ||
| print "" | ||
| print "_No user-visible behavior changes recorded yet._" |
There was a problem hiding this comment.
After the first release, this generated sentence remains inside the new Unreleased section; when a later behavior-changing PR follows the documented workflow and adds a bullet without manually deleting it, the next run counts the bullet and moves both the placeholder and the bullet into the versioned section. The published history will therefore claim that the release has no user-visible changes immediately before listing one. Remove this placeholder when bullets > 0, or use a marker that is not carried into the released section.
AGENTS.md reference: AGENTS.md:L36-L36
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3759c87: the stamping pass now drops the placeholder line (and its trailing blank) from the section being released, so only real bullets are carried into the versioned section. Verified with two consecutive stamp runs.
The fresh "Unreleased" stub's placeholder line was carried into the next stamped release section alongside real bullets; the stamping pass now removes it from the released section. Co-Authored-By: Claude Fable 5 <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: 3759c87715
ℹ️ 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".
| @@ -0,0 +1,94 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
This commit adds .github/scripts/stamp-behavior-changes.sh and src/site/markdown/behavior-changes.md without the required project license header. Add the proper header to both new files before merging.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not applicable per project convention: license headers are managed by license-maven-plugin, whose configuration covers only main/test/it/jmh Java sources, and the build's check-file-header gate (failOnMissingHeader=true) passes. No file in src/site/markdown or .github carries a header (a header would also render as visible text on the site page). AGENTS.md's instruction is operationalized by 'mvn license:update-file-header', which by configuration leaves these files untouched.
|
@codex please review again |
|
Codex Review: Didn't find any major issues. Bravo. 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". |
Converts the page from setext underlines to #/## heading style and adapts the release stamping script accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex please review again |
|
Codex Review: Didn't find any major issues. Delightful! 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
Fixes #529.
Problem
printnumerically coerced any operand whose text parsed as a number, losing the original text:This affected not only string constants but also input-sourced values:
echo 0100 | jawk '{ print $1 }'printed100, where gawk and POSIX AWK print0100.Root cause
AwkSink.normalizePrintArgument()converted every non-Numberprint operand whosetoString()parsed as aBigDecimalinto aDoublebefore formatting, then applied OFMT. Per POSIX,printoutputs string values (including strnums) unchanged; OFMT applies only when converting an actual number to a string.Fix
normalizePrintArgument();formatPrintArgument()now formats realNumbers with the integer/OFMT rules and prints everything else verbatim viatoString().PosixIT.spec33OfmtVsConvfmt, whose1.24expectation encoded the buggy double conversion (s = x ""→"1.235"via CONVFMT, then re-coerced and reformatted with OFMT). POSIX printssverbatim:1.235.Verification
mvn verifypasses: 639 unit tests, checkstyle/pmd/spotbugs clean.test_fsfwfs,test_sortfor2,test_nasty2,test_numstr1,test_numsubstr); no regressions vsmain(PosixIT.spec82NumericComparisonWithNumericStringsalready failed onmain).🤖 Generated with Claude Code