Skip to content
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

Update substring parser to comply with Bash's rules #7

Merged
merged 1 commit into from May 26, 2020

Conversation

ticky
Copy link
Contributor

@ticky ticky commented May 15, 2020

The substring parser is a Bash extension to POSIX Parameter Expansion, and it turns out our parsing has been different from how Bash's works for a while now, causing the singular gotcha that it is impossible to substitute a numeric value in the default value expansion.

For example, this expansion: ${EMPTY:-127} works in Bash and Zsh, giving you a result of 127 if EMPTY is unset, however, the same expansion is treated as a negative-offset substring expansion by interpolate, meaning you end up with a blank value rather than the expected default.

This seems to have been written this way intentionally, but it seems to be an oversight.

I've updated the parser to not special-case a numeric character following the :- operator, as the Bash-approved way is to require a space character after the - to disambiguate the two similar, but distinct functions. I’ve also updated the documentation to point that out, and all the tests to comply with this rule.

The substring parser is a Bash extension to POSIX Parameter Expansion, and it turns out our parsing has been different from how Bash's works for a while now, causing the singular gotcha that it is impossible to substitute a numeric value in the default value expansion.

For example, this expansion: `${EMPTY:-127}` works in Bash and Zsh, giving you a result of `127` if `EMPTY` is unset, however, the same expansion is treated as a negative-offset substring expansion by interpolate, meaning you end up with a blank value rather than the expected default.

This seems to have been written this way intentionally, but it seems to be an oversight.

I've updated the parser to not special-case a numeric character following the :- operator, as the Bash-approved way is to require a space character after the - to disambiguate the two similar, but distinct functions.
@ticky ticky requested review from yob, lox and pda May 15, 2020 20:16
@lox
Copy link
Contributor

lox commented May 25, 2020

I think this is probably sensible. I intentionally followed the POSIX rules, but I think the bash approach is pragmatic.

Do we consider other Bash-isms like #6 too?

@ticky
Copy link
Contributor Author

ticky commented May 25, 2020

The offset-based substitutions are not in the linked POSIX standard at all, they are Bashisms!

@ticky
Copy link
Contributor Author

ticky commented May 25, 2020

It looks like the things implemented in #6 are actually POSIX substitutions which are listed in that document!

@ticky ticky merged commit 07f35b4 into master May 26, 2020
@pda pda deleted the bash-compliant-substring-parsing branch May 26, 2020 02:34
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.

None yet

2 participants