bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode

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

> Speaking of screen estate, I would like to get full view of the
> candidate, including prefix.

We could add a config var for it, but the default should be to use the
shorter display eliding the shared prefix.

> This helps me make sense out of the candidate particularly when
> partial completion [I meant substring] is on.

I don't understand: when substring completion is used, icomplete already
displays the full name (since there's no shared prefix to elide).

> Implementation wise, I may have taken a different (probably amateurish)
> route.

Comments below.

> +(defcustom icomplete-decorations
> +  '( "{" "}" " | " " | ..." "[" "]" " [No match]" " [Matched%s]")
> +  "List of strings used by icomplete to display alternatives in minibuffer.
> +There are 8 elements in this list:
> +1st and 2nd elements enclose the prospects.
> +3rd element is the separator between prospects.
> +4th element is the string inserted at the end of a truncated list of prospects.
> +5th and 6th elements are used as brackets around the common match string which
> +can be completed using TAB.
> +7th element is the string displayed when there are no matches.
> +8th element is displayed if there is a single match."
> +  :type '(repeat string)
> +  :version "24.3"
> +  :group 'icomplete)

I don't think I want to have every such little bit customizable.
E.g. I haven't heard anyone objecting to [...] and {...}, and the "No
match" and other such messages in the normal completion aren't
customizable either.
So, I'd only provide customization for the separator " | ".

> +(defcustom icomplete-cycle t
> +  "Non-nil if cycling is to be enabled in `icomplete-mode'.
> +When cycling is enabled, keys \"C-j\", \"C-s\" and \"C-r\" are
> +bound to `icomplete-this-match', `icomplete-next-match' and
> +`icomplete-prev-match' respectively."
> +  :type 'boolean
> +  :version "24.3"
> +  :group 'icomplete)

Why didn't you use the patch I provided?  By using a new keymap, users
can easily configure which keys they want to add/disable/...

> @@ -149,7 +178,7 @@
>    "Return strings naming keys bound to FUNC-NAME, or nil if none.
>  Examines the prior, not current, buffer, presuming that current buffer
>  is minibuffer."
> -  (when (commandp func-name)
> +  (when (commandp (intern-soft func-name))
>      (save-excursion
>        (let* ((sym (intern func-name))
>  	     (buf (other-buffer nil t))

Actually, a better patch just removes this test, since the test is
already performed right before calling the function (and the old code
behaved as if the test was never there, since it received a string and
a string is always a valid command).

> +;;;_  = icomplete-name
> +(defvar icomplete-name nil
> +  "Minibuffer user input.")

Why?  It's only set and never used.

> +;;;_  = icomplete-matches
> +(defvar icomplete-matches nil
> +  "Stored value of completion candidates that are on display.
> +This is set by `icomplete-exhibit', modified by
> +`icomplete-this-match', `icomplete-next-match' and
> +`icomplete-prev-match' and cleared by `icomplete-try'.")

Why not use completion-all-sorted-completions?

> +;;;_  = icomplete-most-try
> +(defvar icomplete-most-try nil
> +  "Value of `completion-try-completion'.
> +When there are multiple matches, it signifies common match string
> +which can be completed using TAB.")
> +
> +;;;_  = icomplete-try
> +(defvar icomplete-try nil
> +  "Part of `icomplete-most-try' that is displayed at the prompt.
> +Same as `icomplete-most-try' but with whole of `icomplete-name'
> +stripped from front, when possible.")

Why?  Recomputing them from completion-all-sorted-completions has never
been a performance problem.

> +      (local-unset-key "\C-j")
> +      (local-unset-key "\C-s")
> +      (local-unset-key "\C-r"))

You've just removed the C-j, C-s, and C-r bindings that the user has
painstakingly installed in his minibuffer-local-completion-map.

> +      (local-set-key "\C-j" 'icomplete-this-match)
> +      (local-set-key "\C-s" 'icomplete-next-match)
> +      (local-set-key "\C-r" 'icomplete-prev-match))

And you make it impossible for the user to choose other keybindings for
icomplete's commands.  BTW, if you use completion-all-sorted-completions
rather than a new icomplete-specific variable, then icomplete-this-match
can be added to minibuffer.el (named minibuffer-force-complete-and-exit)
where it can be useful even for people who don't use icomplete.

> +(defun icomplete-this-match ()
> +  "Input first of the displayed matches to minibuffer prompt.
> +See `icomplete-matches'."
> +  (interactive)
> +  (delete-region (minibuffer-prompt-end) (point))
> +  (when icomplete-matches
> +    (insert (car icomplete-matches)))
> +  (exit-minibuffer))

I think it should still call test-completion and obey
minibuffer-completion-confirm if that test fails.


        Stefan