rodjek / vim-puppet

Puppet niceties for your Vim setup
Apache License 2.0
500 stars 137 forks source link

Apostrophe in heredoc breaks highlighting #103

Closed genebean closed 4 years ago

genebean commented 5 years ago

When a heredoc includes an apostrophe it breaks the highlighting as shown here:

heredoc-with-apostrophe-vim

When you remove the apostrophe it fixes the highlighting as shown here:

heredoc-without-apostrophe-vim

This code block can be used to reproduce the issue:

class foo {
  profile::notes { $title:
    notes => @("NOTES"),
      A note with an apostrophe in it's midst breaks
      highlighting.
      |-NOTES
  }

  include blah

  file { 'some-file':
    ensure => 'present'
  }
}

This same issue exists in VS Code and can be seen at https://github.com/lingua-pupuli/puppet-editor-syntax/issues/22

genebean commented 5 years ago

FYI - the issue above is resolved via https://github.com/lingua-pupuli/puppet-editor-syntax/pull/23

salderma commented 5 years ago

Would really be great if this could be fixed. I would be happy to make an attempt (though I've never done such a thing) if someone will shove me in the right direction of where in this project that VSCode fix can be adapted.

Maybe here: https://github.com/rodjek/vim-puppet/blob/4793b074ddbfc05ed0189e19de343870611e4bdc/syntax/puppet.vim#L76

lelutin commented 5 years ago

hello! I've reproduced this, but fwiw if you remove the comma after the @("NOTES") token the whole heredoc is colorized correctly.

off the top of my head I don't remember if that comma right next to the token is valid according to the syntax definition in puppet. if it is, then we should fix the pattern to permit it.

@salderma I think you've pointed to the right line indeed. if something needs to be changed, then it's probably in the start=+...+ regex.

lelutin commented 5 years ago

This seems to specify that anything after the heredoc token is continuation of puppet code on the same line. so the comma in the original example is quite valid.

https://puppet.com/docs/puppet/4.10/lang_data_string.html#heredocs

We need to remove the "$" at the end of the start regex. but I'm wondering if this might mean that it'll match in more places than it should.

also, it means that vim will interpret then end of the line as a part of the string, which it is not -- the multiline string only starts on the next line. I'm not sure how this should be handled...

salderma commented 5 years ago

Ok I'm not sure I follow the use of the , in the example above, but I can say colorization screws is messed up for me in a way that I thought was similar to the OP's experience.

Basically, I tend to make Puppet Heredocs for shell commands that I deem are too involved to reference in the command => 'shell command', of an exec. No I'm not using the |- to end my heredoc, but according to the Puppet Doc those are optional. For example:

bad-highlighting

P.S. I know that's ugly, and the code seems like a very non-puppet-like way of doing things but I have my reasons, don't judge me :blush:

lelutin commented 5 years ago

@salderma ah! your issue seems to be affected by a tiny bit different detail (e.g. the "end" regex instead of the "start" one).

for your case, maybe the fix would be to add parentheses around the whole beginning of the regex (after the + which is a delimiter for a regex), and close them before the \1 ?

lelutin commented 5 years ago

because changes to a regex are awkward to read in words, I mean this:

end=+(|-\? *)\z1 *$+
lelutin commented 5 years ago

with a bit more work and thought to it, @salderma I think the following might fix your case:

diff --git a/syntax/puppet.vim b/syntax/puppet.vim
index 00cb5fe..c4fb5be 100644
--- a/syntax/puppet.vim
+++ b/syntax/puppet.vim
@@ -73,9 +73,9 @@ syn match   puppetVariable      "${[a-zA-Z0-9_:'\[\]]\+}" contains=@NoSpell
 " match anything between simple/double quotes.
 " don't match variables if preceded by a backslash.
 syn region  puppetString        start=+'+ skip=+\\\\\|\\'+ end=+'+
-syn region  puppetString        start=+@(\z\([^/)]*\)\(/[\\nts$uL]*\)\?)$+ end=+|-\? *\z1 *$+
+syn region  puppetString        start=+@(\z\([^/)]*\)\(/[\\nts$uL]*\)\?)$+ end=+\(|-\? *\)\?\z1 *$+
 syn region  puppetString        start=+"+ skip=+\\\\\|\\"+ end=+"+ contains=puppetVariable,puppetNotVariable
-syn region  puppetString        start=+@("\z\([^/)]*\)"\(/[\\nts$uL]*\)\?)$+ end=+|-\? *\z1 *$+ contains=puppetVariable,puppetNotVariable
+syn region  puppetString        start=+@("\z\([^/)]*\)"\(/[\\nts$uL]*\)\?)$+ end=+\(|-\? *\)\?\z1 *$+ contains=puppetVariable,puppetNotVariable
 syn match   puppetNotVariable   "\\$\w\+" contained
 syn match   puppetNotVariable   "\\${\w\+}" contained

are you able to apply this change and verify that it's doing the right thing for you?

lelutin commented 4 years ago

@genebean and @salderma : I've retested both your cases on the current master, and I believe that the recent merger of the voxpupuli syntax highlighting fixed the regexes for heredocs for both of your cases.

In order to avoid future regressions, I've added test cases for both of your syntax highlighting problems.

please re-open if I'm mistakent about this.