search

atom / language-puppet

Puppet package for Atom
Other
36 stars 31 forks source link

Add basic support for heredocs #51

Open jwarlander opened 7 years ago

jwarlander commented 7 years ago

Trying to match the syntax as closely as possible, given the description:

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

Plain heredocs as well as double quoted heredocs (allowing interpolation) should be supported. However, this grammar doesn't yet consider which escape sequences are enabled or not in the given flags.

jwarlander commented 6 years ago

@50Wliu anything I can do to get this moving forward? if something seems to be incorrect or missing in the grammar, I'm happy to update it.

rsese commented 6 years ago

Just a heads up that @50Wliu is out for a bit /cc https://github.com/atom/language-less/pull/82#issuecomment-334293564.

jwarlander commented 6 years ago

👍

ajpaul25 commented 6 years ago

I'm using this right now and it looks good. I'd like to suggest one improvement. If escape switches are not enabled (no / after the end text in the heredoc tag) the syntax highlighting for escape sequences should be disabled. I'm not sure if this means two more patterns need to be created, or if this could somehow be integrated into the two existing patterns.

ajpaul25 commented 6 years ago

Probably should have mentioned @jwarlander in my last comment. This issue is a bit stale and he's most-likely not following it anymore.

jwarlander commented 6 years ago

Thanks for the heads-up! It's been a while since I worked with Puppet code now, and this change, so I'm not entirely "on top" of it anymore ;) It also seemed like it would be a bit more complex to add support for differentiating between escape sequences. I'll have a quick look out of curiosity, but I'd rather see this PR accepted as-is, and then me (or someone else) opening one to properly handle escape sequence switches.

ajpaul25 commented 6 years ago

I think that makes sense. I could probably take a crack at it later. It is definitely a major improvement just the way it is. @50Wliu - what do you think?