Re: [PATCH] icomplete-vertical

[Ergus <spacibba@aol.com>]

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.)