--context=0 wrongly gives 2 lines of context

[epa]

--context=0 or -C 0 prints two lines of context before and after, not zero as requested. I believe the fix is

diff --git a/ConfigLoader.pm b/ConfigLoader.pm
index 9a92b95..8a0ecf1 100644
--- a/ConfigLoader.pm
+++ b/ConfigLoader.pm
@@ -281,7 +281,7 @@ EOT
         'A|after-context=i' => \$opt->{after_context},
         'B|before-context=i'
                             => \$opt->{before_context},
-        'C|context:i'       => sub { shift; my $val = shift; $opt->{before_context} = $opt->{after_context} = ($val || 2) },
+        'C|context:i'       => sub { shift; my $val = shift; $opt->{before_context} = $opt->{after_context} = ($val // 2) },
         'a'                 => removed_option('-a', $dash_a_explanation),
         'all'               => removed_option('--all', $dash_a_explanation),
         'break!'            => \$opt->{break},

[epa]

...or (defined $val ? $val : 2) since the minimum perl supported by ack is currently 5.8.8.

[petdance]

Seems to me that -A, -B or -C of 0 should be an error. Why were you trying to get 0 lines of -C?

[epa]

I was just playing with the different options - trying to get ordinary grep format as it happens.

For my own uses of ack I would not mind if -C 0 were an error; on the other hand, it seems simple to implement and to explain so might as well be allowed. Certainly I can see some utility for showing context after but no lines of context before (-A 0) and vice versa. That can be useful for quick and dirty multiline matching.

[petdance]

I don't see how -C 0 would be any different than omitting it.

[epa]

I imagined it would have the behaviour given by the patch above: switch to the -C style output format, and specify zero lines of context before and after. This differs from the normal output in that a -- separator is printed between each match. I agree, this is not particularly useful, but then again head -n 0 is not that useful either, but still works for the sake of regularity.

[epa]

FWIW, grep --context=0 works in the way I describe.

[petdance]

Just had a user email me with another case. He's got --context=2 in his .ackrc, and --context=0 on the command line will not override it. THAT is a big problem.

[petdance]

@epa: Did you try your proposed fix above? Did it work? Pass tests? It's not passing tests for me.

[epa]

Sorry, you are right, the patch doesn't work. If the integer after --context is omitted then Getopt::Long sets the value to zero (rather than undef), so you can't distinguish between an explicit zero and the argument being left out.

I agree that having no way to override --context is a problem. However, it might make more sense to turn it off with --no-context or similar. That would allow --context=0 to work the same way as it does in grep. (I don't mind for my own use, but decide whether to "do what grep does".)

If you do choose to follow grep and have --context=0 mean print the -C format but with zero lines of context, then some enhancement to Getopt::Long might be needed.

[petdance]

--no-context might make sense, but we still have to allow -B 0 to override -B 2, and -A 0 to override -A 2. :-(

[petdance]

Aha, looks like we can specify a default value

: number [ desttype ]
Like :i , but if the value is omitted, the number will be assigned.

[epa]

Ah, so you can set the default to -999 or something and then check for that.

[petdance]

Or just -1 and anything negative, whether because of no argument specified, or a negative number passed as in --context=-5 get the default of 2.

If you want to go after this, I have added tests to the context branch that are currently failing. Or I can tend to it tomorrow.

[epa]

Thanks, I will leave it to you.

[petdance]

Sounds good. Thanks for the research into the default values. I, too, just assumed that no value would get an undef instead of a 0.

[petdance]

Fixed. 0b75aa4

[epa]

Cool. I think the docs may need to mention that --context=-1 now means reset it to the default.

[petdance]

--context=1 is not the default. --context=0 is.

[petdance]

I've added it to the docs. 619a8f5

[epa]

It might be worth an explicit mention that it's different from grep (where --context=0 is for zero lines of context). In the current version of ack, --context=0 is the same as --context=2.

[petdance]

It now matches grep. I just fixed it. That's why this ticket is closed.

[epa]

Ah, sorry for the spam - this is fixed in 619a8f50d2beeaa1f49a6f36fa8e8413469081e8 but not yet on the master branch. [blushes]

[petdance]

It won't be on master until I make a release of 2.17_01. It's not even on dev branch yet, because I'm doing more fixing to context. By the end of the day it should be on dev and then released to CPAN.

[epa]

...however, there is still a sense in which it does not match grep.

% seq 1 10 >numbers.more
% grep --context=0 5 numbers numbers.more
numbers:5
--
numbers.more:5
% perl -Iblib/lib ./ack --context=0 5 numbers numbers.more
numbers
5:5

numbers.more
5:5

With grep, --context=0 means use the -C output format, printing zero lines of context before and after each match. With ack, --context=0 is the same as turning off -C altogether and going back to the normal format.

Another oddity is that while -A 0 -B 1 works as expected, and so does -A 1 -B 0, specifying -A 0 -B 0 again changes the output format altogether, and isn't really consistent with the earlier two.

(this on branch 'context' by the way, revision 619a8f50d2beeaa1f49a6f36fa8e8413469081e8)

[petdance]

Please wait until I have 2.17_01 out and then make reports against that. No point in reporting on things that I am working on right now. Thanks.

[epa]

OK -- will reflile this against the new release (unless you would like me to provide test cases or a possible patch for your review).

[petdance]

Fixed on dev in 64505b2

[epa]

Sorry, I still see it behaving differently to grep as outlined above in https://github.com/petdance/ack2/issues/595#issuecomment-286777464

This is with revision 64505b2236a13a71c0cf53d0853e0fab875befc6

I believe the following test case added to t/context.t will check that it handles --context=0 in the way grep does:

# But although --context=0 prints no context lines before and after,
# it should still output in the 'context' format.
#
CONTEXT_ZERO_MULTIPLE_MATCHES: {
    my $target_file = File::Next::reslash( 't/text/boy-named-sue.txt' );
    my @expected = split( /\n/, <<"EOF" );
And it got a lot of laughs from a' lots of folks,
--
And some guy'd laugh and I'd bust his head,
--
I heard him laugh and then I heard him cuss,
EOF

    my $regex = 'laugh';
    my @files = qw( t/text );
    my @args = ( '-C', 0, $regex );

    ack_lists_match( [ @args, @files ], \@expected, "Looking for $regex - context=0" );
}

[epa]

(If you are still working on this then apologies -- I hope the test case will be useful.)

[petdance]

With grep, --context=0 means use the -C output format, printing zero lines of context before and after each match. With ack, --context=0 is the same as turning off -C altogether and going back to the normal format.

I'm OK with that.

[epa]

OK -- this is what I meant by "It might be worth an explicit mention that it's different from grep".