atom / language-yaml

YAML package for Atom
Other
24 stars 39 forks source link

Another large rewrite #62

Closed winstliu closed 7 years ago

winstliu commented 7 years ago

Requirements

Description of the Change

For some reason whenever I work on language-yaml I end up rewriting the whole thing each time. Oh well. This PR should probably have been split up into like 10 different ones, but many of the changes depend on the others.

Alternate Designs

None. This is an overall rewrite to the grammar to hopefully make it easier to maintain in the future. Code duplication was getting out of hand, and many of the more obscure rules weren't working properly.

Benefits

Better overall code tokenization and accuracy.

Possible Drawbacks

Possible regressions, as is possible with any rewrite. Spec coverage has been improving each rewrite though, so the chance of severe regressions happening shouldn't be that large anymore.

Applicable Issues

Fixes #53

winstliu commented 7 years ago

Continuing the conversation here so that it's more visible: no, it currently doesn't. The ! tag was already there, so I kept it, but didn't add any of the other ones. That said, they look similar, so I could just group them together: [?!](?=\\s)

MaximSokolov commented 7 years ago

I am concerned about using punctuation.definition.keyword.yaml scope name for !!, because syntax--keyword selector can override styles for syntax--punctuation. It should be changed either to punctuation.definition.smth-else.yaml or keyword.other.smth.yaml.

As for ! tag, I'll leave final decision to you, as I am not quite familiar with YAML syntax

winstliu commented 7 years ago

punctuation.definition.tag.omap?

MaximSokolov commented 7 years ago

punctuation.definition.tag.omap

:+1:

winstliu commented 7 years ago

I think I'm going to avoid adding more scopes in this PR. Those can be added separately.