Provide better api for configuration if using completing-read

[minad]

I would like to set the no-move-default-candidate option for the next completing-read call (this would be helpful for consult-line for example). The goal is that I don't have to use the selectrum api, but could still enhance it while degrading gracefully if not using selectrum. I don't see a possibility to do this using the completing-read api. It is important that the setting is not set in the whole block, since that would apply to all recursive invocations. Therefore the selectrum-should-sort-p api should be changed too I guess.

Could something like this work?

(setq selectrum-options '(:sort nil :move-default-candidate nil))
(completing-read ...) ;; this will read and reset selectrum-options immediately if selectrum is used

See also issue #242 by @clemera

Sorry for flooding you with issues!

[minad]

I could hack something like this myself by advising the selectrum api, but I would prefer if I don't have to!

See https://github.com/minad/consult/blob/8453761de8f4048be941f88a164f0dd6c71880f5/consult.el#L193

(defmacro consult--selectrum-no-sort (body)
  `(minibuffer-with-setup-hook
       (lambda () (setq-local selectrum-should-sort-p nil))
     ,body))

(defmacro consult--selectrum-no-move-default (body)
  (let ((advice (make-symbol "advice")))
    `(if (bound-and-true-p selectrum-mode)
         (letrec ((,advice (lambda (args)
                            (advice-remove #'selectrum-read ,advice)
                            (append args '(:no-move-default-candidate t)))))
           (advice-add #'selectrum-read :filter-args ,advice)
           (unwind-protect
               ,body
             (advice-remove #'selectrum-read ,advice)))
       ,body)))

[clemera]

What about providing a wrapper to make calling completing-read with certain selectrum options easier?

(cl-defun selectrum-cr (prompt candidates 
                               &key predicate require-match initial-input
                               hist def inherit-input-method
                               sort no-move-default-candidate)
  (minibuffer-with-setup-hook
      (lambda ()
        (setq-local selectrum-should-sort-p sort)
        (setq-local selectrum--move-default-candidate-p
                    (not no-move-default-candidate)))
    (completing-read prompt candidates
                     predicate require-match initial-input
                     hist def inherit-input-method)))

[minad]

Oh there is selectrum--move-default-candidate-p :) I should read the selectrum code at some point, currently doing the blackbox thing...

I agree that the proposed code is ok! I think this resolves this for me. This idiom should be documented prominently somewhere. Since this is how selectrum should actually be invoked I guess instead of selectrum-read - with graceful fallback to any other completion system!

[minad]

But in your example above, should-sort-p should actually not be used there. Instead the completion metadata should be used as in my consult--read wrapper.

[minad]

Could we at least make selectrum--move-default-candidate-p public api?

EDIT: @clemera I am not yet fully convinced. Things are getting mega ugly. If you look at my consult--read function, there is already the mess needed for preview. Then another minibuffer-with-setup-hook block, it is getting out of the hands. Maybe we could think a bit more about this...

[clemera]

Since this is how selectrum should actually be invoked I guess instead of selectrum-read - with graceful fallback to any other completion system!

Maybe we could even use something like this as selectrum-read and make the current selectrum-read private. I really like the idea, what do you think @raxod502?

But in your example above, should-sort-p should actually not be used there. Instead the completion metadata should be used as in my consult--read wrapper.

Yes, whenever the official API supports settings we should use it, my bad.

Could we at least make selectrum--move-default-candidate-p public api?

If we decide something like the proposed wrapper this is the way to go I think.

EDIT: @clemera I am not yet fully convinced. Things are getting mega ugly. If you look at my consult--read function, there is already the mess needed for preview. Then another minibuffer-with-setup-hook block, it is getting out of the hands. Maybe we could think a bit more about this...

I think the mess is the reason we provide wrappers so users don't have to deal with it so maybe it's okay? Let's wait a bit if we can think of a better approach and also until @raxod502 has the chance to react.

[minad]

Maybe we could even use something like this as selectrum-read and make the current selectrum-read private. I really like the idea, what do you think @raxod502?

Making selectrum-read private would be the greatest if we can make it work. But only if I can get a working consult-buffer implementation without selectrum-read 😆

If we decide something like the proposed wrapper this is the way to go I think.

There is one thing however. The wrapper cannot be part of the selectrum package, in order to not incur a hard dependency. The wrapper must be simple enough for the users to write themselves :)

Take a look at my consult--configure-minibuffer macro which I am using for now https://github.com/minad/consult/blob/da4ca11855edb95ee91a6a842f5b19f753e3b0e9/consult.el#L196.

I think the mess is the reason we provide wrappers so users don't have to deal with it so maybe it's okay? Let's wait a bit if we can think of a better approach and also until @raxod502 has the chance to react.

I am not expecting anything to be done here in a hurry. But maybe we can profit of the experiences to improve the selectrum api! I would very much like to hear what @raxod502 has to say about the api.

[clemera]

There is one thing however. The wrapper cannot be part of the selectrum package, in order to not incur a hard dependency. The wrapper must be simple enough for the users to write themselves :)

I don't think we can/need to avoid introducing a hard dependency here. You have to install selectrum but the benefit would be you don't have to use it for commands written with the wrapper, those would work fine with any completing-read compatible framework.

[minad]

I don't think we can/need to avoid introducing a hard dependency here. You have to install selectrum but the benefit would be you don't have to use it for commands written with the wrapper, those would work fine with any completing-read compatible framework.

Why not? As of now I don't have a hard dependency in consult and I would like it that way. I rather sacrifice nice API in order to avoid the dependency. So what I am looking for - the best possible API which works with completing-read only and which works without the hard dependency. My consult--preview and consult--configure-minibuffer functions do that for me right now.
I am not particularily fond of the minibuffer-setup hook dance just to set some variables. This feels too heavy.

Therefore my proposal was to provide a single selectrum option variable (a plist) which the user can set to add options for the next selectrum-read call. Even if the selectrum--read call is not even happen publicly.

(setq selectrum-options '(:sort nil :move-default-candidate nil))

Then this would work nicely without incurring a hard dependency. The user would only have to defvar this one variable.

[minad]

Alternatively, maybe this is a better idea! What about adding a 'selectrum-metadata command which is queried from the candidates function? This would feel better. But I am not sure if the completing-read api allows for such extensions. Probably not...

[clemera]

Therefore my proposal was to provide a single selectrum option variable (a plist) which the user can set to add options for the next selectrum-read call. Even if the selectrum--read call is not even happen publicly.

If we reset the options on each call that could work, I did not think of that. So it would be a one off, you can let bind this to some value and the options only last for the very next call. That sounds nice!

[minad]

Let's see what @raxod502 says about that. I think something like this could replace selectrum-read completely for me and we could indeed make it private which would be kind of bold. Note that consult-buffer still won't work then and this is kind of a show-stopper for me. I think in that case I could still use selectrum--read. Regarding making api private, selectrum also makes another parts of its api public (selectrum-read-file etc and the reason is not entirely clear to me).

But let's keep the issues separate. Making api private is orthogonal to allowing small selectrum configuration tweaks together with completing-read, which what I would like to achieve here.

[minad]

Closed in favor of #244, where the discussion continued.