Re: [PATCH] icomplete-vertical

[Stefan Monnier <monnier@iro.umontreal.ca>]

> 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