search

beyondgrep / ack2

**ack 2 is no longer being maintained. ack 3 is the latest version.**
https://github.com/beyondgrep/ack3/
Other
1.48k stars 138 forks source link

Improper highlighting for patterns with parenthesis #364

Closed rkleemann closed 7 years ago

rkleemann commented 11 years ago

When specifying a pattern with a set of parenthesis, the highlighted portion is not the full match, but only the parenthesized portion. Interestingly, -o outputs the correct thing.

For example:

$ ( echo foobar ; echo foobaz ; echo foobat ) | ack '\w+(bar|bat|baz)'
foo_bar_
foo_baz_
foo_bat_

Where the highlighted portions are surrounded by the underscores.

$ ack --version
ack 2.08
Running under Perl 5.14.4 at /Users/Shared/perl5/perls/perl-5.14.4/bin/perl
petdance commented 11 years ago

See also #324 and #304.

petdance commented 11 years ago

@hoelzro: I think it would behoove us to stop and rethinking highlighting entirely, including coming up with a way to do Perl tests on highlights. This is clearly a pain point for us, and we need seat belts.

rkleemann commented 11 years ago

Yup, this is definitely a repeat of those bugs.

It would seem that since -o is outputting the correct match, that you have the right logic somewhere, and it just needs to be replicated.

petdance commented 11 years ago

There's that word, "just". :smiley:

The highlighting code is not simple, @rkleemann, which is why I want to overhaul it.

hoelzro commented 11 years ago

Yeah, it could probably use some love. Especially since we have a lot of duplicated stuff for optimization reasons.

gedge commented 10 years ago

one workaround for this is to use (?:...)

( echo foobar ; echo foobaz ; echo foobat ) | ack '\w+(?:bar|bat|baz)'
rkleemann commented 10 years ago

@gedge, yes, that would work around that problem. If you needed to repeat a pattern somewhere though, then it's not going to work properly:

$ ( echo foobarfoo ; echo FoobazFoo ; echo foobatfoo ) | ack '(\w+)(?:bar|bat|baz)\1'
_foo_barfoo
_Foo_bazFoo
_foo_batfoo
petdance commented 7 years ago

This is an intentional feature. It is removed in ack3.

rkleemann commented 7 years ago

I disagree that this is an intentional feature, but I'll patiently wait for ack3 to address it.

petdance commented 7 years ago

I'm not sure how it is that you can "disagree" that it's a feature.

Go look at print_line_with_options and the section of code at if ( @+ > 1 ) { # If we have captures...

rkleemann commented 7 years ago

Ah, well then I have more faith in the description of it being an intended "feature".