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

Add profile, other options to the perlcritic linter #675

Merged
merged 3 commits into from
Jun 29, 2017

Conversation

rsrchboy
Copy link
Contributor

This should bring the perlcritic linter in line with the perl
linter, allowing for custom (and per-buffer) perlcritic options and
executable.

We also supply a "standard" perltidy profile setting.

This should bring the `perlcritic` linter in line with the `perl`
linter, allowing for custom (and per-buffer) perlcritic options and
executable.

We also supply a "standard" perltidy profile setting..
Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

Good ideas. See my comments.

\ get(g:, 'ale_perl_perlcritic_executable', 'perlcritic')

let g:ale_perl_perlcritic_profile =
\ get(g:, 'ale_perl_perlcritic_profile', '.../.perlcriticrc')
Copy link
Member

Choose a reason for hiding this comment

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

You should use the ale#path#FindNearestFile function below to find .percriticrc instead. See the JSHint implementation for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for that -- I'd mistaken the ... as something perlcritic does, rather than a shell construct.

let l:critic_verbosity = '%l:%c %m [%p]\n'
endif

return "perlcritic --verbose '". l:critic_verbosity . "' --nocolor"
return ale_linters#perl#perlcritic#GetExecutable(a:buffer)
Copy link
Member

Choose a reason for hiding this comment

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

You should apply ale#Escape to the executable here if it's configurable, as the executable path could contain spaces, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've also applied it to the profile file.


Before:
runtime ale_linters/perl/perlcritic.vim
silent noautocmd new testfile.pl
Copy link
Member

Choose a reason for hiding this comment

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

I recommend using ale#test#SetFilename instead. new produces weird output when you run the Vader tests inside of Vim while editing the tests.

@w0rp
Copy link
Member

w0rp commented Jun 25, 2017

I think there might be a commit you haven't pushed yet.

@rsrchboy
Copy link
Contributor Author

There is :) I'm having trouble with your last comment, concerning the use of ale#test#SetFilename(). I can run the tests without a problem in the current incantation, but I can't quite seem to replicate that with ...#SetFilename().

I think I'm just going to bypass that and use a Given: block, allowing Vader to handle it.

Update how we search for profile, setting up a scratch buffer for the
Vader tests, etc.
@w0rp w0rp merged commit 3f1cab3 into dense-analysis:master Jun 29, 2017
@w0rp
Copy link
Member

w0rp commented Jun 29, 2017

Cheers! 🍻

@rsrchboy
Copy link
Contributor Author

Thanks! :)

@rsrchboy rsrchboy deleted the perlcritic-options branch June 29, 2017 22:49
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.

None yet

2 participants