-
-
Notifications
You must be signed in to change notification settings - Fork 533
shlexing happens _after_ substitution causing space paths to be multiple arguments #924
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
Comments
CC @eirnym (originally reported #121 (comment)) |
Confirmed it is still a problem, and is the same underlying problem as #763 . |
There is one difference to #763 that makes spaces 'harder', the word splitter in |
Paths were already stored inside SectionReader._subs as path objects, however they are turned into strings when they go through Replacer, and various transforms occur on these paths, for example breaking paths containing '#' and ' '. This change defaults to path objects being retained through the Replacer, and path segments being joined together using path object joining semantics. This mode can be disabled for settings of type `path` by wrapping the value in quotes, and disabled for other values by disabling new global setting literal_paths. Fixes tox-dev#763 and tox-dev#924
Paths were already stored inside SectionReader._subs as path objects, however they are turned into strings when they go through Replacer, and various transforms occur on these paths, for example breaking paths containing '#' and ' '. This change defaults to path objects being retained through the Replacer, and path segments being joined together using path object joining semantics. This mode can be disabled for settings of type `path` by wrapping the value in quotes, and disabled for other values by disabling new global setting literal_paths. Fixes tox-dev#763 and tox-dev#924
Requiring users to quote substitutions is essentially how shell scripting has handled this problem: https://www.shellcheck.net/wiki/SC2086 Not saying shell scripting is the epitome of style and safety, but keeping the substitution expansion separate from any sort quoting or splitting makes the configuration language more flexible and composable (albeit adding some footguns to watch for) |
In
/private/tmp/t t
:Expected:
If I quote
{toxinidir}
it works though:Intuitively, I think
{toxinidir}
should probably be considered a specific argument, though if people are already depending on this behaviour it would potentially break their configuration.Related, if we treat it as an indivisible argument, there would have to probably be some special handling for things like
{toxinidir}/.coveragerc
🤔The text was updated successfully, but these errors were encountered: