Skip to content

Fix single-quoted strings with embedded newlines, and make heredoc body parts lazy-loadable#3

Open
jdiamond wants to merge 2 commits intowebpro-nl:mainfrom
jdiamond:single-quote-backticks-and-heredocs
Open

Fix single-quoted strings with embedded newlines, and make heredoc body parts lazy-loadable#3
jdiamond wants to merge 2 commits intowebpro-nl:mainfrom
jdiamond:single-quote-backticks-and-heredocs

Conversation

@jdiamond
Copy link

This fixes two related bugs discovered while writing tests for single-quoted strings containing backticks and $().

Bug 1: single-quoted strings with embedded newlines were parsed incorrectly

computeWordParts had a heuristic — if word.text contained a newline, route to buildHereDocParts (heredoc scanning). The intent was to handle heredoc bodies, but any single-quoted string with a literal newline also triggered this path. The result: backticks and $() inside a multi-line single-quoted string were expanded as real shell substitutions instead of treated as literals.

Fixed by splitting the two responsibilities: computeWordParts always uses buildWordParts (correct shell-word semantics), and a new computeHereDocBodyParts handles heredoc scanning explicitly.

Bug 2: redirect.body.parts always returned undefined

Heredoc body words were constructed without a source string, so word.parts returned undefined immediately — consumers had no way to access the structural parts via the normal API.

Fixed by wiring heredoc body words into the lazy parts system. WordImpl now accepts an optional per-instance resolver at construction time (alongside the existing static default), and heredoc body words are constructed with computeHereDocBodyParts as their resolver. redirect.body.parts now works as expected.

New tests cover both: multi-line single-quoted strings with backticks and $(), and the existing heredoc expansion tests now validate through word.parts rather than calling the internal function directly.

@jdiamond
Copy link
Author

Heads up: PR #2 also modifies parts.ts. If both are merged, the merge commit will need a test run — I hit a silent integration issue where git combined the two parts.ts changes without conflict but moved a resolution loop into the wrong function. The tests caught it. Working fix is on https://github.com/jdiamond/unbash/tree/combined-fixes. Happy to submit a follow-up PR for the integration fix if that would be helpful.

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.

1 participant