Skip to content
This repository was archived by the owner on Aug 7, 2023. It is now read-only.

Move to a broader regex#28

Merged
keplersj merged 2 commits intoAtomLinter:masterfrom
Arcanemagus:fix-regex
Aug 13, 2015
Merged

Move to a broader regex#28
keplersj merged 2 commits intoAtomLinter:masterfrom
Arcanemagus:fix-regex

Conversation

@Arcanemagus
Copy link
Copy Markdown
Member

Also generate the messages in the linter to fix several issues such as the file path being incorrect on Windows. Since ruby only gives a line number for errors provide a range that covers the entire line.

Should fix steelbrain/linter#822 and make the errors show on the file instead of the "project" on Windows.

Review on Reviewable

Also generate the messages in the linter to fix several issues such as
the file path being incorrect on Windows. Since ruby only gives a line
number for errors provide a range that covers the entire line.
@Arcanemagus
Copy link
Copy Markdown
Member Author

For reference here is a test of the regex on every message variation that I could find as a possible output in the Ruby source: https://regex101.com/r/zC2nG8/4

lib/main.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of going one level deep, how about you use a return

Removes the unnecessary `msg` variable. Also returns early if no matches
were found instead of wrapping the entire inner part in the if block.
@Arcanemagus
Copy link
Copy Markdown
Member Author

Anything you can spot wrong with this @k2b6s9j? Anything else @steelbrain?

@steelbrain
Copy link
Copy Markdown
Contributor

Ruby is not my thing, I'll leave this to @k2b6s9j

@keplersj
Copy link
Copy Markdown
Contributor

While I would prefer to have the error selected in cases where it is shown ( for example, in the case of syntax errors ) this does fix the issue very well. Thank you @Arcanemagus.

keplersj added a commit that referenced this pull request Aug 13, 2015
@keplersj keplersj merged commit 3f5620d into AtomLinter:master Aug 13, 2015
@keplersj
Copy link
Copy Markdown
Contributor

Thank for demonstrating the regex using regex101 by the way.

@keplersj
Copy link
Copy Markdown
Contributor

This will be publish in the next patch release.

@Arcanemagus Arcanemagus deleted the fix-regex branch August 13, 2015 20:39
@Arcanemagus
Copy link
Copy Markdown
Member Author

How does this look? https://regex101.com/r/zC2nG8/5

(I'm assuming that you meant you wanted the "syntax error, " part cut out of the messages where it is irrelevant.)

@keplersj
Copy link
Copy Markdown
Contributor

It looks nice. Thanks for that!

@Arcanemagus
Copy link
Copy Markdown
Member Author

Btw version 2 of that also includes a capturing group for the traces, but would need the output processed as a whole instead of line-by-line. If you were interested 😉.

@keplersj
Copy link
Copy Markdown
Contributor

Oh boy. That sounds great. I'm busy right now with prepping for the upcoming school year. So I'm really only maintaining things right now, not writing new things. If you have the time and are interested you could implement it in a pull request and I'll merge it in.

@keplersj
Copy link
Copy Markdown
Contributor

This has been published in version 1.0.2.

@Arcanemagus Arcanemagus added the bug label Dec 2, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Uncaught InvalidCharacterError: Failed to execute 'add' on 'DOMTokenList': The token provided ('u...

3 participants