Clarify snippet placeholder structure by echasnovski · Pull Request #2032

Clarify snippet placeholder structure by echasnovski · Pull Request #2032 · microsoft/language-server-protocol

Conversation

Now grammar implies that placeholder value should be a single any node, while in most applications it can be any number of consecutive any nodes. For example, as described in earlier "Placeholders" section: ${1:another ${2:placeholder}}.

Using any+ instead of any seems to be more precise.

Would it make more sense to leave these unchanged, and instead change any to:

(tabstop | placeholder | choice | variable | text)+

currently the grammar technically implies that there can only be one top-level any node in a snippet

Would it make more sense to leave these unchanged, and instead change any to:
(tabstop | placeholder | choice | variable | text)+
currently the grammar technically implies that there can only be one top-level any node in a snippet

Yeah, maybe. This more feels like a style preference. I don't think grammar is a place for implications, so not sure if the first (or the broadest) definition should be treated as top level. But having an explicit mention about top level snippet parts might be useful indeed.

For sure. And just to be clear, either way the grammar right now is incorrect in that it only allows one any node at the top level (even something like text$1 is incorrect right now because it is a sequence of two any nodes). I didn't want to make a PR to change any to (tabstop | placeholder | choice | variable | text)+ because this one seems like it would be the perfect place to do that since it solves two issues with one. That said, we could keep this PR unchanged and then add something like

such that now source is the top level node rather than any, but that seems a lot less concise

Now grammar implies that placeholder value should be a single `any`
node, while in most applications it can be any number of consecutive
`any` nodes. For example, as described in earlier "Placeholders"
section: `${1:another ${2:placeholder}}`.

Adding `+` quantifier to `any` seems to be more precise.

This seems to also fix the grammar for initial snippet input (which is
implied to be `any`).

I finally remembered why I chose the route of not updating any directly: it is a better fit for resolving #2033. Its one solution (shown in the patch) requires any and placeholders be separate entities. But I agree that as an isolated change modifying any directly should be better, so I pushed it.

@dbaeumer, maybe you have own reservations about this (and #2033, for that matter)?

Ah makes sense, thx for linking

Conversation

2 participants