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

[WIP] Begin to distinguish between Perl warnings and errors #933

Merged
merged 7 commits into from Mar 2, 2018

Conversation

Projects
None yet
2 participants
@oalders
Contributor

oalders commented Sep 14, 2017

This is a work in progress, but I was interested in getting some feedback.

The biggest change here is, I think, not clobbering messages from previous line numbers. I could also change this to concatenate the messages rather than letting the first one win.

@w0rp

I have a few tips here for what you've got so far.

I also recommend trying to get the test suite running with Docker. If you have Docker installed, you should hopefully be able to run ./run-tests and that should be that. I wouldn't be surprised if there's some issue if you're working from a Mac.

Show outdated Hide outdated ale_linters/perl/perl.vim
Show outdated Hide outdated ale_linters/perl/perl.vim
Show outdated Hide outdated ale_linters/perl/perl.vim

oalders added some commits Sep 8, 2017

Let the first Perl error or warning win.
Take the following example:

***

sub foo {
    my $thing;

***

This might have the following messages when we compile it:

Missing right curly or square bracket at warning.pl line 7, at end of
line
syntax error at warning.pl line 7, at EOF
warning.pl had compilation errors.

With the current behaviour, we just get a "syntax error" message, which
isn't all that helpful.  With this patch we get "Missing right curly or
square bracket".
@oalders

This comment has been minimized.

Show comment
Hide comment
@oalders

oalders Feb 28, 2018

Contributor

I think I've addressed all of the issues cited above. I'd like to live with this for a few days before I remove the WIP label. If this looks ok, would you like the commits squashed?

Contributor

oalders commented Feb 28, 2018

I think I've addressed all of the issues cited above. I'd like to live with this for a few days before I remove the WIP label. If this looks ok, would you like the commits squashed?

@oalders

This comment has been minimized.

Show comment
Hide comment
@oalders

oalders Feb 28, 2018

Contributor

BTW, I had no problems running ./run-tests from my Mac. Worked like a charm.

Contributor

oalders commented Feb 28, 2018

BTW, I had no problems running ./run-tests from my Mac. Worked like a charm.

@w0rp

w0rp approved these changes Mar 2, 2018

@w0rp w0rp merged commit 8a77290 into w0rp:master Mar 2, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@w0rp

This comment has been minimized.

Show comment
Hide comment
@w0rp

w0rp Mar 2, 2018

Owner

Cheers! 🍻

Owner

w0rp commented Mar 2, 2018

Cheers! 🍻

@oalders

This comment has been minimized.

Show comment
Hide comment
@oalders

oalders Mar 2, 2018

Contributor

Thanks @w0rp!

Contributor

oalders commented Mar 2, 2018

Thanks @w0rp!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment