Re: [PATCH] icomplete-vertical

[Ergus]

On Tue, Apr 06, 2021 at 09:20:53AM -0400, Stefan Monnier wrote:
> > As I said, it's meant to be a derived minor mode, much like lisp-data-mode
> > is derived from prog-mode.
>
> FWIW, we don't have a notion of "derived minor-mode" yet.
>
> > A similar example (in the same file) is
> > fido-mode which is derived from icomplete-mode, and (in another file)
> > whitespace-newline-mode which is derived from whitespace-mode.
>
> The example of `fido-mode` is actually the one I would put forward to
> argue that icomplete-vertical should not itself activate
> `icomplete-mode` but should just change the way completion are
> displayed, so it can be combined wither with `fido-mode` or with the
> normal `icomplete-mode`.
>
> > Moreover, I think that in general local changes to code are better than
> > global ones whenever that is feasible.  Using a user option would do the
> > same thing as the minor mode, except that the changes would be scattered
> > through the existing code.
>
> I don't see how.  I'd imagine a code like:
>
>    ;;;###autoload
>    (define-minor-mode icomplete-vertical-mode
>      "Toggle the use of vertical display in `icomplete-mode`.
>
>    As many completion candidates as possible are displayed, depending on
>    the value of `max-mini-window-height', and the way the mini-window is
>    resized depends on `resize-mini-windows'."
>      :global t
>      (remove-hook 'icomplete-minibuffer-setup-hook
>                   #'icomplete-vertical-minibuffer-setup)
>      (remove-hook 'icomplete-completions-filter-hook
>                   #'icomplete-vertical-reformat-completions)
>      (when icomplete-vertical-mode
>        (setq icomplete-separator "\n")
>        (setq icomplete-hide-common-prefix nil)
>        ;; ask `icomplete-completions' to return enough completions candidates
>        (setq icomplete-prospects-height 25)
>        (add-hook 'icomplete-minibuffer-setup-hook
>                  #'icomplete-vertical-minibuffer-setup)
>        (add-hook 'icomplete-completions-filter-hook
>                  #'icomplete-vertical-reformat-completions)))
>
> [ BTW, in the above code (which I basically copy/pasted from your
>  patch), we should save&restore the values of `icomplete-separator`,
>  `icomplete-hide-common-prefix`, and `icomplete-prospects-height`.
>  Also, I'd recommend to use "--" in the names of the new hook
>  functions.  And while I'm nitpicking I might as well mention that
>  comments should be capitalized and punctuated.  ]
>
>
>        Stefan

Just to remind. Some months ago I actually asked here about the
possibility/complexity to add a parameter to the define-minor-mode
function so we could do something like:

(icomplete-mode 'vertical)
(icomplete-mode 'horizontal)
(icomplete-mode 'fido)
(icomplete-mode nil)

And so on... in order to set the proper hooks and initializations

The use case I had in mind was exactly that. Because I was working on
icomplete-vertical, icomplete-tabular, and some others that in general
just require minimal differences between each other.

I don't actually like the idea of adding a new mode for every different
implementation and would prefer an option; but I understand why Gregory
needed a new mode to do this (as well as fido-mode did.)