atom / language-puppet

Puppet package for Atom
Other
36 stars 31 forks source link

Support variable subscripts (eg, $v[subscript]) #44

Closed alexjurkiewicz closed 5 years ago

alexjurkiewicz commented 8 years ago

Want to open this pull request by stating I don't really understand what I did here πŸ’

I'm pretty sure the syntax highlighting works, but I'm not sure how the grammar changes interact with variable completion. If it does there may be some issues -- please see the captured groups in the spec for more info.

Test text:


$var[test]
$::var[test]
$var::iable[test]
$::var::iable[test]

${var[test]}
${::var[test]}
${var::iable[test]}
${::var::iable[test]}

$var[te][st]
$::var[te][st]
$var::iable[te][st]
$::var::iable[te][st]

${var[te][st]}
${::var[te][st]}
${var::iable[te][st]}
${::var::iable[te][st]}

Old and busted:

screen shot 2016-08-29 at 1 08 41 pm

New hotness:

screen shot 2016-08-29 at 1 08 25 pm

This change is Reviewable

winstliu commented 8 years ago

Thoughts: the :: and [] parts definitely deserve their own captures. Is the :: the same as, say, the . in something.else in object-oriented languages?

alexjurkiewicz commented 8 years ago

Yes, :: is a namespace separator

On Tue, 30 Aug 2016, 1:37 AM Wliu notifications@github.com wrote:

Thoughts: the :: and [] parts definitely deserve their own captures. Is the :: the same as, say, the . in something.else in object-oriented languages?

β€” You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/atom/language-puppet/pull/44#issuecomment-243161262, or mute the thread https://github.com/notifications/unsubscribe-auth/AAXKdexPKimeEXS4Pe2UTnUeUo5ZlnIdks5qkvylgaJpZM4JvGvd .

winstliu commented 8 years ago

Then it can be tokenized as punctuation.separator.puppet to start.

alexjurkiewicz commented 8 years ago

OK, and what about [ and ] for array subscripts / hash keys?

If I understand the code correctly, $, ${ and } get tokenised as punctuation.definition.variable.puppet -- can you confirm?

winstliu commented 8 years ago

To be honest, I'm not sure about the subscripts since I've never touched Puppet.

alexjurkiewicz commented 7 years ago

I haven't had time to come back to this and I doubt I will -- any chance it could be merged anyway? The captures could be improved as you describe, but at least it fixes the syntax highlighting.