Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mode updates #1236

Merged
merged 7 commits into from
Feb 7, 2013
Merged

Mode updates #1236

merged 7 commits into from
Feb 7, 2013

Conversation

gjtorikian
Copy link
Contributor

Fixes #1219
Fixes #1235

@redchair123
Copy link

You are replacing the regex for the shell comment to

regex : "[^\{]#.*$"

I think that's also broken, because

echo 123#456

isn't actually commented (so it would print 123#456). I think you probably want " #.*$"

@gjtorikian
Copy link
Contributor Author

@nightwing That doesn't work either, what about comments that start right at the beginning of the line (without a space)?

Maybe... .?#.*$ ?

@nightwing
Copy link
Member

maybe /(?:^|\s)#.*$/?

@redchair123
Copy link

@gjtorikian @nightwing what you really need here is a lookbehind.

@nightwing The only downside is that it captures the whitespace character before the hash as well.

@gjtorikian if you are willing to accept a more complicated solution, how about capturing /(^|\s)(#.*)$/ and grabbing the capture block? To show the difference:

> "abc #def".match(/(^|\s)#.*$/)
[" #def"]
> "abc #def".match(/(?:^|\s)(#.*)$/)
[" #def", "#def"]

@gjtorikian
Copy link
Contributor Author

I don't see how /(?:^|\s)#.*$/ is capturing the whitespace. ?: is effectively telling the highlighter to ignore these parens. It would only mark the rest up.

@redchair123
Copy link

(?:) instructs the regexp engine not to create a separate capture group (meaning you won't get a separate whitespace character in the match) but it will be contained in the complete match.

To explain the difference:

>"abc #def".match(/(?:^|\s)#.*$/)
[" #def"] 

Notice how that whitespace in front of # was captured. That's because the entire match includes the non-captured groups. To show an example involving multiple non-capturing groups:
> "foo#bar".match(/(?:^|[a-z])#(?:b|$)/)
["o#b"]
> "#bar".match(/(?:^|[a-z])#(?:b|$)/)
["#b"]
> "#".match(/(?:^|[a-z])#(?:b|$)/)
["#"]
> "foo#".match(/(?:^|[a-z])#(?:b|$)/)
["o#"]

When using lookaheads, you have to select for the part you want:

>"abc #def".match(/(?:^|\s)(#.*)$/)
[" #def", "#def"]

and use the capture (index 1).

@gjtorikian
Copy link
Contributor Author

Ok, I got it and made the appropriate change/fix.

@nightwing
Copy link
Member

Looks good! Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bash syntax highlighting doesn't support array length C++ highlighting bug? - continue
3 participants