asbjornenge / Docker.tmbundle

Dockerfile syntax
MIT License
75 stars 27 forks source link

WIP: Highlight missing \ #6

Closed andyneff closed 7 years ago

andyneff commented 7 years ago

I'm trying to add a special syntax highlight case. I want the newlines not starting with keywords to be highlighted. For example

RUN cd /tmp
    touch file

Results in the spaces and the word touch being highlighted in the invalid color. The purpose of this is to catch missing \'s when writing a docker file.

Currently the only bug with this is, the following example is not highlighted

RUN echo "Hello
    world"
    ls

Because the quote match ends before the newline match can end, and this can not match the newline pattern. I don't know the syntax language enough to fix this.

@asbjornenge Are you interested in this idea?

asbjornenge commented 7 years ago

Yeah, sounds like a good idea to me :smile: I would like an extra pair for eyes on this though since I don't know the sublime syntax all that well either.

Maybe @princemaple could take a quick look?

I would also like to keep the sublime and the textmate versions in sync. Do you any experience with the tmLanguage syntax? Would be great to have the same functionality in both. I'm a bit swamped atm. But there are some neat conversion tools you could take a look at maybe :+1:

princemaple commented 7 years ago

Will take a look.

princemaple commented 7 years ago

Hi @andyneff, could you clarify the expected behavior when quoted string spans multiple lines? I sense the regex could be simplified here. Also, do we still need the newline clause with your latest change?

(Haven't got to test it yet, in bed now and will do it tomorrow)

princemaple commented 7 years ago

Just got to play with this PR a little bit, very interesting addition :)

princemaple commented 7 years ago

This PR is good, but it has one flaw:

KEYWORD "string"
    "string"
^^^^ <- not highlighted

I think I have a way to fix it, but it will need some big rewrite :)

andyneff commented 7 years ago

@princemaple Correct, I still have not figured out how to fix that one case. It's because of the string on the first line, this also fails too

KEYWORD "any string"
    ls /tmp
^^^^ <- not highlighted

I'm open to any suggestions. I understand why it happens. Sublime syntax highlighting is limited to single line matches only (as documented). Which is why I had to rely on the push/pop system. And pop match takes the trailing quote, and so I can't match it again for the invalid highlighting. I've tried all the simple ideas I've come up with, but no luck.

Glad the idea is well received though. I know when writing Dockerfiles all day, the missing slash gets me every now and then, which is why I came up with the idea.

andyneff commented 7 years ago

I played around with that a little, I like the improved highlighting result

I was not able to tweak the following examples to work

RUN echo hello \

    echo world
RUN echo hello \
#comment
    echo world
RUN echo hello \

#comment

    echo world

I haven't tried your new merged version yet though.

princemaple commented 7 years ago

I actually intentionally left out the first case you mentioned, I thought it wasn't supposed to work. I can tweak it to make it work tomorrow.

On Sat, 17 Sep 2016, 23:41 Andy Neff notifications@github.com wrote:

I played around with that a little, I like the improved highlighting result

I was not able to tweak the following examples to work

RUN echo hello \

echo world
  • It doesn't ignore the blank (or white spaced) line, which is valid

RUN echo hello \

comment

echo world
  • It doesn't color code the comment, which is also valid

RUN echo hello \

comment

echo world
  • Just a mix of the two

I haven't tried your new merged version yet though.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/asbjornenge/Docker.tmbundle/pull/6#issuecomment-247770644, or mute the thread https://github.com/notifications/unsubscribe-auth/ABRKNCULsRPAVpfk6U0HMBhUy8fUSlWCks5qq-4ggaJpZM4J9MkX .

princemaple commented 7 years ago

Comments can be added quite easily, too.

asbjornenge commented 7 years ago

Good job you guys :heart_eyes: :tada: