Added possibility to specify global ignores. #73
Conversation
|
If you agree on this I can try to add test cases and documentation. |
| @@ -252,6 +252,10 @@ sub new_from_conf_file { | |||
| die "no such file '$conf_file'" unless $conf_file->is_file; | |||
| my $conf_params = $class->_read_conf_file($conf_file); | |||
| my $main_params = delete( $conf_params->{'_'} ) || {}; | |||
| my $global_ignores = delete $main_params->{'ignore'} || []; | |||
There was a problem hiding this comment.
There's no need for delete here.
| @@ -252,6 +252,10 @@ sub new_from_conf_file { | |||
| die "no such file '$conf_file'" unless $conf_file->is_file; | |||
| my $conf_params = $class->_read_conf_file($conf_file); | |||
| my $main_params = delete( $conf_params->{'_'} ) || {}; | |||
| my $global_ignores = delete $main_params->{'ignore'} || []; | |||
| for my $plugin_name (keys %{ $conf_params }) { | |||
| push @{ $conf_params->{$plugin_name}->{'ignore'} || []}, @$global_ignores; | |||
There was a problem hiding this comment.
I don't think altering the plugin config like this is the way to make this work. I know it's the easiest approach, but it doesn't seem right to me.
I think it'd be better to add an attribute to the Code::TidyAll object called global_ignores and then use that in the find_matched_files method.
|
I see your point, and I just tried to change it as suggested. There is one drawback with that: if somebody doesn't use |
|
Ok, please ignore my previous comment. I implemented it as you wished, the tests are all passing. |
| @@ -112,6 +112,12 @@ has 'plugins' => ( | |||
| required => 1, | |||
| ); | |||
|
|
|||
| has 'global_ignore' => ( | |||
| is => 'ro', | |||
| ias => t('ArrayRef'), | |||
|
This looks generally good modulo the typo that @2shortplanks noticed. |
|
Fixed. This was a C&P error from a few lines above that had the same error (and I'm not familiar with Moo(se)). I also fixed that line. |
| @@ -659,6 +669,10 @@ sub find_matched_files { | |||
| my $root_length = length( $self->root_dir ); | |||
| foreach my $plugin ( @{ $self->plugin_objects } ) { | |||
| my @selected = grep { -f && !-l } $self->_zglob( $plugin->selects ); | |||
| if ( @{ $self->global_ignore || [] } ) { | |||
There was a problem hiding this comment.
If global_ignore had a default of sub { [] } you wouldn't need the || [] bit here.
Also, this could be combined with the check for the plugin ignores in one map statement and call to $self->_zglob.
| has 'select_regex' => ( is => 'lazy' ); | ||
| has 'selects' => ( is => 'lazy' ); | ||
| has 'global_ignore_regex' => ( is => 'lazy' ); | ||
| has 'global_ignores' => ( is => 'lazy' ); |
There was a problem hiding this comment.
Instead of making every plugin generate these two attributes over and over let's just move the regex to Code::TidyAll and then call $self->tidyall->global_ignore_regex in the matches_path method.
|
POD and tests modified to cover global ignores. I copied the |
|
I could need some help with the Storable issue. It seems that dclone cannot handle CODE refs, which causes failures on unit tests passing in a custom msg_output handler. Maybe it would be better to do without Storable? |
|
Solved. Dependency on Storable is removed again. Reason was a buggy _recurse_dump() implementation, which was broken at least for array refs. |
|
Looks like the code isn't tidy ;) |
|
Should be fixed, sorry. |
|
I did some additional fiddling and merged this from the CLI. Thanks! |
As suggested in #70.