ack3 icon indicating copy to clipboard operation
ack3 copied to clipboard

Strange behaviour when search string begins with + character

Open epa opened this issue 1 year ago • 11 comments

I want to search for the literal string +1 but the obvious command line does something strange:

% ack -Q '+1'
ack: No regular expression found.

I can reproduce this also with current dev branch.

I believe it's caused by Getopt::Long, which by default accepts the obsolete + prefix for long options as an alternative to --. (Some GNU utilities used to accept the + form but moved to -- in the early 1990s.)

It should be possible to say

use Getopt::Long qw(:config prefix_pattern=--? );

or alternatively

Getopt::Long::Configure('prefix_pattern=--?')

but because of the way the code calls that module I wasn't successful in creating a patch. Could you have a look please?

epa avatar Jun 03 '24 11:06 epa

Ending the options part of the command line arguments with -- is the standard way to handle this, for example:

$ rm -x
usage: rm [-f | -i] [-dIPRrvWx] file ...
       unlink [--] file
$ rm -- -x
rm: -x: No such file or directory

So ack -Q -- '+1' would do what you want.

blmatthews avatar Jun 03 '24 16:06 blmatthews

Yes, -- works around it and I would use that if the pattern began with a - character. But it's strange that it should be needed for the + character too. You could document that, but in my opinion it would be simpler to treat arguments beginning with + as normal ones.

epa avatar Jun 03 '24 16:06 epa

To be clear, I don’t have anything to do with ack other than as a satisfied user. Just trying to be helpful by pointing out the standard (far beyond just ack) way of getting around the “non-option argument looks like an option” problem.

blmatthews avatar Jun 03 '24 17:06 blmatthews

Getopt::Long documentation says

The + form is now obsolete and strongly deprecated.

which we can all agree upon, but i do not quickly see any Getopt::Long setting to disable it; so any technique to

simpler to treat arguments beginning with + as normal ones.

(as suggested) are likely to be brittle / fragile. Worth considering, I may go look at Getopt::Long's Issues, but unlikely worth the maintenance complexity to fix on our end in Ack?

While ack -Q -- '+1' is generic "take following args as not a flag args", leveraging ack -Q to ignore the other, RE meaning of + , an alternative that would prevent both flag and RE meanings simultaneously is

ack '[+]1'
(or uglier ack \\+1 , one \ to escape the second \ from shell) (on Windows CMD line, probably have to use "" instead of '' unless using WSL or similar shell shell as one does)

(And thank you @blmatthews for being good community member here.)

n1vux avatar Jun 03 '24 18:06 n1vux

i may go look

hah, @epa Ed already did so. https://github.com/sciurius/perl-Getopt-Long/issues/30

wherein maintainer Johan Vromans replies

You can set environment variable POSIXLY_CORRECT to a true value, or use Getopt::Long qw(:config prefix_pattern=--? );

which are documented in the POD. So yes, we could force it. (I'm unconvinced the above syntax is quite correct but hint is there.)

n1vux avatar Jun 03 '24 18:06 n1vux

I note an issue with the suggested ENV fix.

$ alias ack6="POSIXLY_CORRECT=1 ack"
$ ack6 +1
Unknown option: type-add
…
Unknown option: type-add
ack: Invalid option in Defaults

So that workaround may not be available. Unclear immediately if bug is in Ack or Getopt::Long.

n1vux avatar Jun 03 '24 18:06 n1vux

I would expect the combination of args below to not read $HOME/.ackrc, but i had to comment out --type-set there in order to test further.

$ alias ack6="POSIXLY_CORRECT=1 ack"
$ echo > empty.ackrc
$ ack6 --ackrc=./empty.ackrc --ignore-ack-defaults  -Q +1
ack: Invalid regex '+1'
Regex: +1
       ^---HERE Quantifier follows nothing in regex

-Q isn't even protecting the +1 from RE correctly, once i've cheated hard to protect it from Getopt?? ???

Either we need an additional flag to not process $HOME/.ackrc or there is a bug in --ackrc=file ?

Edgecases are fun ! :eyes: :roll_eyes:

n1vux avatar Jun 03 '24 19:06 n1vux

because of the way the code calls that module I wasn't successful in creating a patch.

Do you have any tests written that would save me from writing them?

petdance avatar Jun 03 '24 21:06 petdance

I haven't written tests , no

n1vux avatar Jun 03 '24 22:06 n1vux

I haven't written tests , no

I was asking @epa who sounded like he had been working on code.

petdance avatar Jun 03 '24 23:06 petdance

Here's an example test to add to ack-Q.t which currently fails:

subtest 'Plus sign at start' => sub {
    plan tests => 2;

    my @args = qw( +ssh t/swamp );
    ack_lists_match( [ @args ], [], 'No matches without the -Q' );

    my $target = reslash( 't/swamp/Rakefile' );
    my @expected = line_split( <<"HERE" );
$target:44:  baseurl = "svn+ssh:/#{ENV['USER']}\@rubyforge.org/var/svn/#{PKG_NAME}"
$target:50:  baseurl = "svn+ssh:/#{ENV['USER']}\@rubyforge.org/var/svn/#{PKG_NAME}"
HERE
    for my $arg ( qw( -Q --literal ) ) {
        ack_lists_match( [ @args, $arg ], \@expected, "$arg should make svn+ssh finable" );
    }
};

epa avatar Jun 04 '24 05:06 epa