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

Strange behaviour when search string begins with + character #378

Open
epa opened this issue Jun 3, 2024 · 11 comments
Open

Strange behaviour when search string begins with + character #378

epa opened this issue Jun 3, 2024 · 11 comments

Comments

@epa
Copy link

epa commented Jun 3, 2024

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?

@blmatthews
Copy link

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.

@epa
Copy link
Author

epa commented Jun 3, 2024

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.

@blmatthews
Copy link

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.

@n1vux
Copy link
Contributor

n1vux commented Jun 3, 2024

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
Copy link
Contributor

n1vux commented Jun 3, 2024

i may go look

hah, @epa Ed already did so. sciurius/perl-Getopt-Long#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
Copy link
Contributor

n1vux commented Jun 3, 2024

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
Copy link
Contributor

n1vux commented Jun 3, 2024

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 ! 👀 🙄

@petdance
Copy link
Collaborator

petdance commented Jun 3, 2024

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?

@n1vux
Copy link
Contributor

n1vux commented Jun 3, 2024

I haven't written tests , no

@petdance
Copy link
Collaborator

petdance commented Jun 3, 2024

I haven't written tests , no

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

@epa
Copy link
Author

epa commented Jun 4, 2024

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" );
    }
};

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

No branches or pull requests

4 participants