zsh-syntax-highlighting icon indicating copy to clipboard operation
zsh-syntax-highlighting copied to clipboard

SHORT_LOOPS syntax "while {....} {....}" not recognized

Open arkzo opened this issue 3 years ago • 14 comments

It is possible to write a loop using a different syntax like:

while (( i++ < 10 )) { echo i is $i } 

Issue:

The syntax highlighting fails on the { echo i is $i } part. Has this been a conscious decision made in the past? If not is it possible to fix this?

arkzo avatar Jun 23 '22 19:06 arkzo

No. Yes.

danielshahaf avatar Jun 26 '22 01:06 danielshahaf

Hi, can I take this up? What could be a possible fix?

another-strange-cat avatar Dec 05 '22 15:12 another-strange-cat

Sure. Welcome aboard.

The fix is going to involve state machine work in the main highlighter, but we're not there yet. We should first establish exactly which syntaxes are / aren't valid, both when SHORT_LOOPS is set and when it isn't.

Upstream's documentation doesn't fully answer the question: it just says "suitably delimited, such as […]" but doesn't give a complete specification of valid and invalid cases.

Thanks :)

danielshahaf avatar Dec 08 '22 22:12 danielshahaf

I have read the part of the documentation in the link, and if I'm understanding it correctly, invalid cases exist only for if, while and until when the condition part is not delimited.

The delimited part should be of the form '[[ ... ]]` or '(( ... ))'.

Do we have to establish valid syntaxes for these two forms? If yes, what should be taken into consideration for defining which are valid and which aren't? Also, what exactly is going wrong here, is the highlighter not able to distinguish between the condition part and the body?

another-strange-cat avatar Dec 10 '22 13:12 another-strange-cat

I have read the part of the documentation in the link, and if I'm understanding it correctly, invalid cases exist only for if, while and until when the condition part is not delimited.

The delimited part should be of the form '[[ ... ]]` or '(( ... ))'.

Do we have to establish valid syntaxes for these two forms?

I'm not asking you to establish valid syntaxes for shell tests and for arithmetic expressions. I'm asking what, apart from the [[ builtin and the arithmetic evaluation statement-like syntax (( … )), zsh's manual means when it says "suitably delimited".

Also, what exactly is going wrong here, is the highlighter not able to distinguish between the condition part and the body?

No. The highlighter gets a stream of tokens —

% () { print -raC1 -- "${(@qqqq)${(z)argv}}" } 'while (( i++ < 10 )) { echo i is $i }'  
$'while'
$'(( i++ < 10 ))'
$'{'
$'echo'
$'i'
$'is'
$'$i'
$'}'

— and has to determine whether that { introduces a block (as it does in f() { foo; }), is an ordinary word (as in echo bar { baz), or is a syntax error (as in the last brace of { } }, and as in repeat 42 pwd with SHORT_LOOPS and SHORT_REPEAT both unset).

danielshahaf avatar Dec 10 '22 13:12 danielshahaf

I guess what specifically happens is that the } is recognized as special but the { isn't, so the ${braces_stack} logic kicks in and flags the } as an error.

danielshahaf avatar Dec 10 '22 13:12 danielshahaf

Thanks for the detailed reply. The manual does not mention any other forms apart from [[ ... ]] and (( ... )), so I think we can safely assume that these are the only two.

another-strange-cat avatar Dec 12 '22 06:12 another-strange-cat

Thanks for the detailed reply. The manual does not mention any other forms apart from [[ ... ]] and (( ... )),

I'm well aware of that.

so I think we can safely assume that these are the only two.

Please check that assumption. If it's correct, we should report a documentation bug to zsh upstream. If it's not, we can move on to implementing this in z-sy-h.

danielshahaf avatar Dec 12 '22 21:12 danielshahaf

Could you please tell me how to check that assumption? I tried searching through the zsh source code repository to find the exact implementation of the syntax but couldn't find it.

another-strange-cat avatar Dec 14 '22 16:12 another-strange-cat

grepping for instances of SHORTLOOPS finds several instances in Src/parse.c. The easiest of them to read seems to be the one in par_while(), which parses its condition here:

https://github.com/zsh-users/zsh/blob/5c9713603d571fd228efc4c25c0efc9064d95a87/Src/parse.c#L1523-L1524

The meaning of SEPER wasn't very clear, so I committed https://github.com/zsh-users/zsh/commit/5c9713603d571fd228efc4c25c0efc9064d95a87.

This suggests "suitably delimited" means "is a zshmisc(1) 'list' that, when parsed, is followed by do or by { that's a token" (let's not worry about the CSH_JUNKIE_LOOPS case for now).

This checks out: while (( 1 )) && (( 2 )) { pwd } works, and with the braces removed, it works iff SHORT_LOOPS is set.

So:

  • Can we make upstream's docs clearer about this, now that we know what the rule is?
  • What can we reasonably support in z-sy-h?
  • What are some test cases that do and don't work? For starters, there's the two I just mentioned:
    • while (( 1 )) && (( 2 )) { pwd }
    • while (( 1 )) && (( 2 )) pwd

danielshahaf avatar Dec 15 '22 20:12 danielshahaf

Those two cases should be tested both with and without SHORT_LOOPS.

danielshahaf avatar Dec 15 '22 20:12 danielshahaf

We can borrow additional test cases from zsh's own test suite: https://github.com/zsh-users/zsh/blob/5c9713603d571fd228efc4c25c0efc9064d95a87/Test/E01options.ztst#L1117-L1119

danielshahaf avatar Dec 15 '22 21:12 danielshahaf

  • Yes, we can update the docs to make it clearer what is meant by "suitably delimited" and what alternate short forms are acceptable.
  • What should be considered for determining which syntax should be supported? To me, it seems reasonable to support the for, while and until forms mentioned in the docs.

I think that I'm missing something; the syntax without braces not working is confusing to me. Could you please link to some documentation about SHORT_LOOPS? I tried searching what SHORT_LOOPS option does but couldn't find related stuff.

So the first thing that needs to done is updating the documentation, right? Which file needs to be edited for this?

another-strange-cat avatar Jan 07 '23 08:01 another-strange-cat

Yes, we can update the docs to make it clearer what is meant by "suitably delimited" and what alternate short forms are acceptable.

:+1:

What should be considered for determining which syntax should be supported? To me, it seems reasonable to support the for, while and until forms mentioned in the docs.

The criteria for supporting a syntax include:

  • whether anyone is volunteering to implement support for it;
  • maintenance costs;
  • prioritization of outstanding pull requests by maintainers.

So, feel free to add support for whichever SHORT_LOOPS syntaxes you may choose to spend time on — but expect a PR adding support for the short form of select to be lower-priority to merge than one adding support for the short form of for, for example. (Personally, I use short if and repeat regularly too; others may have different use patterns.)

I think that I'm missing something; the syntax without braces not working is confusing to me. Could you please link to some documentation about SHORT_LOOPS? I tried searching what SHORT_LOOPS option does but couldn't find related stuff.

The manual always spells the option name like you just did, so you can just grep zshall(1) for "SHORT_LOOPS" and find everything relevant:

https://zsh.sourceforge.io/Doc/Release/Options.html#index-SHORT_005fLOOPS https://zsh.sourceforge.io/Doc/Release/Shell-Grammar.html#Alternate-Forms-For-Complex-Commands (linked above)

So the first thing that needs to done is updating the documentation, right? Which file needs to be edited for this?

https://github.com/zsh-users/zsh/blob/master/Doc/Zsh/grammar.yo

Patches for that go to [email protected]. See Etc/zsh-development-guide in that repository as well (the "Documentation" section). And thanks!

danielshahaf avatar Jan 07 '23 10:01 danielshahaf