Syntax highlighting for include keyword seems incorrect following puppet_new_syntax merge.

[KeithWard]

After doing a git pull earlier today I noticed that the syntax highlighting of the 'include' keyword is no longer acting correctly. it seems to be treating it as if it were a bareword instead of a keyword as it did before.

Looking back the change occured in 80fe4cdbdb2456e13a17cc15441ae1bf2d2d6298

Images shown below - was this change intentional?

[KeithWard]

In above images - first image was before the change (with the include keyword highlighted in amber) Now it appears like in the second image with both the include and class in the same colours.

[lelutin]

Hi @KeithWard ! this is weird, I can't seem to reproduce this issue.

do you have an example manifest snippet that shows the issue for you?

[KeithWard]

Hi lelutin .

Sorry for the delay,

I tried removing all my other addons except for vim-puppet and blanking my vimrc to rule out changes my side, but no dice.

Example manifest below, for some reason as above it highlights include and ::b in the same color instead of separately.

This is with FC30 / Vim 8.1.2102

class a { include ::b }

[lelutin]

@KeithWard no worries for the delay, and thanks for the added feedback!

I was running vim 8.1.1401, on debian sid. I saw that there was an upgrade available so I did it, and now I have 8.1.2244 and I'm still unable to see the same behaviour as you are. there must be something more that's different.

interesetingly though I'm now getting a couple of failures on unit tests for the vim-puppet plugin.

@KeithWard could you run the unit tests (./test/run-tests.sh at the top of vim-puppet's code directory) and report the output from the command here?

[lelutin]

interesetingly though I'm now getting a couple of failures on unit tests for the vim-puppet plugin.

oops false alarm there. I was on a branch with a local change that was actually breaking some tests. on master, all tests are successful with vim 8.1.2244

so the theory is that if something is broken on your setup, we might get more information from test results.

[KeithWard]

Sadly the tests didn't provide any information: vim-puppet_tests.txt

[KeithWard]

I tried switching from pathogen to vim 8's native loading, in case that was causing the issue, however it still shows with include ::x being treated oddly

[KeithWard]

I've done some preliminary tests on my end, and part of the issue seems to be (at least at cursory glance) that previously include was identified as a puppet keyword, and now it seems to be a puppetFunction instead.

[KeithWard]

The removal of this line specifically seems to be part of the issue: re-adding it seems to fix the highlighting of the include keyword, itself but not the word after it: " match keywords and control words except when used as a parameter syn match puppetKeyword "\(\<import\>\|\<inherits\>\|\<include\>\|\<require\>\|\<contain\>\)\(\s*=>\)\@!"

[lelutin]

ah, maybe you put your finger on something important: is it possible that your color scheme applies the same coloration for Function than for Identifier (the highlight class is now being class names) ?

You can list all of the highlight classes currently used, with a sample of their coloration, with the :highlight command

[KeithWard]

@lelutin You're right, checking :highlight does show that the types involved both puppetName and puppetFunction look exactly the same whereas puppetKeyword is different, so i guess that explains why everything now looks the same color, I'm not importing any color schemes on my end, I'm just using whatever happens to be upstream. So I guess that just makes the issue being that include is now being identified as a function rather than keyword which seems a bit odd.

[KeithWard]

I do note that upstream has both contain and include labelled as a functions which is where this change originated, I always believed include and contain were reserved keywords, but it seems not, and they also have a return value of 'Any' - although I don;t recall having ever used a return value of an include for anything. I might query this in slack with the puppet devs when I get a moment as I'm curious as to the logistics behind it.

However the issue remains what can we do to prevent both the include and puppet name resolving to the same color combination by default?

I can work around this myself by adding the syn match abover to my own vimrc, but from my testing this issue appears to show up for any users of the default color scheme (at least from a few people I've had test).

Could we re-add the syn match above so that those words are treated as keywords? It would negate the issue for users of the default scheme, but on the other hand the definition of those words in vim-puppet would then differ slightly from upstream - what are your thoughts on this?

[lelutin]

ah good, so now we have a clear understanding of what's happening :)

I feel like it's a bit weird that vim's default colorscheme has the same coloring for Function and Identifier (they're the highlight classes to which the puppet stuff resolves in the end) and I would view that as the true source of the issue ... but actually getting that changed in vim might be a huge hassle :\

I feel a bit uneasy with changing the highlight class just because one theme makes it look bad. maybe vim-puppet could redefine the colors for Identifier iif the colorscheme used is the default one and a certain variable is not non-null (e.g. to keep some method of disabling this behaviour)?

@rodjek do you have any thoughts about this? I'm not sure my approach is the most appropriate one