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

Alternative crm idea #160

Closed
minad opened this issue Jul 4, 2021 · 53 comments
Closed

Alternative crm idea #160

minad opened this issue Jul 4, 2021 · 53 comments
Labels
enhancement New feature or request

Comments

@minad
Copy link
Contributor

minad commented Jul 4, 2021

I thought maybe it is better to give up on crm and use something else based directly on completing-read. Try this:

(defun simple-crm (prompt candidates)
  (let ((selected-list)
        (selected-hash (make-hash-table :test #'equal))
        (items candidates))
    (while
        (let ((item
               (completing-read
                (format "%s (%s/%s): " prompt
                        (hash-table-count selected-hash)
                        (length candidates))
                (lambda (str pred action)
                  (if (eq action 'metadata)
                      `(metadata (display-sort-function . ,#'identity)
                                 (cycle-sort-function . ,#'identity)
                                 (group-function
                                  . ,(lambda (cand transform)
                                       (cond
                                        (transform cand)
                                        ((get-text-property 0 'simple-crm-selected cand) "Selected")
                                        (t "Not selected")))))
                    (complete-with-action action items str pred)))
                nil
                t
                nil
                nil
                "")))
          (unless (equal item "")
            (cond
             ((gethash item selected-hash)
              (remhash item selected-hash)
              (setq selected-list (delete item selected-list)))
             (t
              (puthash item t selected-hash)
              (setq selected-list (nconc selected-list
                                         (list (propertize item 'face 'region
                                                           'simple-crm-selected t))))))
            (setq items
                  (append selected-list
                          (seq-remove (lambda (x)
                                        (gethash x selected-hash))
                                      candidates)))
            t)))
    selected-list))

(simple-crm "Select multiple" '("first" "second" "third" "fourth"))

Fortunately there is also no need to implement a custom completion table. I wonder if such a functionality deserves to be provided by some package.

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

Instead of writing a separate simple-crm function here, one could also replicate the completing-read-multiple API and replace this using an advice. The question is then where to package this crm replacement. It could go to Consult since it is completion UI agnostic. What do you think?

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

Yes; this is basically @clemera's original idea!!

Is there a way, or could you extend it, to then deselect?

To be clear, for my case, I don't imagine deselection would be a common need, but say I make a mistake, and need to remove a candidate.

A similar case could be the cc field for an email.

It could go to Consult since it is completion UI agnostic. What do you think?

+100.

With this approach, then I could also rely on consult--read, and so get other benefits, like narrowing, as an alternative to the current "initial-input" approach? That would then also solve:

minad/vertico#59
#144

... plus the fact there is no consult--read-multiple.

Right?

In any case, if you do put together a branch/PR for Consult for this, I'm happy to continue the discussion to figure out the details!

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

Deselect already works if you select the item again.

Regarding consult my idea was to provide a function consult-completing-read-multiple which is NOT supposed to be used by packages. It is meant as replacement for completing-read-multiple to be set in the user configuration (advice override).

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021 via email

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

In your package here you would just continue to use crm without any trickery. And then if the user configures consult appropriately, the enhanced version will be used. Doom could be configured to use consult-completing-read-multiple as drop-in replacement if compleseus is used ;)

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

Doom could be configured to use consult-completing-read-multiple as drop-in replacement if compleseus is used ;)

I can't see any way this wouldn't be a preferable interface to CRM, TBH, even for short candidate strings. So that would be my preference.

WDYT @iyefrat?

What @minad is proposing here is basically the ideas we earlier discussed for selectrum, but in this case would work for selectrum, vertico, etc.

radian-software/selectrum#489

Certainly for bibtex-actions, the improvement would be dramatic.

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

I will try to prototype this for consult and then we can see if this works out.

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

Just notes as I'm playing with it; I'll probably edit this if other thoughts occur to me:

  1. I find the select/deselect UI works very well in general
  2. if you only want to select one, though, it seems to require (unless I'm missing something) hitting return twice; not sure if there's a better alternative, but single vs multiple could be something to test.
  3. as I hinted above, would it be feasible to use a Consult feature like narrowing with consult-completing-read-multiple? It's an alternative to the "initial-input" approach we currently use; not sure if it's better, but would be nice to have as an option.

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

  1. You could easily write a command which presses RET twice for you. But this would be a better fit for a user-level config then.
  2. How would that look like exactly? I am missing what you mean by the initial-input approach.

See here for the current consult version: https://github.com/minad/consult/tree/crm

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

3. How would that look like exactly? I am missing what you mean by the initial-input approach

Now, we use "initial-input" to filter the candidate list.

For example, for the command bibtex-actions-open-pdf, we add has:pdf, so that only those candidates with an associated pdf are initially visible.

I actually think it works pretty well, notwithstanding the CRM issue with it.

I think @mohkale had been experimenting with narrowing to do the same thing. That's what motivated that question, and also why he was earlier asking about CRM support for Consult.

See here for the current consult version: https://github.com/minad/consult/tree/crm.

👍

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

An initial test with bibtex-actions is really encouraging @minad!

I see the TODO notes in the code. When and how do you want further feedback?

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

You can give feedback right away. But I am still not sure about this. There are multiple issues.

  1. There is the issue with how to handle sorting properly. I would like to keep the sorting intact (by recency etc). But I also want to make sure that the "Selected" items appear at the top. These requirements are in conflict and it is hard to make this robust.
  2. The UI may flicker since I am restarting completing-read. To solve this I would have to implement crm as part of Vertico/Selectrum. This would also solve the sorting issue. But then we lose the nice UI independence of the current implementation.
  3. The initial-input filtering you are doing won't work anymore. Or at least it will be very brittle to distinguish between initial candidate input and initial filtering input (See the corresponding TODO). There is no robust solution at sight for this one, since the completing-read-multiple API is too restrictive. As a heuristic, I could treat initial-input strings (separated by the crm-separator) which are members of the candidate list as preselected and the last string if not a member of the list as filter initial input. Then I could pass the filter as initial-input to every completing-read call in the while loop. However this may once again be undesired, since the user of completing-read-multiple may only intend to restrict the next selection and not every subsequent selection (as you are intending with has:pdf). There is a reason why initial-input is strongly discouraged by the documentation of completing-read ;)
  4. Multi selections of the same candidate are not possible. But this is actually solvable, by giving the selected candidates a prefix.

But overall I think consult-completing-read-multiple is on the quality level of consult-completion-in-region. It won't be a perfect solution and it may not work in every scenario one can envision. However it will work and offer an improved experience in most cases (describe-face, your bibtex actions, etc.). It is a best effort solution.

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

  1. How do you handle sorting within groups generally? Is this case any different, other than we want "selected" at the top? I guess I don't see the problem ATM.
  2. I haven't noticed any flickering yet.
  3. Maybe there's a better way for us to achieve the same thing than initial-input? It did have the advantage that it a) reduced the candidate list to only the relevant ones, but b) one could delete it, to get the full list. But otherwise, I'm open to alternatives.
  4. This case isn't important for me.

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

  1. Yes, we want selected at the top. But the UI may decide to sort the candidates differently. I can of course disable sorting completely, by setting display/cycle-sort-function=identity, but then we also lose the sorting by recency. Therefore I am currently adusting the history, such that the selected items appear always at the top. This works well with Vertico but it may not work with other UIs, which sort differently (not by recency for example).
  2. This depends on the number of candidates, but it is very well possible that we get flickering since we close and reopen the minibuffer for each selection. With Vertico/Selectrum it will be okay since these UIs are fast, but you can try a slower UI like Icomplete. But this is probably not a strong enough reason to give up on the approach I am trying here.
  3. I think you should prefilter the candidates before passing them to completing-read-multiple. The only downside of that is that you cannot access the unfiltered candidate list again by deleting the has:pdf filter string. But still, the result will be more robust and portable across UIs.
  4. Agree. I am not aware of a scenario where selecting the same candidate multiple times is desired. I will probably not solve this within consult-completing-read-multiple.

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

I think you should prefilter the candidates before passing them to completing-read-multiple.

OK.

  1. cycling through history is a bit funky from a UI POV, though I don't have any specific suggestions (may be unavoidable)
  2. not sure if this is intended, but the cdr of the selected candidates (the citation keys) appears, but not in the "not selected"

Screenshot from 2021-07-04 17-42-59

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

@ilupin @publicimageltd - you two might be interested in this. Here's what it looks like with bibtex-actions:

Screenshot from 2021-07-04 17-56-51

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

I am playing around with this and I am still not happy with it. Two other issues:

  1. The double RET you mentioned. Ideally TAB would select/deselect the candidate and RET would return with the currently selected candidate or with the empty prompt if the prompt is selected. But this can only be implemented on the level of the UI, which hints at implementing this in Consult is the wrong level and it should rather be added to Vertico.
  2. I am annoyed by the jumpiness of the UI. After selecting a candidate we jump again to the top and the filter string is lost.

@minad
Copy link
Contributor Author

minad commented Jul 4, 2021

I may try a different approach now, taking Embark actions as inspiration. I bind a select command to TAB in the minibuffer for consult-completing-read-multiple. If this command is invoked the current candidate is selected/deselected. This could solve the flicker issue (2) and the jumpiness issue (8) since the current minibuffer session is kept alive. But maybe this will introduce a bunch of other issues.

@bdarcus
Copy link
Contributor

bdarcus commented Jul 4, 2021

Ideally TAB would select/deselect the candidate and RET would return with the currently selected candidate or with the empty prompt if the prompt is selected.

Agree.

After selecting a candidate we jump again to the top and the filter string is lost.

Would it be feasible to also have a modifier (shift-TAB?) to preserve the filter string?

See:

radian-software/selectrum#395 (comment)

radian-software/selectrum#489 (comment)

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

Can the "not selected" group be unnamed? Should it?

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

On this ...

3. I think you should prefilter the candidates before passing them to completing-read-multiple.

... do you mean by using a predicate; like:

(lambda (x) (string-match-p "has:pdf" x))

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

I experimented with this a bit longer. I tried also to implement a prototype in Vertico directly. But somehow I am not satisfied with the outcome. Either I implement the consult-completing-read-multiple function which has multiple issues (flicker, brittle sorting and double-RET selection) or I implement this in Vertico, where it will be an intrusive change.

completing-read-multiple is not widely used in Emacs, there are maybe 5 call sites in comparison to hundreds for completing-read. Plain old completing-read-multiple with comma-separated candidates with Vertico works sufficiently well (or even better) for the simple use cases (describe-face, short string/tag selection, mail address selection). For these reasons I am putting this back once again.

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

My recommendation for your project is to use your own simple-crm implementation as outlined above. This will give a sufficiently good UI for your particular use case. We can revisit this at some later point in Consult/Vertico and then you could still switch to completing-read-multiple.

(defun simple-crm (prompt candidates history)
  (let ((selected)
        (items candidates)
        (history-items (symbol-value history)))
    (while
        (progn
          (set history (append (mapcar #'substring-no-properties selected) history-items))
          (let ((item
                 (completing-read
                  (format "%s (%s selected): " prompt
                          (length selected))
                  (lambda (str pred action)
                    (if (eq action 'metadata)
                        `(metadata (group-function
                                    . ,(lambda (cand transform)
                                         (cond
                                          (transform cand)
                                          ((get-text-property 0 'simple-crm-selected cand) "Selected")
                                          (t "Not selected")))))
                      (complete-with-action action items str pred)))
                  nil
                  t
                  nil
                  history
                  "")))
            (unless (equal item "")
              (if (member item selected)
                  (setq selected (delete item selected))
                (setq selected (nconc selected
                                      (list (propertize item 'face 'region
                                                        'simple-crm-selected t)))))
              (setq items
                    (append selected
                            (seq-remove (lambda (x)
                                          (member x selected))
                                        candidates)))
              t))))
    selected))

(simple-crm "Select multiple" '("first" "second" "third" "fourth") 'minibuffer-history)

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

Ok, thank you for taking the time to give it a try.

Just to clarify ...

Either I implement the consult-completing-read-multiple function which has multiple issues (flicker, brittle sorting and double-RET selection) or I implement this in Vertico, where it will be an intrusive change.

So you could not get this approach to address those issues.

With a solution specific to vertico or selectrum, you can, but that certainly conflicts with the minimalism of vertico (but selectrum?).

It also requires different code.

If the above is all true, it sounds like even I might not include this per se; maybe put it on the wiki or README initially, and encourage people to experiment with it?

I might also want to look at describe-face, since you say it works fine in CRM. They are different. Describe-face uses annotations or affixation for the wide display, which means that content isn't searchable.

And final big question: doesn't this approach seem more consistent with a vertical UI than the CRM prompt, and ideally, shouldn't it be available in Emacs? maybe, but premature to conclude that

@bdarcus

This comment has been minimized.

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

So you could not get this approach to address those issues.

I cannot get these issues to be fixed reliably as part of Consult, which has to treat the completion UI as abstract. If I solve it inside the UI package itself (Selectrum/Vertico/whatever), then I can of course make it work.

With a solution specific to vertico or selectrum, you can, but that certainly conflicts with the minimalism of vertico (but selectrum?).

No, that's not the only problem. If I would find a satisfactory solution, I would not oppose adding it to Vertico. The intrusiveness/non-minimalism is just one factor. The bigger problem is that I am not actually sure how the crm UI should look like. On the level of Vertico I can do everything, since I control the full UI. This opens a large space of design freedom and I don't know what I want exactly. The prototypes simple-crm are consult-completing-read-multiple are good and simple, but it does not feel like they are the ideal UI either.

If the above is all true, it sounds like even I might not include this per se; maybe put it on the wiki or README initially, and encourage people to experiment with it?

Of course, I could do this. Maybe it is a bit too complex for the wiki? My proposal is that you add your own implementation here based on simple-crm. If the crm issue comes up again we can revisit this on a more general level in Vertico or Consult.

They are different. Describe-face uses annotations or affixation for the wide display, which means that content isn't searchable.

Yes, describe-face and your use case are very different. My point is that in most simple completing-read-multiple scenarios, the status quo is good enough. For example, selecting faces, buffer names, org heading tags, etc. However it will not work for long candidates as in your case. Your scenario is not the common use case of completing-read-multiple. In general completing-read-multiple seems to be underused, despite the file crm.el being present for at least 20 years.

And final big question: doesn't this approach seem more consistent with a vertical UI than the CRM prompt, and ideally, shouldn't it be available in Emacs?

consult-completing-read-multiple is an improvement in some cases but overall I am not so sure. For short strings, separating by comma works very well and works across different UIs, including default completion.

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

To give you a picture of the design possibilities.

  1. This is the UI as presented by consult-completing-read-multiple. The problem
    is that the user has to move below the selected candidates to select further
    candidates.
prompt: input   <- input filters everything
--- Selected ------------------------------
selected1       <- filtered
selected2
...
--- Not selected --------------------------
candidate1      <- filtered
candidate2
...
  1. In order to facilitate the common case of selecting candidates one can
    exchange selected/not-selected.
prompt: input   <- input filters everything
--- Not selected --------------------------
candidate1      <- filtered
candidate2
...
--- Selected ------------------------------
selected1       <- filtered
selected2
...
  1. We could decide to not filter the selected candidates always, which is on the one hand is good since it shows the current selection status, but on the other hand makes the UI less convenient to use, since the user has to move below the selected candidates to select additional candidates.
prompt: input   <- input filters only the candidates
--- Selected ------------------------------
selected1       <- not filtered
selected2
...
--- Not selected --------------------------
candidate1      <- filtered
candidate2
...
  1. The ideal UI probably looks more like this. Unfortunately this is a
    significant deviation from how Vertico works currently. However it is not that far off from case 2) given that the user can move to the end, set vertico-cycle or use vertico-next/previous-group.
selected1       <- the selected candidates can be marked and deselected
selected2       <- filtered/not filtered?
...
prompt: input
candidate1      <- filtered
candidate2
...
  1. Then there are vast other functionality one may want to have. You may want to have the option to select all the currently visible candidates. You may want to have the ability to select all the candidates from the current position, holding shift and then moving down (similar to selecting text) etc. Of course this goes to far, but it is unclear where to make the appropriate cutoff. I think Helm is heavily centered around selecting multiple candidates. I wonder how they are solving the issues. As far as I know, Helm works like this, but I haven't checked recently.
prompt: input
  candidate1    <- filtered
* selected2     <- marked in different color?
  candidate3
* selected4

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

Yes. I wonder if there's a chicken-and-problem, though, where the lack of a good, general, and consistent UI holds back adoption, and the lack of adoption holds back further innovation?

Of course.

From what I can tell, helm has no such distinction between single and multi selection; in any helm package one can simply mark and unmark candidates.

Regarding the Helm model, I forgot to state my criticism. The problem with this model is that it is very inconvenient if you want to unselect candidates if you have many candidates. Either you have to search again for the selected candidates by scrolling through the long list or you have to input the filter string again. For this reason, I dismissed this approach right away, the models I've proposed above (1-4) all separate selected/non-selected items by grouping. Unfortunately this introduces the problem, that the user has to move across these candidates every time.

I think your example 4 is basically an evolution of that?

I think my example 4 would indeed be pretty good. Unfortunately this model would be quite complicated to implement and deviate a lot from how Vertico works now. The minimalism is holding me back on this one. This will introduce another new set of problems/features. As I said one may want to have the select all feature/select by holding down shift. Then one may want to integrate this with Embark, which would require heavy changes there, since Embark fundamentally acts on single candidates. If you want to act on more than one candidate, use embark-collect and act in the collect buffer. I am afraid that by adding such advanced multi selection, the simplicity of the current stack will be lost.

Also note that multi selection will not work for completing-read-based commands, e.g., find-file. If one takes multi-selection for granted one would also want to have it there as in Helm.

The completing-read-multiple functionality seems to have been neglected everywhere, except Helm. So if one really fundamentally wants to have this, Helm may be the better solution. However one should also point out the existence of tools like Ibuffer and Dired. These buffer-based tools allow multi-selection, acting on multiple candidates and many more advanced use cases. For this reason I don't really feel the need to implement perfect multi-selection functionality in completion UIs. I've never really missed the multi selection functionality in completion UIs honestly. If I want to act on multiple candidates I can just act on each candidate individually instead of selecting first and then committing the action on the selected set.

So start from your simple example above, rather than your attempt at a more general one for consult?

Yes, my proposal is to start from #160 (comment).

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

@publicimageltd did raise the possibility of just ditching multiple selection altogether, or least for the commands that don't really need it.

Citation insertion is probably the most important place where a clean and efficient multi-selection UI is important, and it's probably the most important command for authors.

The new almost-ready-to-mainline native "org-cite" module, interestingly, uses CRM for the initial insertion, and then CR for editing existing citations. I may, for example, borrow from that for org.

It has also occurred to me, as you note, that embark-collect offers some general possibilies for acting on multiple candidates, that might be appropriate for some commands.

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

I cannot get these issues to be fixed reliably as part of Consult, which has to treat the completion UI as abstract

I started the linked PR.

Is it possible to fix any of this in simple-crm? Or does your conclusion above apply to this as well?

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

No, these issues are not fixable in simple-crm. But the issues are not severe. It depends on the use case. I think for bibtex-action the benefits outweigh the downsides.

@publicimageltd
Copy link
Contributor

It's fun to follow that conversation. It shows that helm actually has a USP, and it also shows, in my eyes, that multiple selection is actually not such a clear concept. I agree with @minad that multiple selection soon raises the demand for more advanced possibilities, such as selecting with shift hold, so that there is no clear cut between minibuffer selection and a complete selection UI which mimicks dired or whatever.

I like the proposed implentation 4, however. Why not just use the entries above the prompt as sort of FIFO stack? If we select something, we can push it onto the stack; there is some easy way to pop it (say, by moving the cursor up); but no further thrills: no filtering, no nifty selection of candidates. After all, this interface is not meant to deal with so many entries; I think that's one main result from the discussion. It's more of a shortcut.

And thinking about such a way of handling things, why not doing something like "live insert"? The idea is: We select an item, and it gets inserted (and maybe removed from the candidates list). But we stay on the prompt. Now we can either insert another item, or do something to undo the last insertion. Such a behavior could be triggered by using modifier keys (M-RETURN instead of RETURN), don't know. That would not be a CRM interface, but something which would be quite intuitive to use, once we get a good keybinding. And it would solve @bdarcus concrete problem, since that is focussed on the task of inserting a list of citations in a buffer.

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

@publicimageltd

It shows that helm actually has a USP, and it also shows, in my eyes, that multiple selection is actually not such a clear concept.

Yes, nobody doubts that helm has its unique features. Multi-selection is probably the most unique concept. Every other concept is to some extent available elsewhere. But the multi-selection concept is not present in Emacs completion itself, except for completing-read-multiple, which is a poor API and implementation and which is very much underused. Basically you could remove it from Emacs proper and not many packages would be affected. This is very different from completing-read which is ubiquitous. The Consult/Vertico/Embark package set concentrates on this use case. Helm has its own ideas, but as a consequence these ideas don't integrate as smoothly.

I like the proposed implentation 4, however.

Yes, this is probably the best proposal for crm. But it will be a significant effort and in the end it will only raise the demand for more advanced possibilities as you said. Therefore my current stance is to not implement such advanced multi completion stuff and focus on single completion. This focus is not unreasonable given that completing-read callsites occurs a hundred times more often than completing-read-multiple callsites in the Emacs source. Probably ELPA/MELPA will give a similar picture.

And thinking about such a way of handling things, why not doing something like "live insert"?

I must admit I don't understand your proposal. It is not concrete enough. I am arguing on a very concrete implementation level here, I have a very clear idea how all the models I've shown above can be implemented. For example (1) is the simple-crm/consult-completing-read-multiple implementation from above. For (3) I have a Vertico prototype, (5) can be implemented in Vertico directly or on the level of crm/consult-completing-read-multiple. (4) is significantly more effort, it can only be implemented in Vertico and it will be intrusive.

@publicimageltd
Copy link
Contributor

I must admit I don't understand your proposal.

You mean on an implementation level, or as a possible UI?

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

From UI POV, might it basically mean using CR, but having some key binding that keeps the minibuffer open with the search string, but that the candidate gets passed to CR behind the scenes?

So if one has to insert a citation with three keys, rather than calling the command three times, you call it once, but select each key in turn?

Is something like that WYM @publicimageltd?

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

From UI POV, might it basically mean using CR, but having some key binding that keeps the minibuffer open with the search string, but that the candidate gets passed to CR behind the scenes?

This is similar to #160 (comment). One would end up with an UI like (5) I suppose, where the selected candidates are marked? This is not complicated to implement, but I don't find it satisfactory since you don't see the selected candidates at a single spot.

@publicimageltd
Copy link
Contributor

So if one has to insert a citation with three keys, rather than calling the command three times, you call it once, but select each key in turn?

Yes, that was the idea. Basically a while loop around a call to completing-read, with some way to propagate the alternative way of selecting an item. And "selecting" in such a loop means to do the action and then return to the selection interface.

I don't find it satisfactory since you don't see the selected candidates at a single spot.

You do: in the buffer where the candidates have been inserted.

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

You do: in the buffer where the candidates have been inserted.

Right, that works for this case (in buffer insertion), but doesn't generalize; does it?

@publicimageltd
Copy link
Contributor

Actually, I was toying around with a similar idea to implement something like hierarchical list selection. The idea is to somehow have two ways of "selection": one meaning "do action and quit", and the other one "do action and repeat with a modified set of candidates."

@publicimageltd
Copy link
Contributor

You do: in the buffer where the candidates have been inserted.

Right, that works for this case (in buffer insertion), but doesn't generalize; does it?

You are right, it's quite specific in that respect.

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

Ah okay, now I get it. This is totally dependent on bibtex-actions then, just like calling bibtex-insert in a loop with the additional tweak of removing already selected items from the list of candidates. This would probably require quite a few changes to bibtex-actions since you interleave selection and insertion?

@publicimageltd
Copy link
Contributor

publicimageltd commented Jul 5, 2021

This would probably require quite a few changes to bibtex-actions since you interleave selection and insertion?

It depends on how the second type of selection ('do action and repeat') is communicated. I had toyed around with a similar idea a while ago, and looking at the code, it seemed like one needs a global variable which is set before calling minibuffer-exit. Then the loop can use that variable to e.g. modifify the existing candidates list or whatever. What hold me back, however, was the recognition that there is not one, completion frontend agnostic way to do this. Selectrum for example remaps exit-minibuffer to do its own stuff, and local-minibuffer-map is substituted by a mode specific map, etc. So I could not figure out a general way to communicate that alternative (selection type 1 or 2) to the calling program.

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

FWIW, my experiments with the capf were to provide another way to do citation insertion and editing. It doesn't work correctly on my end ATM though. I'll fix it when org-cite is merged.

@publicimageltd
Copy link
Contributor

FWIW, my experiments with the capf were to provide another way to do citation insertion and editing.

And maybe that's the best way to do it. It's more clearly centered on the buffer text and does not demand any new knowledge of alternative keybindings to select an item.

I still like my idea, however. It could be useful for browsing through hierarchical lists, like notes in org-roam, or imenu. I imagine a kind of "folder" like experience of pressing, say, tab on an item, and then a sublist opens up. And pressing shift-tab, and we're back in the old candidate list. If @minad knows a way to solve the problem how to communicate the two styles of selection to the calling loop, I might try that for org-roam. :-)

@bdarcus
Copy link
Contributor

bdarcus commented Jul 5, 2021

FYI, the branch now works, if anyone wants to test it.

Details at #163.

@andersjohansson
Copy link
Contributor

Thanks for working and thinking about this

I think you are right; not sure if you can do things like select-all.
Helm has helm-mark-all, but otherwise, the problem with marked candidates not being visible when changing the query is a problem.

I have been a long-time helm user and have used a similar looping completion (from the initial discussions in helm here and here) but for adding org tags in my orgqda library (where I use org tags as codes for qualitative data analysis, hence a need for quickly inserting relevant tags): https://gitlab.com/andersjohansson/orgqda/-/blob/main/orgqda-helm-tags.el

This actually implements the stack idea by putting the already selected candidates at the end, but mostly I never see them when selected because the list is too long. This is all a bit of a hack, modifies the completion list after each selection, and is probably not terribly efficient (now that I try to migrate from helm to the much faster machinery of newer packages, I am considering reimplementing this, but just realized that CRM is still a mess, thanks for enlightening me in this thread).

Skärmbild från 2021-07-05 22-43-24

I will gladly try out your new branch @bdarcus. I guess a source of inspiration could also be what Zotero (or perhaps some of the commercial bibliography managers) does for inserting citations (https://www.zotero.org/support/libreoffice_writer_plugin_usage). It can also be said to implement the stack idea, but my emacs-tuned fingers have never really liked it.
As I use zotero for managing my references, I actually tried out inserting org-cite references with it. I put that code up here now: https://gitlab.com/andersjohansson/oc-zotero-insert

@minad
Copy link
Contributor Author

minad commented Jul 5, 2021

@andersjohansson This issue here is about something different than the org tags issue. completing-read-multiple works perfectly for short candidate strings (org tags, mail addresses etc) with Vertico or Selectrum.

The problem with the org tags command is that it uses a badly written dynamic completion table. This completion table basically only works with default completion (tab completion and the *Completions* buffer). It won't even work with Icomplete or Vertico which try to support CR to its full extent. If org would use CRM for tags everything would be alright. See https://github.com/minad/vertico#org-set-tags-command for more information.

The problem here is that bibtex-actions has special requirements due to its very long candidate strings. The default crm implementation is insufficient for this use case. Either a replacement for CRM or the looping CR could be a solution.

@andersjohansson
Copy link
Contributor

andersjohansson commented Jul 5, 2021 via email

@bdarcus
Copy link
Contributor

bdarcus commented Jul 6, 2021

Thinking more about option 4. Notwithstanding implementation challenges, that I can't assess ...

I think my example 4 would indeed be pretty good. Unfortunately this model would be quite complicated to implement and deviate a lot from how Vertico works now. The minimalism is holding me back on this one. This will introduce another new set of problems/features. As I said one may want to have the select all feature/select by holding down shift.

I think it's safe to say this would be the case. But that's not a terrible idea; just a question where to draw the line.

Then one may want to integrate this with Embark, which would require heavy changes there, since Embark fundamentally acts on single candidates. If you want to act on more than one candidate, use embark-collect and act in the collect buffer. I am afraid that by adding such advanced multi selection, the simplicity of the current stack will be lost.

Does the embark discussion on #163 address this?

From a UI POV wouldn't it be the case that for CR things could be exactly the same?

Also note that multi selection will not work for completing-read-based commands, e.g., find-file.

This is the chicken-egg issue I earlier mentioned.

Perhaps with this sort of UI, more commands would use CRM?

@minad
Copy link
Contributor Author

minad commented Jul 6, 2021

While 4 may be a nice solution, it is restricted to Vertico. It cannot be implemented generically in consult. It would change vertico too much and affect every part of it. I don't want vertico to become a multi selection system. In contrast simple-crm/consult-crm is a single isolated function (good cost/benefit ratio). But if it isn't good enough then it may not make sense to add it to consult. The embark issues are orthogonal to this whole discussion. For any of the discussed UIs one could enhance embark multi selection support, but I would also prefer the simplest possible solution. For example I would avoid complicating the embark calling convention.

@minad
Copy link
Contributor Author

minad commented Jul 6, 2021

Current consult-completing-read-multiple version: https://github.com/minad/consult/blob/71045e7ac27ce6c88abc0809e48eef5393497d7d/consult.el#L2154-L2250

Does not support Embark yet, still have to figure out how to do this. Besides that, I think it works well enough.

@minad minad closed this as completed Jul 7, 2021
@bdarcus
Copy link
Contributor

bdarcus commented Jul 9, 2021

The problem with the org tags command is that it uses a badly written dynamic completion table. This completion table basically only works with default completion (tab completion and the *Completions* buffer). It won't even work with Icomplete or Vertico which try to support CR to its full extent. If org would use CRM for tags everything would be alright. See https://github.com/minad/vertico#org-set-tags-command for more information.

Just FYI, this was just fixed. Org tags now uses CRM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants