Single quotes should be literal in subst part of "${var-subst}"#176
Merged
Conversation
If a parameter expansion with a PT_MINUS, PT_PLUS, PT_ASSIGN, or
PT_ERROR type is in double quotes, single quotes are not treated as
quoting characters in the substitution word. This is what is specified
in POSIX.1-2024 XCU 2.2.3:
> For parameter expansions other than the four varieties that provide
> for substring processing, within the string of characters from an
> enclosed "${" to the matching '}', the double-quotes within which the
> expansion occurs shall preserve the literal value of all characters,
> with the exception of the characters double-quote, backquote,
> <dollar-sign>, and <backslash>.
Commit a333044 attempted to fix this, but it only addressed the
expansion behavior. This commit fixes the parsing behavior.
The parser needs to be aware of whether the parameter expansion is in
double quotes or not, so the `parse_paramexp_in_brace` function now
takes an additional `indq` argument to indicate if the expansion is in
double quotes. This flag is passed down to the `parse_word` function,
which then decides whether to allow single quotes in the substitution
word.
Fixes #174
There was a problem hiding this comment.
Pull Request Overview
This PR fixes handling of single quotes in the word part of parameter expansions when inside double quotes, updates parser internals to support this behavior, and adds tests and release notes to cover the new behavior.
- Treat single quotes as literal characters in
${foo+word},${foo-word},${foo=word},${foo?word}when within double quotes - Refactored
parse_word,parse_paramexp_in_brace, and related logic to introduceallow_single_quotesand renametestfunctois_delimiter - Added new test cases in
tests/quote-p.tstand updatedNEWS/NEWS.jadocuments
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/quote-p.tst | Added tests for single-quote literal behavior in substitution |
| parser.c | Introduced allow_single_quotes flag, renamed testfunc param |
| NEWS | Documented the bug fix for single-quote handling in expansions |
| NEWS.ja | Japanese release note updated for the same fix |
Comments suppressed due to low confidence (3)
parser.c:1177
- The doc comment for
parse_wordis missing a description of the newallow_single_quotesparameter—add a line explaining its effect on parsing single-quote characters.
/* Parses a word at the current position.
NEWS:12
- The code span for
${foo=word}is missing a leading backtick—wrap it as${foo=word}for consistent formatting.
${foo=word}`, and `${foo?word}` were treated as quoting characters
NEWS.ja:10
- The third expansion
${foo=word}is missing its opening backtick—add it to match the others.
`${foo+word}`, `${foo-word}`, ${foo=word}`, and `${foo?word}`
| [a$'\t'b] | ||
| [a''c][a'b][a$'\t'b] | ||
| __OUT__ | ||
| #' |
There was a problem hiding this comment.
This looks like a stray #' marker in the test file—remove it so that the test harness will parse the closing marker correctly.
Suggested change
| #' |
| } | ||
| goto check_closing_brace; | ||
|
|
||
| parse_subst2: |
There was a problem hiding this comment.
[nitpick] The label parse_subst2 is not very descriptive—consider renaming it to something like parse_subst_in_double_quotes to clarify its purpose.
Suggested change
| parse_subst2: | |
| parse_subst_in_double_quotes: |
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.
This pull request addresses a bug related to the handling of single quotes in parameter expansions within double-quoted contexts. It also includes updates to the parser logic and corresponding test cases to ensure the issue is resolved and behaves as expected.
Bug Fixes:
wordof parameter expansions (${foo+word},${foo-word},${foo=word},${foo?word}) were incorrectly treated as quoting characters when the expansion was inside double quotes. (NEWS,NEWS.ja) [1] [2]Parser Updates:
parse_wordfunction to include a new parameterallow_single_quotesand renamed thetestfuncparameter tois_delimiterfor clarity. (parser.c) [1] [2]parse_paramexp_in_bracefunction to accept anindqparameter, ensuring correct behavior when parsing parameter expansions in double-quoted contexts. (parser.c) [1] [2]parse_namesection to handle single quotes correctly based on theallow_single_quotesparameter. (parser.c) [1] [2]Test Enhancements:
${a+b},${a-b}, and${a=b}. (tests/quote-p.tst) [1] [2] [3]