New multi-selection UI #163
Conversation
|
@minad @oantolin - the big, and only significant, issue ATM is Embark doesn't work correctly. The menu is accessible as expected, but whether one or multiple candidates, I get no results. Is this because simple-crm is using CR, while I have existing code like this? (interactive (list (bibtex-actions-read :rebuild-cache current-prefix-arg)))Edit: I'll experiment with moving |
|
Ouch, this is a serious issue - since |
|
Quickly tried it out and really like it! Hope the issues are solvable. |
|
We could solve it by checking manually for the |
|
@bdarcus Given that everyone seems to like the UI I may reconsider adding consult-completing-read-multiple to consult in the generic form with embark support. It has the few issues (flicker, double-RET, potential sorting problems) as we discussed but the cost/benefit ratio is not bad, in particular since it is compatible with different UIs. Let's gather a bit more feedback :) The latest consult version is here: https://github.com/minad/consult/blob/3d28108d841a698bf2ec42e32fa954adbb88a87a/consult.el#L2154-L2226 |
I don't know if this is better, but it's at least different: We could arrange for a target finder that if simple-crm is running returns a target that is a list of selected items, not a string. And we could modify Embark so that if the target is a non-empty list, instead of injection the command gets called with the list as argument, whether or not the action is a command. |
|
Well but this would require the command to take a list as argument. Furthermore I am not happy with the idea of list targets. When acting on the candidates in CRM you still act on single candidates (single string). I would not overcomplicate things here - it is sufficient if we ensure that the single candidate injection works since this allows using simple-crm-based commands as embark actions. |
|
Well, without a special target finder for simple-crm, running embark-act from it will just pick up the single candidate for the current run of completing-read, no? So we still need someway to extract the selected candidates from simple-crm and it might as well be as a list (I guess the other option is as a single string with separators). |
|
Okay, that's right. We would not pick up the selected items, only the current item. If we would want to support this we would need a special target finder. But then I would prefer if the embark action is called once for each item in the list. I only oppose adding another Embark calling convention for handling a list. Adding a special CRM target finder returning a list seems okay conceptually. Alternatively one injects the list separated by crm-separator, this would also be acceptable but would exclude using CR actions from CRM. |
|
Current Does not support Embark yet, still have to figure out how to do this. Besides that, I think it works well enough. |
|
When the consult version gets merged, my recommendation is to not add anything special here. Just use Take a look at the current version in minad/consult#352. Main problem is Embark. This is certainly fixable but may require some Embark detection (gluing code). |
Beyond the big embark issue, I guess one little issue remains: the filtering for pdf, etc. Still the case that initial-input won't work? If yes, do I want to add a predicate for the pre-filtering? |
Initial input is completely treated as candidates. It is not treated as a filter string. Maybe that is not the best behavior. I could tweak it as follows:
However the filter string should probably still be used only once, since this is more aligned with how If you add your own implementation here, you can of course special case this somehow, but I think it would be better if you use a common CRM implementation, since this potentially leads to a more consistent UI. Another option we could discuss is the following:
|
|
I'll defer to you on what's the best general approach for But on this:
Is there some downside to using a predicate here? I have a cache of all the candidates, where some commands need to get different subsets. There is some value in being able to return to the full set, as we can now with initial-input, but that's probably not necessary. |
The predicate will be a bit more expensive since it filters the candidates every time. But the cost will probably be small. The downside of predicate/filter vs your current "has:pdf" approach is that the user cannot remove the filter. In contrast with "has:pdf" the user can delete this filter string in order to see all candidates. Is that important/desired? |
Yes, it is. That's why we used I just don't know how important that flexibility is; it's possible not very. If I don't need to make that choice now, I would probably leave things as is. But the intial-input would need to consistently apply to all candidate selections for it to work as we expected. As you know, it doesn't in CRM. |
|
|
|
So I dropped the initial commit here, and replaced with a test script modified to use It doesn't currently work locally for me, but I think the problem is something other than what I committed. Next I'll update the docs. |
This implements the ideas in #160, as a means to fix various problems with CRM for this package.
The key UI thing to realize is you don't really want to use
TABhere; justRET.Also, deselect works the same as select.
I added a defcustom for this, but I have set it to
there, as something like this UI is always what I had in mind, so if this doesn't work well, I may need to explore other alternatives (shorter candidate strings, avoiding CRM in general, focusing on Embark more for acting on multiple candidates, etc.).I've also added some minimal documentation.
As always, you can test by running the test/run.sh script.
Note: the screenshot is with the
vertico-crmprototype version of this.