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

Command flag toggling support for consult--async-command #238

Closed
wentasah opened this issue Feb 27, 2021 · 8 comments
Closed

Command flag toggling support for consult--async-command #238

wentasah opened this issue Feb 27, 2021 · 8 comments

Comments

@wentasah
Copy link

I'd like to have narrowing working with consult--async-command in a way that narrowing changes the executed command (e.g. adds a flag). As far as I can see, this is currently not supported because the command (cmd argument) is not reevaluated after narrowing. Is that true?

If yes, what would be needed to have this working? I'm not much familiar with how async stuff works in consult, but maybe accepting cmd as a function (in addition to string) would be sufficient. What do you think?

I can see that adding a flag to the command can be done by inserting -- followed by the flags, but that's not as convenient as narrowing.

@minad
Copy link
Owner

minad commented Feb 28, 2021

Narrowing should work with async commands, but as of now none of the async commands make actual use of it. Narrowing only means to restrict the set of the candidates to a subset. Narrowing is not meant to influence the generation of async candidates.

In order to achieve toggling of the async command flags, you can adjust the minibuffer content directly.

(defun consult-grep-ignore-case ()
  (interactive)
  (let* ((str (minibuffer-contents-no-properties))
         (new-str
          (if (string-match-p " -- --ignore-case" str)
              (replace-regexp-in-string " -- --ignore-case" "" str)
            (replace-regexp-in-string  "\\`\\(#[^#]*\\)" "\\1 -- --ignore-case" str 'fixedcase))))
    (delete-minibuffer-contents)
    (insert new-str)))

(define-key consult-async-map (kbd "M-c") #'consult-grep-ignore-case)

Is that a satisfactory solution or do you want to see a narrowing-like indicator in the prompt instead of the command option? I think seeing the option is actually better, since then it is part of the input string and will be part of the input history. The narrowing flag in contrast is out of band information, which is not stored in the input string.

If you really want to use an out-of-band flag one would have to adapt the consult--command-args function.

(defvar-local consult--flags nil)
(defvar-local consult--flags-overlay nil)

(defun consult--command-args (cmd)
  "Split command arguments and append to CMD."
  (setq cmd (split-string-and-unquote cmd))
  (lambda (input)
    (save-match-data
      (let ((opts))
        (when (string-match " +--\\( +\\|\\'\\)" input)
          ;; split-string-and-unquote modifies the match data
          ;; and fails if the quotes are invalid. Ignore it.
          (setq opts (substring input (match-end 0))
                input (substring input 0 (match-beginning 0))
                opts (ignore-errors (split-string-and-unquote opts))))
        (unless (string-blank-p input)
          (mapcan (lambda (x)
                    (if (string= x "OPTS")
                        (append opts consult--flags) ;; MODIFICATION: Add consult--flags
                      (list (replace-regexp-in-string "ARG" input x 'fixedcase 'literal))))
                  cmd))))))

(defun consult-grep-ignore-case ()
  (interactive)
  (when consult--flags-overlay
    (delete-overlay consult--flags-overlay))
  (if consult--flags
      (setq consult--flags nil)
    (setq consult--flags '("--ignore-case")
          consult--flags-overlay
          (consult--overlay
           (- (minibuffer-prompt-end) 1) (minibuffer-prompt-end)
           'before-string
           (propertize " [I]"
                       'face 'consult-narrow-indicator))))
  (insert " ")
  (delete-char -1))

(define-key consult-async-map (kbd "M-c") #'consult-grep-ignore-case)

Currently I am a bit hesitant to add such functionality until it is fully clear how everything should look. There has also been the idea to add a transient interface to the async grep commands (#170).

@minad minad changed the title Narrowing support for consult--async-command Command flag toggling support for consult--async-command Feb 28, 2021
@wentasah
Copy link
Author

Thanks for quick answer :)

Narrowing only means to restrict the set of the candidates to a subset. Narrowing is not meant to influence the generation of async candidates.

Yes, I know. I'm playing with @jaor's consult-notmuch and want to add narrowing by date, which in its simplest form means adding something like date:7days..now to the query.

In order to achieve toggling of the async command flags, you can adjust the minibuffer content directly.

(define-key consult-async-map (kbd "M-c") #'consult-grep-ignore-case)

That's a good idea. I think I could use this as a quick solution for my use case, but I can see two problems with it:

  • Adding date:... to the minibuffer content would only make sense for consult-notmuch and not for other async commands, which all share consult-async-map. Perhaps, this could be resolved by using custom :keymap argument to consult--read.

  • More importantly, preferred implementation for this kind of narrowing would be to modify the minibuffer like this:

     date:7days..now and (PREVIOUS MINIBUFFER CONTENT)
    

    which would make editing of just the content of the parentheses less convenient.

Is that a satisfactory solution or do you want to see a narrowing-like indicator in the prompt instead of the command option? I think seeing the option is actually better, since then it is part of the input string and will be part of the input history. The narrowing flag in contrast is out of band information, which is not stored in the input string.

For the above mentioned reason, I think that narrowing-like indicator would be better, but I also understand the advantages of modifying the minibuffer.

If you really want to use an out-of-band flag one would have to adapt the consult--command-args function.

OK, I'll look more closely at this. Thanks for the example.

Currently I am a bit hesitant to add such functionality until it is fully clear how everything should look.

Agreed. I'll try to come up with something that we can discuss here.

@minad
Copy link
Owner

minad commented Feb 28, 2021

Adding date:... to the minibuffer content would only make sense for consult-notmuch and not for other async commands, which all share consult-async-map. Perhaps, this could be resolved by using custom :keymap argument to consult--read.

Yes, that's better. I considered writing this, but didn't want to over complicate my answer. Since you are aware of all the details, I should have been more precise.

For the above mentioned reason, I think that narrowing-like indicator would be better, but I also understand the advantages of modifying the minibuffer.

Okay.

Agreed. I'll try to come up with something that we can discuss here.

If you come up with a coherent design, we can include this. Feel free to propose a PR. Ideally it should be somehow extensible, such that async commands can modify the flags via some toggles. The toggles could either be individual commands or a transient UI. The question is also what to do with the options already present in the consult-grep-command variable for example, in a full-featured solution it should be possible to toggle them too.

@minad minad mentioned this issue Mar 2, 2021
20 tasks
@oantolin
Copy link
Contributor

oantolin commented Mar 4, 2021

You can also use --, @rileyrg, #search_term -- extra_cmdline_args#filter_on_this.

@rileyrg
Copy link

rileyrg commented Mar 5, 2021

You can also use --, @rileyrg, #search_term -- extra_cmdline_args#filter_on_this.

Yup, a reddit user pointed this out. Great. Thank you. I missed that. It might seem minor, but it would be nice to allow this option before the search term in order not to search unnecessary files (more important when editing remotely) but maybe that's not possible with the current input format. eg if grep input field starts with "--" don't start async grep until the first #+x characters. (as an aside a mention in the docs outside of the example secion to set file type would be useful I think as its a very common usecase). Great tool

@minad
Copy link
Owner

minad commented Mar 5, 2021

I think it does not start a query if you enter # -- -foo first

@rileyrg
Copy link

rileyrg commented Mar 5, 2021

I think it does not start a query if you enter # -- -foo first

It does. I don't think I have anything special in my setup. And it freezes, but I guess that's down to ripgrep?
Bildschirmfoto vom 2021-03-05 10-33-18

@minad
Copy link
Owner

minad commented May 7, 2021

Closing due to inactivity, the feature is noted in the wishlist #6

@minad minad closed this as completed May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants