Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Update puppet grammar #25

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Conversation

ccaum
Copy link

@ccaum ccaum commented Feb 18, 2016

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.

This commit looks for the beginning and end of a resource and only
allowing certain patterns in between such as a title, parameters, and
valid parameter values.
This commit adds conditionals to the repository and includes them in
the global patterns. This will make it easier to maintain conditional
logic over time.
Easier to maintain over time
This commit makes functions a repository item and removes old ways of
identifying them.
This commit does not match the ending '{' character that signifies the
end of a class metadata declaration, and the begining of the class
content. Further, it treats the class name in the 'inherits' declaration
the same as a class name
This comit creates a 'defineparmaeters' repository item that is used for
matching parameters in defined types, classes, and application
definitions.
This commit properly parses resource parameters and adds new spec tests
for resource tokenization
This commit properly parses class definitions with or without parameter
and with or without class inheritance.  Tests were also added
This commit replaces the defined type pattern with the class pattern
since they're almost perfectly identical patterns
Tokenizing resource titles makes it impossible to have syntax
highlighting for strings and variables for titles
This commit tokenizes integers and floats anywhere in the Puppet code.
In the future, this should be reduced to only where numbers are allowed
- as parameter values or assignment operation values.
@ccaum
Copy link
Author

ccaum commented Feb 19, 2016

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

'endCaptures':
'0':
'name': 'punctuation.definition.class.begin.puppet'
'end': '{'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this end above the endCaptures?

This commit adds scoping for case statements.  Case statements are
really tricky since their parameter syntax structure is unique, but are
easy to parse as hash parameters.  This causes problems.

Since each parameter can contain any valid Puppet code, and we need to
parse each parameter explicitly, all valid Puppet code has been moved to
a repository item called '#puppet'. This will give the rest of the
parser quite a bit of flexibility as we can use this to fully match
conditionals blocks
@ccaum
Copy link
Author

ccaum commented Feb 23, 2016

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.

]
}
]
'puppet':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name seems ambiguous...what does this group do exactly?
EDIT: I see now - you might want to rename this to code to maintain consistency with other language packages.

@winstliu
Copy link
Contributor

winstliu commented Mar 1, 2016

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

@ccaum
Copy link
Author

ccaum commented Mar 7, 2016

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

@petems
Copy link
Contributor

petems commented Mar 11, 2016

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

Before: (Vanilla install):
screenshot 2016-03-11 15 22 36

After: (Using this branch)
screenshot 2016-03-11 15 19 17

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants