Dirty hack to test transient popup with consult-grep

[gexplorer]

Here's a quick and dirty hack to test how a transient popup could work with consult.

I created a tiny popup to change one single argument and I modified the consult--command-args method so it gets the arguments from a variable instead of the minibuffer.

So let's give it a try:

• Execute consult-grep
• Search for something that should work but with an uppercase char: defU
• Hit M-h
• Toggle the -i argument and hit s to update the search arguments
• Add another character to force the update: defUn (I told you it was dirty )
• Rejoice with the case insensitive search

[minad]

This looks nice! It would be great to have this. This will certainly make the command much more user friendly. Instead of using the grep args variable I propose to instead to just update the minibuffer string #... -- args#..., such that the arguments can still be seen.

[gexplorer]

Ok I had to tackle a problem with opening a transient popup from within the minibuffer but it's sorted out and I opened a PR in the transient repo in case they are interested.

I also added the rest of the args I had in my package so you can give them a try. Besides if you edit it by hand the popup will get the values you entered, as long as they are in the long form (for now).

[minad]

Cool, this sounds really good! I will try it. I wonder about how we should proceed here. I think I prefer to have this managed as some kind of external consult-grep-popup integration package, maybe maintained as part of your grep-popup packages. I don't know what your plans are regarding those. Do you plan to create multiple grep-popup packages for ag, grep, ripgrep etc or do you create a unified single grep-popup package which supports multiple grep backends and which has integrations for consult/counsel?

[minad]

I just gave it a test! Regarding the argument handling, is it possible to not delete the existing arguments when showing the transient window and somehow update them on the fly? If that would be possible the updating behavior would be perfect!

In the grep window there are many options which are not relevant for Consult or which even break Consult: The general output control, prefix control and context line control options. I wonder what could be done about that. At this point a general purpose grep-popup is a bit in conflict with a version adapted for Consult. What are your thoughts?

EDIT: And one thing I forgot, there are already some existing options used by consult-grep. These will be overwritten by the options coming later, so there is no need to handle those specifically, I guess? Except maybe that it would be a bit better if the transient popup could already highlight them by default.

[gexplorer]

Yes , the required arguments is a problem with the general purpose approach. The best solution would be a consult specific popup, be it in a separate package or included in here.

About arguments with default values, the specific approach would work better because there would be no need to hack the popup, we could really use the full potential of the library that includes setting values for a session or even saving values across sessions, the same way you can do in magic.

As for replacing the minibuffer when the popup opens/closes, I've been taking a look at the search format #search -- args#filter, and I need some helpers to split and join back the values, I tried to extract it from the code but it's not straight forward and I didn't have time.

Besides there's the problem of editing the args by hand and feeding them to the popup. By default the popup stores the values in long form so it expects the values to be in that precise format so the short form doesn't work out of the box.

[minad]

Yes , the required arguments is a problem with the general purpose approach. The best solution would be a consult specific popup, be it in a separate package or included in here.

Okay, I agree. It is not perfect since from the reusability aspect, but at least the UI could be tailored perfectly to the Consult commands. Furthermore the way the arguments are injected in Consult is different from Counsel etc, such that reuse is not possible?

...even saving values across sessions, the same way you can do in magic.

Since the arguments are part of the input string, they will be stored as part of the minibuffer history!

As for replacing the minibuffer when the popup opens/closes, I've been taking a look at the search format #search -- args#filter, and I need some helpers to split and join back the values, I tried to extract it from the code but it's not straight forward and I didn't have time.

I don't think there are good reusable functions for that except consult--async-split-string and this handles only the splitting of the async/filter part. The argument splitting is done separately. Maybe this could be improved/generalized.

Besides there's the problem of editing the args by hand and feeding them to the popup. By default the popup stores the values in long form so it expects the values to be in that precise format so the short form doesn't work out of the box.

Yes, this is a serious problem. One could recognize both forms and map them back and forth. Maybe only print the short form in the minibuffer by default for conciseness but also recognize the long form. Maybe the mapping is unnecessary if options are mostly edited via the popup.

[gexplorer]

I'm working on a possible solution for these problems and it's easier to show you an example instead of trying to explain. I was planning on doing it this morning but I kind of got carried away building a cardboard house at home (true story).

[minad]

@gexplorer Friendly ping! Is there still interest in this?

[gexplorer]

Sure thing! Sorry I've been offline for some time. It would be great if you could create a helper to split the input so we can edit only the appropriate part with the popup. 🥺

[minad]

I am a bit wary with adding helpers without knowing exactly what you need. There is the consult--async-split-string function

(defun consult--async-split-string (str)
  "Split STR in async input and filtering part.
If the first character is a punctuation character it determines
the separator. Examples: \"/async/filter\", \"#async#filter\"."
  (if (string-match-p "^[[:punct:]]" str)
      (save-match-data
        (let ((q (regexp-quote (substring str 0 1))))
          (string-match (concat "^" q "\\([^" q "]*\\)" q "?") str)
          (cons (match-string 1 str) (match-end 0))))
    (cons str (length str))))

This function could be expanded as follows such that it returns both the async and filter string. Maybe it should also return the prefix character in order to allow you reconstructing the input string. Furthermore there is still the argument splitting of the async string which must be performed separately at --. But this should not be part of this function, since the async-split function is more general and not tied only to commands.

(defun consult--async-split-string (str)
  "Split STR in async input and filtering part.

The function returns a list with three elements: Async string,
filter string and starting position of filter string.
If the first character is a punctuation character it determines
the separator. Examples: \"/async/filter\", \"#async#filter\"."
  (if (string-match-p "^[[:punct:]]" str)
      (save-match-data
        (let ((q (regexp-quote (substring str 0 1))))
          (string-match (concat "^" q "\\([^" q "]*\\)" q "?") str)
          (list (match-string 1 str) (substring str (match-end 0)) (match-end 0))))
    (list str "" (length str))))

My sentiment is that such a helper function should be written outside of Consult. And if it turns out that we can unify the external function with the existing internal function we can still do that. I don't think it is a good approach to over generalize the functions internally, in particular if it is private API and I am not using these functions in that form.

So my recommendation would be to just replicate outside what you need and then we can look afterwards if it makes sense to merge parts of it to Consult.

[minad]

I added the transient menu feature to the wishlist #6. I close this PR since it cannot be merged in this form. Please let me know if you want to discuss more!