Skip to content
This repository was archived by the owner on Apr 22, 2020. It is now read-only.

Add support for Puppet#435

Open
jorgemorgado wants to merge 1 commit intogooglearchive:masterfrom
jorgemorgado:master
Open

Add support for Puppet#435
jorgemorgado wants to merge 1 commit intogooglearchive:masterfrom
jorgemorgado:master

Conversation

@jorgemorgado
Copy link
Copy Markdown

No description provided.

@googlebot
Copy link
Copy Markdown

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@jorgemorgado
Copy link
Copy Markdown
Author

I signed it!

@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

Comment thread src/lang-puppet.js
],
[
// Matching keywords
[PR['PR_KEYWORD'], /^(?:case|class|default|define|else|elsif|false|if|inherits|node|unless|undef|true)\b/, null],
Copy link
Copy Markdown

@hlindberg hlindberg May 8, 2016

Choose a reason for hiding this comment

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

There are additional keywords - here are all the current keywords.

  KEYWORDS = {
    'case'     => [:CASE,     'case',     4],
    'class'    => [:CLASS,    'class',    5],
    'default'  => [:DEFAULT,  'default',  7],
    'define'   => [:DEFINE,   'define',   6],
    'if'       => [:IF,       'if',       2],
    'elsif'    => [:ELSIF,    'elsif',    5],
    'else'     => [:ELSE,     'else',     4],
    'inherits' => [:INHERITS, 'inherits', 8],
    'node'     => [:NODE,     'node',     4],
    'and'      => [:AND,      'and',      3],
    'or'       => [:OR,       'or',       2],
    'undef'    => [:UNDEF,    'undef',    5],
    'false'    => [:BOOLEAN,  false,      5],
    'true'     => [:BOOLEAN,  true,       4],
    'in'       => [:IN,       'in',       2],
    'unless'   => [:UNLESS,   'unless',   6],
    'function' => [:FUNCTION, 'function', 8],
    'type'     => [:TYPE,     'type',     4],
    'attr'     => [:ATTR,     'attr',     4],
    'private'  => [:PRIVATE,  'private',  7],
  }
  APP_MANAGEMENT_KEYWORDS = {
    :with_appm => {
      'application' => [:APPLICATION, 'application',  11],
      'consumes'    => [:CONSUMES,    'consumes',  8],
      'produces'    => [:PRODUCES,    'produces',  8],
      'site'        => [:SITE,        'site',  4]
    },

@hlindberg
Copy link
Copy Markdown

hlindberg commented May 8, 2016

I would much rather see a highlighter that is correct in what it highlights and leaves the rest plain than something that is more of a guess that will be wrong a lot of the time. Happy to collaborate and contribute but it needs to start from solid ground. Puppet is quite complex to lex correctly.

The real lexer is here: https://github.com/puppetlabs/puppet/blob/master/lib/puppet/pops/parser/lexer2.rb

@jorgemorgado
Copy link
Copy Markdown
Author

Thanks @hlindberg for your feedback. I don't have the time to implement all these :( at lease not alone. True that Puppet is quite complex to lex on all cases but based on my experience most people don't use all the syntax options that are available (amen to that!) Likely, some of those will even be discontinued, so does it really worth the trouble?

On a similar example, there are some puppet-linter plugins that report "broken code" on some edge cases (which are 100% syntactically correct). Still, that does not render the plugin useless simply because most people are not using those edge cases.

What do you think is a good compromise solution? Extending the list of keywords and add support for C-style multiline comments should be trivial. Others might take a bit longer and still not cover all cases.

@hlindberg
Copy link
Copy Markdown

@jorgemorgado People do use syntax highlighters to document and publish articles about the puppet language ;-). Some constructs are naturally more common than others, but you will see many of the new additions in puppet 4.x gradually being used more. I think it is far better if a highlighter is missing a highlight than if it highlights the wrong way. A good set to start with are the basic tokens (comments, strings (single and double quoted with interpolation), numbers, regexp, names (lower case bare words), types (upper case bare words), variables, and keywords). Then there is only the tricky heredoc left to deal with of the lexical constructs.

At present there are no lexical constructs in puppet that are slated for deprecation. There are some under discussion but removal of those, should such decisions be made, are years away.

Thus, in terms of changes to this PR, I think it is mostly a matter of removing the highlighting rules that will not work well.

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