Update puppet grammar - Githubissues

atom / language-puppet

Puppet package for Atom

Other

36 stars 31 forks source link

Update puppet grammar #25

Open ccaum opened 8 years ago

ccaum commented 8 years ago

This pull request updates the puppet grammar to improve matching of classes, comparisons, functions, numbers, and resource parameters. In addition, it improves the parameter matching for class, defined type, and application definitions. This still needs lots more test coverage and it doesn't yet cover the additional syntaxes added in Puppet 4.

This turned out to be a large PR, so I'm happy to work through it as works best for the maintainers.

ccaum commented 8 years ago

I noticed a few bugs and fixed those as well as added a few more tests.

winstliu commented 8 years ago

A few general comments:

The structure for matches should be begin -> beginCaptures -> end -> endCaptures -> name -> patterns (or match -> captures -> name -> patterns)
It would be awesome if you could also assign some scopes to punctuation (eg ;, {, [)
Can you change the names from stuff like defineparameters to either define-parameters or define_parameters to fit in better with the existing names?

ccaum commented 8 years ago

Thanks for reviewing @50Wliu. I think I resolved all of your concerns. Please double check and let me know what else needs to be fixed.

winstliu commented 8 years ago

I'm a bit busy right now, but I'll try to get back to this soon.

ccaum commented 8 years ago

I found a bug in how parameters in functions were being parsed so I fixed that and added a test that should catch problems in the future.

mschuchard commented 8 years ago

I just noticed this package was missing syntax highlighting on 'unless' and also noticed this PR fixes that issue. However, when I was taking a look at the changes to ensure that issue was fixed, I noticed line 423 is referencing a nonexistent 'cases.' Does that seem right?

ccaum commented 8 years ago

Ah, I did indeed forget case statements. So, those were tricky. The case values look similar enough to hashes that it didn't parse correctly. I had to explicitly capture case values, which can contain any valid Puppet code. To do that, I had to make a repository item called '#puppet' that contains all valid Puppet code. This is a good idea because we can fully match definitions and conditional blocks in the future.

winstliu commented 8 years ago

Nice work @ccaum! I've left some comments for you to look at :smile:.

ccaum commented 8 years ago

I haven't forgotten about this. I've just been pulled onto other things for a bit.

petems commented 8 years ago

I've found that this breaking highlighting a lot for me:

Before: (Vanilla install):

After: (Using this branch)