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

syntax: add a LangVariant parameter to Quote #737

Merged
merged 1 commit into from
Oct 1, 2021
Merged

syntax: add a LangVariant parameter to Quote #737

merged 1 commit into from
Oct 1, 2021

Conversation

mvdan
Copy link
Owner

@mvdan mvdan commented Sep 29, 2021

(see commit message)

@mvdan
Copy link
Owner Author

mvdan commented Sep 29, 2021

cc @andreynering @pwaller if you're up for a review :)

@mvdan
Copy link
Owner Author

mvdan commented Sep 30, 2021

Thanks for the review :) Added more tests, moved the code to quote.go, and I've added a QuoteError type.

@mvdan
Copy link
Owner Author

mvdan commented Oct 1, 2021

One last pre-merge change: QuoteError.RuneOffset is now ByteOffset, because a rune offset isn't particularly helpful on its own. It's not even easy to slice the string to get to the concerning rune quickly, but you can do that with a byte offset.

I had missed that $'' expansions are non-POSIX,
and only implemented by Bash and mksh.
So, in POSIX mode, we can't quote non-printable characters.

Moreover, fuzzing uncovered that mksh implements \x differently,
meaning that we require extra logic to follow its rules.
Keep all the fuzz crashers that we found in the process.

Since we've started having more edge cases that we can't quote,
start returning an error in the API, with a QuoteError type.
All it gives right now is a character position and a reason.

Finally, document what versions of Bash and mksh we develop with.
This matters, because some systems ship with very old versions,
which can implement slightly different quoting or escaping rules.

While at it, start using quicktest for the tests.
@mvdan mvdan merged commit 8bd780f into master Oct 1, 2021
@mvdan mvdan deleted the quote-lang branch October 1, 2021 21:50
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