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

Add "smart" autoquote mode #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

goolmoos
Copy link

@goolmoos goolmoos commented Jun 9, 2022

Allows setting the autoquote option to "smart".

When enabled, the part before "--" is taken as is, and the part after is splitted.
For example:
prompt: multi word search -- -f --another-flag
args: "multi word search", "-f", "--another-flag"

like autoquote=true, this is disabled when the prompt begins with one of ' " -

@weeman1337
Copy link
Collaborator

Hi @goolmoos thank you for your contribution.

Would it work for you if the idea from here will be implemented: #13 (comment) :

There has been an excessive discussion about that topic with the conclusion that a shortcut to quote the current prompt would solve most of the issues. In terms of key presses -- = 2 and a shortcut like Ctrl + ' are 2 as well.

From your example:

  • Prompt: multi word search
  • Press Ctrl + '
  • Prompt: "multi word search" _ (_ = cursor position)

@weeman1337 weeman1337 self-requested a review June 12, 2022 16:34
@ofirgall
Copy link

Hey @weeman1337 I actually support @goolmoos idea, it feel more intuitive to use -- like any cli tool, also ripgrep doesn't parse -- so it doesn't conflict with it.

@weeman1337
Copy link
Collaborator

Can you give this branch a try #22 ?


it feel more intuitive to use -- like any cli tool

Can you give me an example? Using rg from the CLI doesn't work like this:

$ rg @param User -- -tphp
User: No such file or directory (os error 2)
-tphp: No such file or directory (os error 2)

Instead I would do:

$ rg "@param User" -tphp

The latter is exactly what the branch provides you a shortcut for.

@jesseleite
Copy link
Contributor

This wouldn't mess with the smart quoting behaviour described in #6, or the ctrl-k mapping in #22, would it?

@goolmoos
Copy link
Author

goolmoos commented Jul 7, 2022

No, It would not mess with anything.

@steliyan
Copy link
Contributor

You can check my comments here, and see if they make any sense: #27 (comment)

I don't know if I came up with the idea of using -- for this plugin or I read this thread sometime in the past and I've just retold the story. :)

@goolmoos
Copy link
Author

Personally I find the -- method easier and more intuitive to use than the quote mapping.
It seems that I am not the only one that prefers this method (for example, ofirgall earlier in this thread), and since the two methods don't prevent each other from working, I would be glad to see this merged.

@jesseleite
Copy link
Contributor

My main problem with the -- delimiter is that the prompt input for this live-grep-args picker is supposed to let you pass your own args to the command line, and -- actually means something on the command line.

also ripgrep doesn't parse --

^ I didn't realize ripgrep doesn't parse --, but even so, this suggests opposite behaviour to how bash builtins handle --. As mentioned in the above link...

A double dash (--) is used in most Bash built-in commands and many other commands to signify the end of command options, after which only positional arguments are accepted.

Whereas the functionality being suggested in this PR suggests you can pass options/flags after the --, which is not normally possible with other bash built-ins and commands that parse the --.

If "live-grep-args" is meant to allow you to pass "args" directly to ripgrep just as you would on the command line, then I'd lean towards saying prompt input should reflect that. Rather than relying on custom prompt input magic, why not leverage the picker mapping system (as @weeman1337 did with his awesome quote mapping, especially considering it's the same number of keystrokes even)?

Also as an aside, if we start treating -- in the prompt input as a magic delimiter, wouldn't that make searching for a lua comment like -- TODO a bit more unpredictable? For example, say I'm searching for all my 'todo' comments in a lua codebase, ie)

function isAppClosed(appName)
  return not isAppOpen(appName) -- TODO: Refactor this
end

Anyway, just some thoughts ❤️

@goolmoos
Copy link
Author

Regarding the meaning of the double dash, this is exactly why I chose it, as its meaning here is to indicate a change of interpretation for the following arguments. I agree that it is not exactly the same as the "regular" meaning, but I found it intuitive to use, and I am not the only one that feels that way:

Hey @weeman1337 I actually support @goolmoos idea, it feel more intuitive to use -- like any cli tool

As to the problem with searching for lua comments, you can always quote the prompt, and it will work as expected: "-- TODO" in this case.

And of course, if someone finds this mechanism confusing, it is entirely optional and doesn't have to be turned on, the default is still auto_quoting=true

@ofirgall
Copy link

I know I already said my opinion but after trying both ways the auto quote method feels much more intuitive, if both ways can leave in the source code and can be enable by an option it will be the best.

the part before -- is taken as is, and the part after is splitted.
For example:
multi word search -- -flag --another-flag =>
"multi word search", "-flag", "--another-flag"
@goolmoos goolmoos force-pushed the add-smart-autoquote branch from 0bb516d to e50914e Compare December 19, 2022 10:27
@goolmoos goolmoos force-pushed the add-smart-autoquote branch from e50914e to 6168c45 Compare December 19, 2022 10:29
@weeman1337
Copy link
Collaborator

Thanks for updating this PR @goolmoos 👍 I will review it soon.

@weeman1337
Copy link
Collaborator

After thinking once more about the discussion above, I don't want to add this to the parser. Instead I've opened a pull request, that enables you to provide a custom parser function: #49

The „smart“ auto quoting is controversial (see discussion) and I don't want to maintain a higher complexity in the parser.

Can you test if that is enough to implement the „smart auto quoting“? We can probably maintain different parser strategies in issues or the wiki of this project. If you can post your snippet here I will create an issue for that.

@goolmoos
Copy link
Author

It probably can be implemented using the custom parser, by copying the code of the parser.
Doing this would cause a lot of code duplication, and on every change to the parser the custom parser will need to be rebased, but it will work.

@weeman1337
Copy link
Collaborator

Coming back to this. I have read the ripgrep man page and found this for the pattern:

 INPUT OPTIONS
       -e PATTERN, --regexp=PATTERN
           A  pattern  to  search for. This option can be provided multiple times, where all patterns given are searched, in addition to any
           patterns provided by -f/--file. Lines matching at least one of the provided patterns are printed.  This flag  can  also  be  used
           when searching for patterns that start with a dash.

           For example, to search for the literal -foo:

               rg -e -foo

           You can also use the special -- delimiter to indicate that no more flags will be provided. Namely, the following is equivalent to
           the above:

               rg -- -foo

I would be up for providing e a smart quote option that behaves like described. That would mean it will behave exactly the opposite of what is suggested in the description, e.g.

Prompt: --iglob src/** -- something

What do you think?

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

Successfully merging this pull request may close these issues.

5 participants