-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
String substitution in variable expansion #4427
Conversation
Signed-off-by: Tristan Stenner <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Docs LGTM :-)
Surprisingly, all CI checks passed even though I added more tests. There's one decision left: referencing an undefined variable is not an error (as it is in bash without |
PTAL @jsternberg |
Sorry for taking so long to get to this. I've had it on my backburner for awhile. I'll have a review for this by the end of my current day. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
One of the variable expansion functions missing from #4287 was string substition, so I implemented it.
Motivating example:
The two parts following the variable name (pattern and replacement) need to be parsed separately with
processStopWords
, otherwise variable expansion could cause a/
in a variable referenced in the pattern to be interpreted as separator between patern and replacement. This means that the second/
is required (i.e.${foo/bar/}
to removebar
, compared to${foo/bar}
as accepted by various shells).On the positive side, wildcards and slashes in referenced variables are handled correctly (e.g.
PAT='*/'; REPL='.../'; ${FOO/$PAT/$REPL}
).