Commit Graph

7 Commits

Author SHA1 Message Date
Johannes Altmanninger
e717b13e75 Fix spurious syntax error on escaped $@ inside quoted command substitution
We detect use of unsupported features like $@ by scanning string tokens
as a whole. With quoted command substitution, this has false positives,
as reported in [1]. We already recursively run the same error checks on
command substitutions, so limit the remaining checks to the gaps in-between
command substitutions.

[1]: 5f94dfd094/.config/fish/README/bug.md (cannot-use-dollar-anchor-in-sed-regex-in-quoted-command-substitution)
2022-04-03 16:18:47 +02:00
Johannes Altmanninger
3e3f507012 Fix regression expanding \$()
When expanding command substitutions, we use a naïve way of detecting whether
the cmdsub has the optional leading dollar. We check if the last character was
a dollar, which breaks if it's an escaped dollar.  We wrongly expand
\$(echo "") to the empty string. Fix this by checking if the dollar was escaped.

The parse_util_* functions have a bunch of output parameters. We should
return a parameter bag instead (I think I tried once and failed).
2022-04-03 15:54:08 +02:00
Johannes Altmanninger
1b668f5675 Don't use results of quoted command substitution in adjacent variable expansion
Given

    set var a
    echo "$var$(echo b)"

the double-quoted string is expanded right-to-left, so we construct an
intermediate "$varb".  Since the variable "varb" is undefined, this wrongly
expands to the empty string (should be "ab"). Fix this by isolating the
expanded command substitution internally. We do the same when handling
unquoted command substitutions.

Fixes #8849
2022-04-03 11:24:55 +02:00
Johannes Altmanninger
4a575b26f5 Fix error check for repeated quoted command substitution
Commit e40eba358 (Treat text following quoted command substitution
as quoted) made parse_util_locate_cmdsubst_range() aware of quoted
command substitutions, by skipping surrounding text via quote_end().

However, it was not quite right. We fail to properly parse
two consecutive command substitutions in the same string,
because we don't maintain the quoting context across calls to
parse_util_locate_cmdsubst_range().  Let's track that bit in a
parameter. This allows us to get rid of the quote_end() hack.

Also apply this to the other place where we call
parse_util_locate_cmdsubst_range() in a loop (highlighting).

Fixes #8500
2021-12-04 16:56:07 +01:00
Johannes Altmanninger
e40eba3585 Treat text following quoted command substitution as quoted
Commit ec3d3a481 (Support "$(cmd)" command substitution without line
splitting, 2021-07-02) started treating an input string like
"a$()b" as if it were "a"$()"b". Yet, we do not actually insert the
virtual quotes. Instead we just adapted the definition of when quotes
are closed - hence the changes to quote_end().

parse_util_locate_cmdsubst_range() is aware
of the changes to quote_end() but some of its
callers like parse_util_detect_errors_in_argument() and
highlighter_t::color_as_argument() are not.  They split strings at
command substitution boundaries without handling the special quoting
rules. (Only the expansion logic did it right.)

Fix this by handling the special quoting rules inside
parse_util_locate_cmdsubst_range(). This is a bit hacky since it
makes it harder for callers to process some substrings in between
command substitutions, but that's okay because current callers only
care about what's inside the command substitutions.

Fixes #8394
2021-10-30 18:02:10 +02:00
Johannes Altmanninger
0ab6735450 Support $(cmd) command substitution as alternative to (cmd)
For consistency with "$(cmd)" and with other shells.
2021-07-13 21:33:42 +02:00
Johannes Altmanninger
ec3d3a481b Support "$(cmd)" command substitution without line splitting
This adds a hack to the parser. Given a command

	echo "x$()y z"

we virtually insert double quotes before and after the command
substitution, so the command internally looks like

	echo "x"$()"y z"

This hack allows to reuse the existing logic for handling (recursive)
command substitutions.

This makes the quoting syntax more complex; external highlighters
should consider adding this if possible.

The upside (more Bash compatibility) seems worth it.

Closes #159
2021-07-13 21:33:42 +02:00