POC/Draft: add selectrum-options in order to allow overwriting options #244
Conversation
(setq selectrum-options '(:no-move-default-candidate t))
(completing-read "Test: " '("one" "two" "three") nil nil nil nil "two")
|
I think it would be nice if we can avoid using |
|
@clemera Yes, I thought about that too. The POC here is the simplest thing which achieves what I want right now. In order to allow more tuning, I suggest adding more options to selectrum-read and allow overwriting specific variables in that way. This would automatically work with this proposal. Furthermore the goal of this proposal is to make it easy to do minor tweaking with graceful degradation and therefore it is good that it is restricted. If I a would want to do more intrusive modifications there is no point in having something like this. Then it would be better to use selectrum-read directly. This is a compromise. I guess your proposal would also work somehow but I find it both too general (allows arbitrary variables (?) and is in not tied to the api offered by selectrum) and at the same time not general enough. If one wants the general thing, I would rather add some kind of one-shot initialization hook. But then I think it is better if things are restricted. |
Okay, but we need to think carefully because once we introduce this it's hard to make changes without breaking user code.
That also means we would need to add more and more keywords over time and we will get issues where people will ask to include this or that as a keyword so it can be used to configure some variable for a single selectrum session. If we do this I think we should add every variable as a keyword upfront but in this case why not use a different approach? Ideally
We can restrict it to only allow selectrum variables so this would be still tied to the API we offer. I would prefer this approach because of the reasons above, I think it will require less maintenance and you get completion...you really want completion don't you? ;) |
Agree. Therefore I am aiming for a more minimal thing. Alternatively, we could just decide to do nothing. There are arguments against doing this since it somehow works around the existing api. I can use advices as I am doing right now and this could be done similarly for every supported completion system. However since selectrum in particular encourages using completing-read, I would like to see something better. But it is a pragmatic compromise, that's it.
I am not sure. You want something different than I want I guess. I am perfectly happy with having things restricted to selectrum-read options, there would not be additional documentation effort, no additional testing effort hopefully. Since it is restricted, fewer things can break. However I understand that one could be tempted to reach for the more powerful thing. But then you can always use the API directly and you can always advice functions. For more complex modifications all the machinery is already there and then it also does not hurt as much if you have to do more work.
Sure, this is possible, I thought about that too. What do you mean - you get completion? Like completion at point? I think this is a rather minor advantage, I cannot have all the nice things it seems ;) Note that there is already some inconsistency between what can be overwritten by variables and what an be overwritten by properties. What I would like is that the API makes a distinction here - there are public variables which should be used for more global configuration, e.g., overwriting the highlight function etc. And then there are the local options, which should be set per selectrum-read call. These are the once I am interested in here. For example, I would like to overwrite initial-input and no-move-default-candidate. I think given your proposal I could only overwrite the move-default variable ant that would not help me. According to this distinction, selectrum-should-sort-p should go away and should be replaced by :sort or :no-sort (Obviously if completing-read is used, it is preferable to use a candidate function to configure the sorting). |
|
@clemera I am experimenting with something else now (minad/consult@0d2c229), which also works nicely, is more local since it does not need an extra variable. I am happy with that. Doing nothing here is an acceptable option. (defun consult--selectrum-config (options)
"Add OPTIONS to the next `selectrum-read' call."
(when (and options (bound-and-true-p selectrum-mode))
(letrec ((advice (lambda (orig prompt candidates &rest args)
(advice-remove #'selectrum-read advice)
(apply orig prompt candidates (append options args)))))
(advice-add #'selectrum-read :around advice))))So to conclude - to do the kind of overwriting you have in mind (or other more complicated modifications), |
|
Should I close this? |
This is also true when we allow to set variables. The only difference is we allow to set them for a single call but they are still the same options which are already documented/tested as the global ones.
Yes, that was what I meant.
I think the case of
Sure, if you are not interested going forward with this. But I still would like to hear @raxod502 opinion as I like the idea of introducing something like |
Yes it is not, but I don't want to tackle all the issues at once. This should also go with an upstream proposal to un-deprecate the option as we discussed before ;)
Fine with me, I would also hear @raxod502's opinion! But the status quo is also okay for me - depending on what is available, I will adapt consult. Instead of adding anything to selectrum, we could just document the two idioms somewhere. |
The file annotations where only being displayed for files in the current directory. This was because the candidates in file completion are not the full path, but rather just the last path component. The function file-attributes wants either a full path or a path relative to the current directory. To fix this bug I added a general function to calculate the "full candidate", i.e, what the minibuffer contents would be if a given candidate is chosen. For most types of completion, the completion candidates are already full candidates. The main exception is file name completion, where the candidates are one path component of the full candidate, which is a full path. (There are other exceptions, such as environment variable name completion inside file name completion.)
|
I retract this. I prefer if selectrum would use completing-read metadata for configuration. Furthermore @clemera is working on the variable based configuration system which is different from this proposal. |
|
Hey y'all! Sorry for the wait. These proposals are very interesting and well discussed. I am excited to see improvements in the area of configuring Selectrum sessions, because I think the current way of doing it is rather poor. I think the most important guiding principle is to develop some way to configure Selectrum such that:
Now, from what I see in the discussion in this thread and #243 (trying to catch up 🙂), another question to ask is how configuration for Selectrum should be encoded. There are a few separate questions here:
I also see some discussion about whether we could deprecate Now, let me give my comments. deprecating selectrum-readI originally wrote the API of I think in order to implement this, we'd re-route all of the Selectrum functions that currently call into new configuration interfaceHere's one way that we might implement the configuration interface. It is only a starting point and I know you two have spent much more time thinking about the tradeoffs than I have, so please let me know what considerations I've missed. I think the proof will be in trying to rewrite existing uses of We could start by introducing a canonical list of possible alist keys, all optional, corresponding to things that are configurable about Selectrum. The default values in the cases that keys are unset might be configurable by some user options, if we want. Then we could introduce a completing-read metadataOne notable point with this idea is that configuration is not passed as We can also have multiple entry points to this configuration system, once the core data model is established. For example, we could support in arbitrary minibuffer setupI think I've seen at least one or two people asking to be able to run arbitrary code on minibuffer setup with Selectrum. We can accommodate that if one of the configuration keys in the alist is an optional function to execute at this point, and I think it would be pretty straightforward to support. An advantage with doing things this way is that it might allow us to get (5) essentially for free without introducing any black magic into Selectrum itself. For example, someone could configure the setup function key in I think we would want to some other thoughtsWhy not make the keys in the alist the same names as the user options for Selectrum? Well, I had that written originally, but then I realized that people were probably going to think that they should let-bind the user options, and that won't work the way they want. An easy way to prevent people from using the user options the wrong way is to make them not user options anymore. It's a little less friendly, but we'd get rid of all these recursive minibuffer problems once and for all. closingSo, yeah! This ended up being longer than I expected. But this is what came to mind (and went through a few revisions and changes of mind before posting) as I read the discussion. Let me know what you think! I suspect there will be many details to work out, and I probably missed some important points that may mean we'll need to go in a different direction from what I suggested. |
|
|
Thanks for the nice summary @raxod502!
I agree that this would be a nice!
I think of this by distinguishing between the following three cases:
I think 2 should have higher precedence than 3, the author makes a good guess of what makes sense for the session but the caller might disagree or wants to have additional settings. I think in the optimal scenario the caller would be able to have a way to overrule the choices of the author one by one. I would like the simplicity that Selectrum settings could be With this considerations my recommendation would be to introduce a |
|
Yeah, those are good thoughts. Your analysis of the limitations of the As long as the multiple ways of doing configuration can be mapped to each other in some way, i.e. the kind of data that goes into each one isn't totally different. |
|
Back when I was using ivy I often had the annoying problem that it wasn't possible to easily reuse configurations of existing code. I really like thal with the approach we are currently discussing this would become very easy. Maybe we could propose such a macro for Emacs itself? Setting variables locally in setup hook is something very common so maybe the Emacs devs would be okay with adding such a macro. |
|
@clemera Sounds like a good idea in general - but I think the first step should be to have a working and sufficiently general version in selectrum. After that, the proposal makes sense! |
|
Yes we can use it internally first and then when we think it is ready and general enough we can propose it. |
Remove many consult-preview-* customizable variables.
This simple patch would solve most configuration problems if one wants to use the
completing-readAPI exclusively but still wants the option to tweak some of the selectrum options, which will only be effective ifselectrum-modeis enabled. Degradation to other completion-systems will be graceful.See #243 for the discussion
@raxod502 @clemera