subshells highlighted as incorrect syntax

[anntzer]

(cd ~; ls): the opening parenthesis is highlighted in red; the first command (cd) is not highlighted though the second one is.

[danielshahaf]

The fix would be to make the for arg in ${(z)…} loop check if arg is an unquoted parenthesis.

[anntzer]

PS: it would be nice if your fix also handled the (more typical) use case: tar cf - * | (cd /target; tar xfp -) (or similar).

[danielshahaf]

It probably will; the code has an internal notion of "at command position" ($new_expression) and subshells always occur at command position.

A quick and dirty implementation is simply to add '(' to the value of ZSH_HIGHLIGHT_TOKENS_PRECOMMANDS in main-highlighter.zsh.

Could I persuade you to write a test case for this?

[anntzer]

Something like

BUFFER='tar cf - * | (cd /target; tar xfp -)'

expected_region_highlight=(
  "1 3 $ZSH_HIGHLIGHT_STYLES[command]" # tar
  "15 16 $ZSH_HIGHLIGHT_STYLES[command]" # cd
  "27 29 $ZSH_HIGHLIGHT_STYLES[command]" # tar
)

?

[danielshahaf]

Yes, thank you.

[danielshahaf]

Committed the test in 22c8736.

[anntzer]

Kindly requesting a reopen, as I realized that the test I wrote was not enough: in fact, the opening parenthesis is still highlighted in red for me.

This should be a more exhaustive test:

BUFFER='tar cf - * | (cd /target; tar xfp -)'

expected_region_highlight=(
  "1 3 $ZSH_HIGHLIGHT_STYLES[command]" # tar
  "14 14 none" # (
  "15 16 $ZSH_HIGHLIGHT_STYLES[command]" # cd
  "27 29 $ZSH_HIGHLIGHT_STYLES[command]" # tar
  "36 36 none" # )
)

[danielshahaf]

Good catch. I can reproduce that here, too. I extended the test in 5a38710, but I made it expect the ( to be highlighted as a reserved word, for consistency with () and {.

[danielshahaf]

I think master...danielshahaf:i166bis-subshells fixes this, but I'll give it a closer look later.

[danielshahaf]

Merged. Closing, but please re-open if anything is still wrong.

[anntzer]

Sorry to keep reopening this issue but the closing parenthesis should have the same highlighting as the opening one :-)

[danielshahaf]

There's no need to apologise! I agree that would be good to have, but I think it fits best as a separate ticket. Filed it as #226 ☺