unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
@ 2012-10-13 17:33 Jambunathan K
  2012-10-13 22:14 ` Stefan Monnier
                   ` (4 more replies)
  0 siblings, 5 replies; 42+ messages in thread
From: Jambunathan K @ 2012-10-13 17:33 UTC (permalink / raw)
  To: 12638


I have turned on icomplete mode following a suggestion in this message
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11131#20.

Since I do use ido-mode, I am wondering whether the icomplete UI could
be modified to match ido UI.  This is to ensure uniformity of
experience.

A cursory look at ido.el suggests that it is indeed derived from
icomplete.el but has been specialized for handling files and buffers.
So there is much common ground between the two modes.

Some observations and initial thoughts:

1. The icomplete candidates are comma separated but WITHOUT spaces.  It
   makes readability difficult.
   
   So introduce `icomplete-decorations' which can be a copy of
   `ido-decorations' to begin with.  May be the decorations could be
   extracted to some other file (minibuffer.el?) and commonly shared by
   both ido and icomplete.

2. Support for cycling via C-s and C-r, highlighting and selection of
   current head (all much like ido-mode)

3. (May be) substring matching.  (Is there a way to accomplish this
   already, I am not sure.)

I can prepare a patch for (1).

Other items have to wait till my foo in related areas improves.

In GNU Emacs 24.2.50.19 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2012-10-13 on debian-6.05
Bzr revision: 110533 cyd@gnu.org-20121013095159-t6cou0e9o2g2vg8g
Windowing system distributor `The X.Org Foundation', version 11.0.10707000
Important settings:
  value of $LANG: en_IN
  locale-coding-system: iso-latin-1-unix
  default enable-multibyte-characters: t






^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-13 17:33 bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode Jambunathan K
@ 2012-10-13 22:14 ` Stefan Monnier
  2012-10-14  4:18 ` Drew Adams
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 42+ messages in thread
From: Stefan Monnier @ 2012-10-13 22:14 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> 3. (May be) substring matching.  (Is there a way to accomplish this
>    already, I am not sure.)

The matching style is independent from icomplete-mode.  And yes, you can
have substring matching, by using `substring' in `completion-styles'.
It's already the default for C-x b completion (via
completion-category-overrides).


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-13 17:33 bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode Jambunathan K
  2012-10-13 22:14 ` Stefan Monnier
@ 2012-10-14  4:18 ` Drew Adams
  2012-10-23 19:43 ` Stefan Monnier
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 42+ messages in thread
From: Drew Adams @ 2012-10-14  4:18 UTC (permalink / raw)
  To: 'Jambunathan K', 12638

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

> 1. The icomplete candidates are comma separated but WITHOUT 
> spaces.  It makes readability difficult.

http://www.emacswiki.org/emacs-en/download/icomplete%2b.el

http://www.emacswiki.org/emacs/IcompleteMode#IcompletePlus

[-- Attachment #2: throw-icomplete.png --]
[-- Type: image/png, Size: 14744 bytes --]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-13 17:33 bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode Jambunathan K
  2012-10-13 22:14 ` Stefan Monnier
  2012-10-14  4:18 ` Drew Adams
@ 2012-10-23 19:43 ` Stefan Monnier
  2012-10-23 20:08   ` Jambunathan K
  2012-11-29 21:34 ` Stefan Monnier
  2013-11-15  4:42 ` Jambunathan K
  4 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-10-23 19:43 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> 1. The icomplete candidates are comma separated but WITHOUT spaces.  It
>    makes readability difficult.
>    So introduce `icomplete-decorations' which can be a copy of
>    `ido-decorations' to begin with.  May be the decorations could be
>    extracted to some other file (minibuffer.el?) and commonly shared by
>    both ido and icomplete.

The lack of space is on purpose, to save screen real-estate, so it
indeed needs to be customizable.  But I don't have a strong opinion on
what the default value should be.

> 2. Support for cycling via C-s and C-r, highlighting and selection of
>    current head (all much like ido-mode)

Not sure what "highlighting" refers to; if you mean to put the first
element in bold, then yes, that fine.

Selection of current head can be done with minibuffer-force-complete
(not bound to any key by default), tho it doesn't exit.  But it should be
easy to add a minibuffer-force-complete-and-exit.

To get you started the patch below adds a keymap to icomplete.

Cycling would also be useful and should similarly be easy to add (it
just needs to play around with (completion-all-sorted-completions) and
store it back via completion--cache-all-sorted-completions, like
minibuffer-force-complete does).

> I can prepare a patch for (1).

We're in feature freeze, so please wait a few weeks before sending
your patch.


        Stefan


=== modified file 'lisp/icomplete.el'
*** lisp/icomplete.el	2012-06-22 17:37:28 +0000
--- lisp/icomplete.el	2012-10-23 19:30:20 +0000
***************
*** 169,174 ****
--- 169,179 ----
  Icomplete does not operate with any specialized completion tables
  except those on this list.")
  
+ (defvar icomplete-minibuffer-map
+   (let ((map (make-sparse-keymap)))
+     (define-key map [?\M-\t] 'minibuffer-force-complete)
+     map))
+ 
  ;;;_ > icomplete-mode (&optional prefix)
  ;;;###autoload
  (define-minor-mode icomplete-mode
***************
*** 208,213 ****
--- 213,220 ----
  Usually run by inclusion in `minibuffer-setup-hook'."
    (when (and icomplete-mode (icomplete-simple-completing-p))
      (set (make-local-variable 'completion-show-inline-help) nil)
+     (use-local-map (make-composed-keymap icomplete-minibuffer-map
+ 					 (current-local-map)))
      (add-hook 'pre-command-hook
  	      (lambda () (let ((non-essential t))
                        (run-hooks 'icomplete-pre-command-hook)))






^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-23 19:43 ` Stefan Monnier
@ 2012-10-23 20:08   ` Jambunathan K
  2012-10-23 20:17     ` Jambunathan K
  2012-10-24 13:09     ` Stefan Monnier
  0 siblings, 2 replies; 42+ messages in thread
From: Jambunathan K @ 2012-10-23 20:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

[-- Attachment #1: Type: text/plain, Size: 405 bytes --]


I do have a patch that works (which I am attaching).  Not sure what you
will think of it.  You can patch it locally and see how it feels.

Speaking of screen estate, I would like to get full view of the
candidate, including prefix.  This helps me make sense out of the
candidate particularly when partial completion is on.

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bug-12638-firstcut.diff --]
[-- Type: text/x-diff, Size: 27513 bytes --]

=== modified file 'lisp/hi-lock.el'
--- lisp/hi-lock.el	2012-10-07 00:27:31 +0000
+++ lisp/hi-lock.el	2012-10-23 12:44:54 +0000
@@ -135,12 +135,20 @@
 ;; It can have a function value.
 (put 'hi-lock-file-patterns-policy 'risky-local-variable t)
 
+(defcustom hi-lock-auto-select-face nil
+  "Non-nil if highlighting commands should not prompt for face names.
+When non-nil, each hi-lock command will cycle through faces in
+`hi-lock-face-defaults'."
+  :type 'boolean
+  :group 'hi-lock
+  :version "24.3")
+
 (defgroup hi-lock-faces nil
   "Faces for hi-lock."
   :group 'hi-lock
   :group 'faces)
 
-(defface hi-yellow
+(defface hi-lock-1
   '((((min-colors 88) (background dark))
      (:background "yellow1" :foreground "black"))
     (((background dark)) (:background "yellow" :foreground "black"))
@@ -149,13 +157,13 @@
   "Default face for hi-lock mode."
   :group 'hi-lock-faces)
 
-(defface hi-pink
+(defface hi-lock-2
   '((((background dark)) (:background "pink" :foreground "black"))
     (t (:background "pink")))
   "Face for hi-lock mode."
   :group 'hi-lock-faces)
 
-(defface hi-green
+(defface hi-lock-3
   '((((min-colors 88) (background dark))
      (:background "green1" :foreground "black"))
     (((background dark)) (:background "green" :foreground "black"))
@@ -164,40 +172,50 @@
   "Face for hi-lock mode."
   :group 'hi-lock-faces)
 
-(defface hi-blue
+(defface hi-lock-4
   '((((background dark)) (:background "light blue" :foreground "black"))
     (t (:background "light blue")))
   "Face for hi-lock mode."
   :group 'hi-lock-faces)
 
-(defface hi-black-b
+(defface hi-lock-5
   '((t (:weight bold)))
   "Face for hi-lock mode."
   :group 'hi-lock-faces)
 
-(defface hi-blue-b
+(defface hi-lock-6
   '((((min-colors 88)) (:weight bold :foreground "blue1"))
     (t (:weight bold :foreground "blue")))
   "Face for hi-lock mode."
   :group 'hi-lock-faces)
 
-(defface hi-green-b
+(defface hi-lock-7
   '((((min-colors 88)) (:weight bold :foreground "green1"))
     (t (:weight bold :foreground "green")))
   "Face for hi-lock mode."
   :group 'hi-lock-faces)
 
-(defface hi-red-b
+(defface hi-lock-8
   '((((min-colors 88)) (:weight bold :foreground "red1"))
     (t (:weight bold :foreground "red")))
   "Face for hi-lock mode."
   :group 'hi-lock-faces)
 
-(defface hi-black-hb
+(defface hi-lock-9
   '((t (:weight bold :height 1.67 :inherit variable-pitch)))
   "Face for hi-lock mode."
   :group 'hi-lock-faces)
 
+(define-obsolete-face-alias 'hi-yellow 'hi-lock-1 "24.3")
+(define-obsolete-face-alias 'hi-pink 'hi-lock-2 "24.3")
+(define-obsolete-face-alias 'hi-green 'hi-lock-3 "24.3")
+(define-obsolete-face-alias 'hi-blue 'hi-lock-4 "24.3")
+(define-obsolete-face-alias 'hi-black-b 'hi-lock-5 "24.3")
+(define-obsolete-face-alias 'hi-blue-b 'hi-lock-6 "24.3")
+(define-obsolete-face-alias 'hi-green-b 'hi-lock-7 "24.3")
+(define-obsolete-face-alias 'hi-red-b 'hi-lock-8 "24.3")
+(define-obsolete-face-alias 'hi-black-hb 'hi-lock-9 "24.3")
+
 (defvar hi-lock-file-patterns nil
   "Patterns found in file for hi-lock.  Should not be changed.")
 
@@ -207,12 +225,19 @@
 (define-obsolete-variable-alias 'hi-lock-face-history
                                 'hi-lock-face-defaults "23.1")
 (defvar hi-lock-face-defaults
-  '("hi-yellow" "hi-pink" "hi-green" "hi-blue" "hi-black-b"
-    "hi-blue-b" "hi-red-b" "hi-green-b" "hi-black-hb")
+  '("hi-lock-1" "hi-lock-2" "hi-lock-3" "hi-lock-4" "hi-lock-5"
+    "hi-lock-6" "hi-lock-7" "hi-lock-8" "hi-lock-9")
   "Default faces for hi-lock interactive functions.")
 
-;;(dolist (f hi-lock-face-defaults)
-;;  (unless (facep f) (error "%s not a face" f)))
+(defvar hi-lock-auto-select-face-defaults
+  (let ((l (copy-sequence hi-lock-face-defaults)))
+    (setcdr (last l) l))
+  "Circular list of faces used for interactive highlighting.
+When `hi-lock-auto-select-face' is non-nil, use the face at the
+head of this list for next interactive highlighting.  See also
+`hi-lock-read-face-name'.")
+
+(make-variable-buffer-local 'hi-lock-auto-select-face-defaults)
 
 (define-obsolete-variable-alias 'hi-lock-regexp-history
                                 'regexp-history
@@ -408,9 +433,9 @@
   (interactive
    (list
     (hi-lock-regexp-okay
-     (read-regexp "Regexp to highlight line" (car regexp-history)))
+     (read-regexp "Regexp to highlight line"))
     (hi-lock-read-face-name)))
-  (or (facep face) (setq face 'hi-yellow))
+  (or (facep face) (setq face 'hi-lock-1))
   (unless hi-lock-mode (hi-lock-mode 1))
   (hi-lock-set-pattern
    ;; The \\(?:...\\) grouping construct ensures that a leading ^, +, * or ?
@@ -433,9 +458,9 @@
   (interactive
    (list
     (hi-lock-regexp-okay
-     (read-regexp "Regexp to highlight" (car regexp-history)))
+     (read-regexp "Regexp to highlight"))
     (hi-lock-read-face-name)))
-  (or (facep face) (setq face 'hi-yellow))
+  (or (facep face) (setq face 'hi-lock-1))
   (unless hi-lock-mode (hi-lock-mode 1))
   (hi-lock-set-pattern regexp face))
 
@@ -455,9 +480,9 @@
    (list
     (hi-lock-regexp-okay
      (hi-lock-process-phrase
-      (read-regexp "Phrase to highlight" (car regexp-history))))
+      (read-regexp "Phrase to highlight")))
     (hi-lock-read-face-name)))
-  (or (facep face) (setq face 'hi-yellow))
+  (or (facep face) (setq face 'hi-lock-1))
   (unless hi-lock-mode (hi-lock-mode 1))
   (hi-lock-set-pattern regexp face))
 
@@ -466,10 +491,18 @@
 ;;;###autoload
 (defalias 'unhighlight-regexp 'hi-lock-unface-buffer)
 ;;;###autoload
-(defun hi-lock-unface-buffer (regexp)
+(defun hi-lock-unface-buffer (regexp &optional prefix-arg)
   "Remove highlighting of each match to REGEXP set by hi-lock.
-Interactively, prompt for REGEXP, accepting only regexps
-previously inserted by hi-lock interactive functions."
+Interactively, when PREFIX-ARG is non-nil, unhighlight all
+highlighted text in current buffer.  When PREFIX-ARG is nil,
+prompt for REGEXP.  If the cursor is on a previously highlighted
+text and if the associated regexp can be inferred via simple
+heuristics, offer that regexp as default.  Otherwise, prompt for
+REGEXP with completion and limit the choices to only those
+regexps used previously with hi-lock commands.
+
+If this command is invoked via menu, pop-up a list of currently
+highlighted patterns."
   (interactive
    (if (and (display-popup-menus-p)
 	    (listp last-nonmenu-event)
@@ -497,23 +530,63 @@
 	  ;; To prevent that, we return an empty string, which will
 	  ;; effectively disable the rest of the function.
 	  (throw 'snafu '(""))))
-     (let ((history-list (mapcar (lambda (p) (car p))
-                                 hi-lock-interactive-patterns)))
-       (unless hi-lock-interactive-patterns
-         (error "No highlighting to remove"))
+     ;; Un-highlighting triggered via keyboard action.
+     (unless hi-lock-interactive-patterns
+       (error "No highlighting to remove"))
+     ;; Infer the regexp to un-highlight based on cursor position.
+     (let* (candidate-hi-lock-patterns
+	    (default-regexp
+	      (or
+	       ;; When using overlays, there is no ambiguity on the best
+	       ;; choice of regexp.
+	       (let ((desired-serial (get-char-property
+				      (point) 'hi-lock-overlay-regexp)))
+		 (when desired-serial
+		   (catch 'regexp
+		     (maphash
+		      (lambda (regexp serial)
+			(when (= serial desired-serial)
+			  (throw 'regexp regexp)))
+		      hi-lock-string-serialize-hash))))
+	       ;; With font-locking on, check if the cursor is on an
+	       ;; highlighted text.  Checking for hi-lock face is a
+	       ;; good heuristic.
+	       (and (string-match "\\`hi-lock-" (face-name (face-at-point)))
+		    (let* ((hi-text
+			    (buffer-substring-no-properties
+			     (previous-single-property-change (point) 'face)
+			     (next-single-property-change (point) 'face))))
+		      ;; Compute hi-lock patterns that match the
+		      ;; highlighted text at point.  Use this later in
+		      ;; during completing-read.
+		      (setq candidate-hi-lock-patterns
+			    (delq nil
+				  (mapcar
+				   (lambda (hi-lock-pattern)
+				     (let ((regexp (car hi-lock-pattern)))
+				       (and (string-match regexp hi-text)
+					    hi-lock-pattern)))
+				   hi-lock-interactive-patterns)))
+		      ;; Use regexp from the first matching pattern as
+		      ;; a reasonable default.
+		      (caar candidate-hi-lock-patterns))))))
        (list
-        (completing-read "Regexp to unhighlight: "
-                         hi-lock-interactive-patterns nil t
-                         (car (car hi-lock-interactive-patterns))
-                         (cons 'history-list 1))))))
-  (let ((keyword (assoc regexp hi-lock-interactive-patterns)))
-    (when keyword
-      (font-lock-remove-keywords nil (list keyword))
-      (setq hi-lock-interactive-patterns
-            (delq keyword hi-lock-interactive-patterns))
-      (remove-overlays
-       nil nil 'hi-lock-overlay-regexp (hi-lock-string-serialize regexp))
-      (when font-lock-fontified (font-lock-fontify-buffer)))))
+	(and (not current-prefix-arg)
+	     (completing-read "Regexp to unhighlight: "
+			      (or candidate-hi-lock-patterns
+				  hi-lock-interactive-patterns)
+			      nil t default-regexp))
+	current-prefix-arg))))
+  (dolist (re (if (not prefix-arg) (list regexp)
+		(mapcar #'car hi-lock-interactive-patterns)))
+    (let ((keyword (assoc re hi-lock-interactive-patterns)))
+      (when keyword
+	(font-lock-remove-keywords nil (list keyword))
+	(setq hi-lock-interactive-patterns
+	      (delq keyword hi-lock-interactive-patterns))
+	(remove-overlays
+	 nil nil 'hi-lock-overlay-regexp (hi-lock-string-serialize re))
+	(when font-lock-fontified (font-lock-fontify-buffer))))))
 
 ;;;###autoload
 (defun hi-lock-write-interactive-patterns ()
@@ -567,25 +640,33 @@
     regexp))
 
 (defun hi-lock-read-face-name ()
-  "Read face name from minibuffer with completion and history."
-  (intern (completing-read
-           "Highlight using face: "
-           obarray 'facep t
-           (cons (car hi-lock-face-defaults)
-                 (let ((prefix
-                        (try-completion
-                         (substring (car hi-lock-face-defaults) 0 1)
-                         hi-lock-face-defaults)))
-                   (if (and (stringp prefix)
-                            (not (equal prefix (car hi-lock-face-defaults))))
-                       (length prefix) 0)))
-           'face-name-history
-	   (cdr hi-lock-face-defaults))))
+  "Return face name for interactive highlighting.
+When `hi-lock-auto-select-face' is non-nil, return head of
+`hi-lock-auto-select-face-defaults'.  Otherwise, read face name
+from minibuffer with completion and history."
+  (if hi-lock-auto-select-face
+      ;; Return current head and rotate the face list.
+      (prog1 (intern (car hi-lock-auto-select-face-defaults))
+	(setq hi-lock-auto-select-face-defaults
+	      (cdr hi-lock-auto-select-face-defaults)))
+    (intern (completing-read
+	     "Highlight using face: "
+	     obarray 'facep t
+	     (cons (car hi-lock-face-defaults)
+		   (let ((prefix
+			  (try-completion
+			   (substring (car hi-lock-face-defaults) 0 1)
+			   hi-lock-face-defaults)))
+		     (if (and (stringp prefix)
+			      (not (equal prefix (car hi-lock-face-defaults))))
+			 (length prefix) 0)))
+	     'face-name-history
+	     (cdr hi-lock-face-defaults)))))
 
 (defun hi-lock-set-pattern (regexp face)
   "Highlight REGEXP with face FACE."
   (let ((pattern (list regexp (list 0 (list 'quote face) t))))
-    (unless (member pattern hi-lock-interactive-patterns)
+    (unless (assoc regexp hi-lock-interactive-patterns)
       (push pattern hi-lock-interactive-patterns)
       (if font-lock-mode
 	  (progn

=== modified file 'lisp/icomplete.el'
--- lisp/icomplete.el	2012-06-22 17:37:28 +0000
+++ lisp/icomplete.el	2012-10-23 19:51:46 +0000
@@ -120,6 +120,35 @@
   :type 'hook
   :group 'icomplete)
 
+(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)
+
+(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)
+
+(defface icomplete-first-match  '((t :weight bold))
+  "Face used by icomplete for highlighting first match."
+  :version "24.3"
+  :group 'icomplete)
+
 
 ;;;_* Initialization
 
@@ -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))
@@ -169,6 +198,29 @@
 Icomplete does not operate with any specialized completion tables
 except those on this list.")
 
+;;;_  = icomplete-name
+(defvar icomplete-name nil
+  "Minibuffer user input.")
+
+;;;_  = 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'.")
+
+;;;_  = 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.")
+
 ;;;_ > icomplete-mode (&optional prefix)
 ;;;###autoload
 (define-minor-mode icomplete-mode
@@ -227,7 +279,18 @@
   "Remove completions display \(if any) prior to new user input.
 Should be run in on the minibuffer `pre-command-hook'.  See `icomplete-mode'
 and `minibuffer-setup-hook'."
-  (delete-overlay icomplete-overlay))
+  (unless (memq this-command '(icomplete-this-match icomplete-next-match
+						    icomplete-prev-match))
+    ;; Current command does not belong to icomplete-mode.
+    ;; Clear the matches.
+    (setq icomplete-matches nil)
+    ;; Cleanup local icomplete bindings.
+    (when (eq (key-binding "\C-j") 'icomplete-this-match)
+      (local-unset-key "\C-j")
+      (local-unset-key "\C-s")
+      (local-unset-key "\C-r"))
+    ;; Delete the overlay.
+    (delete-overlay icomplete-overlay)))
 
 ;;;_ > icomplete-exhibit ()
 (defun icomplete-exhibit ()
@@ -235,6 +298,12 @@
 Should be run via minibuffer `post-command-hook'.  See `icomplete-mode'
 and `minibuffer-setup-hook'."
   (when (and icomplete-mode (icomplete-simple-completing-p))
+    ;; Enable icomplete specific key bindings, if needed.
+    (when (and icomplete-cycle
+	       (not (eq (key-binding "\C-j") 'icomplete-this-match)))
+      (local-set-key "\C-j" 'icomplete-this-match)
+      (local-set-key "\C-s" 'icomplete-next-match)
+      (local-set-key "\C-r" 'icomplete-prev-match))
     (save-excursion
       (goto-char (point-max))
                                         ; Insert the match-status information:
@@ -274,6 +343,9 @@
 The display is updated with each minibuffer keystroke during
 minibuffer completion.
 
+A typical display looks like:
+    M-x loa[d-]{load-library | load-file | load-theme}
+
 Prospective completion suffixes (if any) are displayed, bracketed by
 one of \(), \[], or \{} pairs.  The choice of brackets is as follows:
 
@@ -286,96 +358,134 @@
 \(whether complete or not), or ` \[No matches]', if no eligible
 matches exist.  \(Keybindings for uniquely matched commands
 are exhibited within the square braces.)"
-
-  (let* ((md (completion--field-metadata (field-beginning)))
-	 (comps (completion-all-sorted-completions))
-         (last (if (consp comps) (last comps)))
-         (base-size (cdr last))
-         (open-bracket (if require-match "(" "["))
-         (close-bracket (if require-match ")" "]")))
-    ;; `concat'/`mapconcat' is the slow part.
-    (if (not (consp comps))
-        (format " %sNo matches%s" open-bracket close-bracket)
-      (if last (setcdr last nil))
-      (let* ((most-try
-              (if (and base-size (> base-size 0))
-                  (completion-try-completion
-                   name candidates predicate (length name) md)
-                ;; If the `comps' are 0-based, the result should be
-                ;; the same with `comps'.
-                (completion-try-completion
-                 name comps nil (length name) md)))
-	     (most (if (consp most-try) (car most-try)
-                     (if most-try (car comps) "")))
-             ;; Compare name and most, so we can determine if name is
-             ;; a prefix of most, or something else.
-	     (compare (compare-strings name nil nil
-				       most nil nil completion-ignore-case))
-	     (determ (unless (or (eq t compare) (eq t most-try)
-				 (= (setq compare (1- (abs compare)))
-				    (length most)))
-		       (concat open-bracket
-			       (cond
-				((= compare (length name))
-                                 ;; Typical case: name is a prefix.
-				 (substring most compare))
-				((< compare 5) most)
-				(t (concat "..." (substring most compare))))
-			       close-bracket)))
-	     ;;"-prospects" - more than one candidate
-	     (prospects-len (+ (length determ) 6 ;; take {,...} into account
-                               (string-width (buffer-string))))
-             (prospects-max
-              ;; Max total length to use, including the minibuffer content.
-              (* (+ icomplete-prospects-height
-                    ;; If the minibuffer content already uses up more than
-                    ;; one line, increase the allowable space accordingly.
-                    (/ prospects-len (window-width)))
-                 (window-width)))
-             (prefix-len
-              ;; Find the common prefix among `comps'.
-	      ;; We can't use the optimization below because its assumptions
-	      ;; aren't always true, e.g. when completion-cycling (bug#10850):
-	      ;; (if (eq t (compare-strings (car comps) nil (length most)
-	      ;; 			 most nil nil completion-ignore-case))
-	      ;;     ;; Common case.
-	      ;;     (length most)
-	      ;; Else, use try-completion.
-	      (let ((comps-prefix (try-completion "" comps)))
-		(and (stringp comps-prefix)
-		     (length comps-prefix)))) ;;)
-
-	     prospects most-is-exact comp limit)
-	(if (eq most-try t) ;; (or (null (cdr comps))
-	    (setq prospects nil)
-	  (while (and comps (not limit))
-	    (setq comp
-		  (if prefix-len (substring (car comps) prefix-len) (car comps))
-		  comps (cdr comps))
-	    (cond ((string-equal comp "") (setq most-is-exact t))
-		  ((member comp prospects))
-		  (t (setq prospects-len
-                           (+ (string-width comp) 1 prospects-len))
-		     (if (< prospects-len prospects-max)
-			 (push comp prospects)
-		       (setq limit t))))))
-        ;; Restore the base-size info, since completion-all-sorted-completions
-        ;; is cached.
-        (if last (setcdr last base-size))
-	(if prospects
+  (unless icomplete-matches
+    ;; Re-compute the matches.
+    (let* ((md (completion--field-metadata (field-beginning)))
+	   (comps (completion-all-sorted-completions))
+	   (last (if (consp comps) (last comps)))
+	   (base-size (cdr last)))
+      (when (consp comps)
+	(if last (setcdr last nil))
+	(let* ((most-try
+		(if (and base-size (> base-size 0))
+		    (completion-try-completion
+		     name candidates predicate (length name) md)
+		  ;; If the `comps' are 0-based, the result should be
+		  ;; the same with `comps'.
+		  (completion-try-completion
+		   name comps nil (length name) md)))
+	       (most (if (consp most-try) (car most-try)
+		       (if most-try name ""))))
+	  ;; Cache results for use with `icomplete-this-match',
+	  ;; `icomplete-next-match' and `icomplete-prev-match'.
+	  (setq icomplete-name name)
+	  (setq icomplete-matches (nconc (butlast comps) (list (car last))))
+	  ;; If prefix is itself an exact match, move it to the front of
+	  ;; list of matches.
+	  (let ((prefix (let ((comps-prefix (try-completion "" comps)))
+			  (or (and (stringp comps-prefix) comps-prefix) ""))))
+	    (when (member prefix icomplete-matches)
+	      (setq icomplete-matches (cons prefix
+					    (delete prefix icomplete-matches)))))
+	  (setq icomplete-most-try most-try)
+	  ;; Compare name and most, so we can determine if name is
+	  ;; a prefix of most, or something else.
+	  (setq icomplete-try
+		(let ((compare (compare-strings name nil nil
+						most nil nil
+						completion-ignore-case)))
+		  (unless (or (eq t compare) (eq t most-try)
+			      (= (setq compare (1- (abs compare)))
+				 (length most)))
+		    (cond
+		     ((= compare (length name))
+		      ;; Typical case: name is a prefix.
+		      (substring most compare))
+		     ((< compare 5) most)
+		     (t (concat "..." (substring most compare)))))))
+	  ;; Restore the base-size info, since
+	  ;; `completion-all-sorted-completions' is cached.
+	  (if last (setcdr last base-size))))))
+  (if (not icomplete-matches)
+      (nth 6 icomplete-decorations)
+    (let* ((determ (and icomplete-try
+			(concat (nth 4 icomplete-decorations)
+				icomplete-try
+				(nth 5 icomplete-decorations)))))
+      (if (not (eq icomplete-most-try t))
+	  (let* ((comps icomplete-matches)
+		 (prospects-max
+		  ;; Max total length to use, including the
+		  ;; minibuffer content.
+		  (* (+ icomplete-prospects-height
+			;; If the minibuffer content already uses up
+			;; more than one line, increase the
+			;; allowable space accordingly.
+			(/ (string-width (buffer-string)) (window-width)))
+		     (window-width)))
+		 (prospects-len (string-width (buffer-string)))
+		 prospects limit first)
+	    (setq prospects-len
+		  (+ prospects-len (string-width (or determ ""))
+		     ;; Account for { | ...}
+		     (string-width (nth 0 icomplete-decorations))
+		     (string-width (nth 3 icomplete-decorations))
+		     (string-width (nth 1 icomplete-decorations))))
+	    ;; Decorate first of the prospects but remember to make a
+	    ;; copy.  This is to ensure correct behaviour when matches
+	    ;; are cycled with C-s or C-r.
+	    (setq first (copy-sequence (pop comps)))
+	    (put-text-property 0 (length first) 'face
+			       'icomplete-first-match first)
+	    (setq prospects-len (+ prospects-len (string-width first)))
+	    (while (and comps (not limit))
+	      (let* ((p (concat (nth 2 icomplete-decorations) (pop comps))))
+		(setq prospects-len (+ (string-width p) prospects-len))
+		(if (< prospects-len prospects-max)
+		    (setq prospects (concat prospects p))
+		  (setq limit t))))
 	    (concat determ
-		    "{"
-		    (and most-is-exact ",")
-		    (mapconcat 'identity (nreverse prospects) ",")
-		    (and limit ",...")
-		    "}")
-	  (concat determ
-		  " [Matched"
-		  (let ((keys (and icomplete-show-key-bindings
-				   (commandp (intern-soft most))
-				   (icomplete-get-keys most))))
-		    (if keys (concat "; " keys) ""))
-		  "]"))))))
+		    (nth 0 icomplete-decorations)
+		    (concat first prospects)
+		    (and limit (nth 3 icomplete-decorations))
+		    (nth 1 icomplete-decorations)))
+	(concat determ
+		(format (nth 7 icomplete-decorations)
+			(let* ((most (if (consp icomplete-most-try)
+					 (car icomplete-most-try)
+				       (if icomplete-most-try name "")))
+			       (keys (and icomplete-show-key-bindings
+					  (icomplete-get-keys most))))
+			  (if keys (concat "; " keys) ""))))))))
+
+(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))
+
+(defun icomplete-next-match ()
+  "Shift displayed matches to the left.
+Second of displayed matches is promoted to first position and can
+be selected with `icomplete-this-match'."
+  (interactive)
+  (let ((first (pop icomplete-matches)))
+    (setq icomplete-matches (nconc icomplete-matches (list first)))))
+
+(defun icomplete-prev-match ()
+  "Shift displayed matches to the right.
+Last of displayed matches (which could be truncated from display)
+is promoted to first position and can be selected with
+`icomplete-this-match'."
+  (interactive)
+  (let* ((last-but-one (last icomplete-matches 2))
+	 (last (cdr last-but-one)))
+    (when last
+      (setcdr last-but-one nil)
+      (push (car last) icomplete-matches))))
 
 ;;_* Local emacs vars.
 ;;Local variables:

=== modified file 'lisp/replace.el'
--- lisp/replace.el	2012-10-16 23:27:40 +0000
+++ lisp/replace.el	2012-10-23 12:44:54 +0000
@@ -585,27 +585,32 @@
 When PROMPT doesn't end with a colon and space, it adds a final \": \".
 If DEFAULTS is non-nil, it displays the first default in the prompt.
 
-Non-nil optional arg DEFAULTS is a string or a list of strings that
-are prepended to a list of standard default values, which include the
-string at point, the last isearch regexp, the last isearch string, and
-the last replacement regexp.
+Optional arg DEFAULTS is a string or a list of strings that are
+prepended to a list of standard default values, which include the
+tag at point, the last isearch regexp, the last isearch string,
+and the last replacement regexp.
 
 Non-nil HISTORY is a symbol to use for the history list.
 If HISTORY is nil, `regexp-history' is used."
-  (let* ((default (if (consp defaults) (car defaults) defaults))
-	 (defaults
+  (let* ((defaults
 	   (append
 	    (if (listp defaults) defaults (list defaults))
-	    (list (regexp-quote
-		   (or (funcall (or find-tag-default-function
-				    (get major-mode 'find-tag-default-function)
-				    'find-tag-default))
-		       ""))
-		  (car regexp-search-ring)
-		  (regexp-quote (or (car search-ring) ""))
-		  (car (symbol-value
-			query-replace-from-history-variable)))))
+	    (list
+	     ;; Regexp for tag at point.
+	     (let* ((tagf (or find-tag-default-function
+			      (get major-mode 'find-tag-default-function)
+			      'find-tag-default))
+		    (tag (funcall tagf)))
+	       (cond ((not tag) "")
+		     ((eq tagf 'find-tag-default)
+		      (format "\\_<%s\\_>" (regexp-quote tag)))
+		     (t (regexp-quote tag))))
+	     (car regexp-search-ring)
+	     (regexp-quote (or (car search-ring) ""))
+	     (car (symbol-value
+		   query-replace-from-history-variable)))))
 	 (defaults (delete-dups (delq nil (delete "" defaults))))
+	 (default (car defaults))
 	 ;; Do not automatically add default to the history for empty input.
 	 (history-add-new-input nil)
 	 (input (read-from-minibuffer


[-- Attachment #3: Type: text/plain, Size: 2561 bytes --]



Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 1. The icomplete candidates are comma separated but WITHOUT spaces.  It
>>    makes readability difficult.
>>    So introduce `icomplete-decorations' which can be a copy of
>>    `ido-decorations' to begin with.  May be the decorations could be
>>    extracted to some other file (minibuffer.el?) and commonly shared by
>>    both ido and icomplete.
>
> The lack of space is on purpose, to save screen real-estate, so it
> indeed needs to be customizable.  But I don't have a strong opinion on
> what the default value should be.
>
>> 2. Support for cycling via C-s and C-r, highlighting and selection of
>>    current head (all much like ido-mode)
>
> Not sure what "highlighting" refers to; if you mean to put the first
> element in bold, then yes, that fine.
>
> Selection of current head can be done with minibuffer-force-complete
> (not bound to any key by default), tho it doesn't exit.  But it should be
> easy to add a minibuffer-force-complete-and-exit.
>
> To get you started the patch below adds a keymap to icomplete.
>
> Cycling would also be useful and should similarly be easy to add (it
> just needs to play around with (completion-all-sorted-completions) and
> store it back via completion--cache-all-sorted-completions, like
> minibuffer-force-complete does).
>
>> I can prepare a patch for (1).
>
> We're in feature freeze, so please wait a few weeks before sending
> your patch.
>
>
>         Stefan
>
>
>
> === modified file 'lisp/icomplete.el'
> *** lisp/icomplete.el	2012-06-22 17:37:28 +0000
> --- lisp/icomplete.el	2012-10-23 19:30:20 +0000
> ***************
> *** 169,174 ****
> --- 169,179 ----
>   Icomplete does not operate with any specialized completion tables
>   except those on this list.")
>   
> + (defvar icomplete-minibuffer-map
> +   (let ((map (make-sparse-keymap)))
> +     (define-key map [?\M-\t] 'minibuffer-force-complete)
> +     map))
> + 
>   ;;;_ > icomplete-mode (&optional prefix)
>   ;;;###autoload
>   (define-minor-mode icomplete-mode
> ***************
> *** 208,213 ****
> --- 213,220 ----
>   Usually run by inclusion in `minibuffer-setup-hook'."
>     (when (and icomplete-mode (icomplete-simple-completing-p))
>       (set (make-local-variable 'completion-show-inline-help) nil)
> +     (use-local-map (make-composed-keymap icomplete-minibuffer-map
> + 					 (current-local-map)))
>       (add-hook 'pre-command-hook
>   	      (lambda () (let ((non-essential t))
>                         (run-hooks 'icomplete-pre-command-hook)))
>
>
>
>
>

-- 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-23 20:08   ` Jambunathan K
@ 2012-10-23 20:17     ` Jambunathan K
  2012-10-24 13:09     ` Stefan Monnier
  1 sibling, 0 replies; 42+ messages in thread
From: Jambunathan K @ 2012-10-23 20:17 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638


> I do have a patch that works (which I am attaching). 

Just take the icomplete.el part.  (Btw, the diff included the fix for
bug11095.  So no harm if it gets reviewed as well.)

> Speaking of screen estate, I would like to get full view of the
> candidate, including prefix.  This helps me make sense out of the
> candidate particularly when partial completion is on.
                              ^^^^^^^^^^^^^^^^^^
I meant substring completion. (I really like to see bodies with their
heads.  This is what ido does as well.)
-- 





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-23 20:08   ` Jambunathan K
  2012-10-23 20:17     ` Jambunathan K
@ 2012-10-24 13:09     ` Stefan Monnier
  2012-11-02 11:49       ` Jambunathan K
  1 sibling, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-10-24 13:09 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> 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





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-24 13:09     ` Stefan Monnier
@ 2012-11-02 11:49       ` Jambunathan K
  2012-11-02 12:12         ` Jambunathan K
  2012-11-09  2:17         ` Stefan Monnier
  0 siblings, 2 replies; 42+ messages in thread
From: Jambunathan K @ 2012-11-02 11:49 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]


I am attaching a second revision of the earlier patch.  

It is not that patch is final.  It is worthy of further discussion and
experimentation.

Changes are in minibuffer.el and icomplete.el.

1. `minibuffer-summarize-completions' is meant as a replacement for
   `icomplete-exhibit'.  As the name suggests, it is meant to go in to
    minibuffer.el.  It's presence in minibuffer.el proved problematic
   (details in followup mail) and I had to move it to icomplete.el.

   TODO: Handle key binding hints for sole command matches?  Is it
   really necessary.  It seems so `old school'.  May be it made sense
   when Emacs /did not/ provide key binding hints.

2. There is a display overlay that is used in minibuffer (see
   `minibuffer-message').  There is a counterpart in icomplete.el named
   `icomplete-overlay'. 

   I am wondering whether `icomplete-overlay' could be thrown away and
   have it use, yet to be introduced `minibuffer-overlay'.  Should this
   be buffer-local etc etc. I am not sure of.

   I can exchange notes if there is some interest around this area.

3. Apropos `minibuffer-force-complete-and-exit' and `confirm' etc.

    >> +(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.

  Can `test-completion' fail if the prompt is filled from valid
  candidates? Remember, `minibuffer-force-and-complete-and-exit' doesn't
  rely on manual-typing at the prompt.


--8<---------------cut here---------------start------------->8---


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bug12638-rev2-20121102.diff --]
[-- Type: text/x-diff, Size: 19936 bytes --]

=== modified file 'lisp/icomplete.el'
--- lisp/icomplete.el	2012-06-22 17:37:28 +0000
+++ lisp/icomplete.el	2012-11-02 11:12:09 +0000
@@ -71,6 +71,9 @@
 (make-obsolete-variable
  'icomplete-prospects-length 'icomplete-prospects-height "23.1")
 
+(defvar icomplete-separator " | "
+  "String used by icomplete to separate alternatives in the minibuffer.")
+
 ;;;_* User Customization variables
 (defcustom icomplete-prospects-height
   ;; 20 is an estimated common size for the prompt + minibuffer content, to
@@ -102,7 +105,14 @@
   :type 'boolean
   :group 'icomplete)
 
-(defcustom icomplete-minibuffer-setup-hook nil
+(defcustom icomplete-minibuffer-setup-hook
+  '((lambda ()
+      (define-key icomplete-minibuffer-map
+	"\C-j" 'minibuffer-force-complete-and-exit)
+      (define-key icomplete-minibuffer-map
+	"\C-s" 'minibuffer-forward-sorted-completions)
+      (define-key icomplete-minibuffer-map
+	"\C-r" 'minibuffer-backward-sorted-completions)))
   "Icomplete-specific customization of minibuffer setup.
 
 This hook is run during minibuffer setup if icomplete is active.
@@ -118,6 +128,21 @@
 will constrain Emacs to a maximum minibuffer height of 3 lines when
 icompletion is occurring."
   :type 'hook
+  :version "24.3"
+  :group 'icomplete)
+
+(defcustom icomplete-hide-common-prefix t
+  "When non-nil, hide common prefix (if any) from completion candidates.
+Default setting (typically) allows more candidates to be shown
+for given `icomplete-prospects-height'.  When nil, show
+candidates in full."
+  :type 'boolean
+  :version "24.3"
+  :group 'icomplete)
+
+(defface icomplete-first-match  '((t :weight bold))
+  "Face used by icomplete for highlighting first match."
+  :version "24.3"
   :group 'icomplete)
 
 
@@ -169,6 +194,11 @@
 Icomplete does not operate with any specialized completion tables
 except those on this list.")
 
+(defvar icomplete-minibuffer-map
+  (let ((map (make-sparse-keymap)))
+    (define-key map [?\M-\t] 'minibuffer-force-complete)
+    map))
+
 ;;;_ > icomplete-mode (&optional prefix)
 ;;;###autoload
 (define-minor-mode icomplete-mode
@@ -207,7 +237,9 @@
   "Run in minibuffer on activation to establish incremental completion.
 Usually run by inclusion in `minibuffer-setup-hook'."
   (when (and icomplete-mode (icomplete-simple-completing-p))
-    (set (make-local-variable 'completion-show-inline-help) nil)
+    ;; (set (make-local-variable 'completion-show-inline-help) nil)
+    (use-local-map (make-composed-keymap icomplete-minibuffer-map
+    					 (current-local-map)))
     (add-hook 'pre-command-hook
 	      (lambda () (let ((non-essential t))
                       (run-hooks 'icomplete-pre-command-hook)))
@@ -227,155 +259,192 @@
   "Remove completions display \(if any) prior to new user input.
 Should be run in on the minibuffer `pre-command-hook'.  See `icomplete-mode'
 and `minibuffer-setup-hook'."
-  (delete-overlay icomplete-overlay))
+  (unless (memq this-command '(minibuffer-force-complete-and-exit minibuffer-forward-sorted-completions
+						    minibuffer-backward-sorted-completions))
+    ;; Current command does not belong to icomplete-mode.
+    ;; Delete the overlay.
+    (delete-overlay icomplete-overlay)))
+
+(defun minibuffer-summarize-completions (&optional separator max-width
+						   first-match-face
+						   strip-common-substring)
+  "Summarize minibuffer completion and return it as a string.
+
+Return one of \"[No match]\", \"[Sole completion]\" or a string
+of the form:
+
+    \"\(TAB-COMPL\){CAND-1 SEPARATOR CAND-2 ... CAND-N SEPARATOR CONTD}\"
+
+CAND-1, CAND-2 etc are completion candidates.
+
+TAB-COMPL, when present, indicates the string to which next
+`minibuffer-complete' will complete to.
+
+CONTD (string \"...\"), when present, indicates that not all
+completions are shown.  To disclose completions that are hidden,
+use `minibuffer-forward-sorted-completions' and
+`minibuffer-backward-sorted-completions'.
+
+SEPARATOR is a string to separate completions.  If nil, use \" | \".
+
+MAX-WIDTH is width in characters, as a multiple of window width,
+to which summary string is capped.  If MAX-WIDTH is not a
+multiple of `\(window-width\)', it is increased suitably.  If it
+is nil (or in any case), it is forcibly increased to just
+accommodate minibuffer contents.
+
+FIRST-MATCH-FACE is a face to be applied to first of the matches,
+which can be input with `minibuffer-force-complete-and-exit'.  If
+nil, use 'bold face.
+
+STRIP-COMMON-SUBSTRING, a boolean, controls whether how
+completion candidates are displayed.  If nil, show completions in
+full.  Otherwise, strip common prefix of all
+completions (obtained with `try-completion') from completions."
+  
+  ;; Set defaults for optional params.
+  (setq separator (or separator " | "))
+  ;; Ensure max-width is big enough to hold current minibuffer
+  ;; content.  Stretch max-width to a multiple of window width.
+  (let* ((content-width (string-width (buffer-string))))
+    (setq max-width (max content-width (or max-width 0)))
+    (setq max-width
+	  (let* ((normalized-height (/ max-width (window-width)))
+		 (normalized-width (* normalized-height (window-width))))
+	    (if (< normalized-height max-width)
+		(+ normalized-width (window-width))
+	      normalized-width))))
+  (setq first-match-face (or first-match-face 'bold))
+  (let* ((beg (field-beginning))
+         (end (field-end))
+         (string (buffer-substring beg end))
+         (md (completion--field-metadata beg))
+         (try-comp (completion-try-completion
+		    string
+		    minibuffer-completion-table
+		    minibuffer-completion-predicate
+		    (- (point) beg)
+		    md)))
+    (cond
+     ;; No match.
+     ((null try-comp)
+      "[No match]")
+     ;; Sole completion.  Exact and unique match.
+     ((eq t try-comp)
+      "[Sole completion]")
+     (t
+      ;; `completed' should be t if some completion was done, which doesn't
+      ;; include simply changing the case of the entered string.  However,
+      ;; for appearance, the string is rewritten if the case changes.
+      (let* ((comp-pos (cdr try-comp))
+             (completion (car try-comp))
+	     (compare (compare-strings completion nil nil
+				       string nil nil
+				       completion-ignore-case))
+             (completed (not (eq t compare)))
+	     (exact (test-completion completion
+	     			     minibuffer-completion-table
+	     			     minibuffer-completion-predicate))
+	     (comps (completion-all-sorted-completions))
+	     ;; Compute common substring, if user wants it stripped from
+	     ;; completion candidates.
+	     (common-substring (when strip-common-substring
+				 (let ((common (try-completion "" comps)))
+				   (when (stringp common) common))))
+	     (common-substring-len (when common-substring
+				     (length common-substring)))
+	     width)
+	;; Trim completion, when possible.
+	(if (not completed)
+	    (setq completion nil)
+	  (setq compare (1- (abs compare)))
+	  (setq completion (if (= compare (length string))
+			       (substring completion compare)
+			     (concat "..." completion))))
+
+	;; Account for various filler characters.
+	(setq width (string-width (concat (buffer-string)
+					  "(...)" "{" separator "..." "}")))
+	;; Add completion.
+	(when completion
+	  (setq width (+ width (string-width completion)))
+	  (unless (< width max-width)
+	    (setq completion nil)))
+
+	;; Add completion candidates.
+	(let* ((last (last comps))
+	       (base-size (cdr last))
+	       first limit prospects)
+	  ;; Remove the base-size tail to get a properly
+	  ;; nil-terminated list that is amenable for easy iteration.
+	  (when last (setcdr last nil))
+
+	  ;; Decorate the first of completion with requested face.
+	  (setq first (copy-sequence (pop comps)))
+	  (put-text-property 0 (length first) 'face first-match-face first)
+	  (setq first (if (not common-substring-len) first
+			(substring first common-substring-len)))
+	  (setq width (+ width (string-width first)))
+	  (when (< width max-width)
+	    (setq prospects first))
+
+	  ;; Rest of the completions.
+	  (while (and comps (not limit))
+	    (let* ((comp (pop comps)))
+	      (setq comp (concat separator
+				 (if (not common-substring-len) comp
+				   (substring comp common-substring-len))))
+	      (setq width (+ width (string-width comp)))
+	      (if (< width max-width)
+		  (setq prospects (concat prospects comp))
+		(setq prospects (concat prospects separator "..."))
+		(setq limit t))))
+
+	  ;; Restore the base-size info, since
+	  ;; `completion-all-sorted-completions' is cached.
+	  (when last (setcdr last base-size))
+
+	  ;; Return summary output.
+	  (concat (and completion (concat "(" completion ")"))
+		  (and prospects (concat "{" prospects "}")))))))))
 
 ;;;_ > icomplete-exhibit ()
 (defun icomplete-exhibit ()
   "Insert icomplete completions display.
 Should be run via minibuffer `post-command-hook'.  See `icomplete-mode'
 and `minibuffer-setup-hook'."
-  (when (and icomplete-mode (icomplete-simple-completing-p))
-    (save-excursion
-      (goto-char (point-max))
+  (if (and icomplete-mode (icomplete-simple-completing-p))
+      (save-excursion
+	(goto-char (point-max))
                                         ; Insert the match-status information:
-      (if (and (> (point-max) (minibuffer-prompt-end))
-	       buffer-undo-list		; Wait for some user input.
-	       (or
-		;; Don't bother with delay after certain number of chars:
-		(> (- (point) (field-beginning)) icomplete-max-delay-chars)
-		;; Don't delay if alternatives number is small enough:
-		(and (sequencep minibuffer-completion-table)
-		     (< (length minibuffer-completion-table)
-			icomplete-delay-completions-threshold))
-		;; Delay - give some grace time for next keystroke, before
-		;; embarking on computing completions:
-		(sit-for icomplete-compute-delay)))
-	  (let ((text (while-no-input
-			 (icomplete-completions
-			  (field-string)
-			  minibuffer-completion-table
-			  minibuffer-completion-predicate
-                         (not minibuffer-completion-confirm))))
-		(buffer-undo-list t)
-		deactivate-mark)
-	    ;; Do nothing if while-no-input was aborted.
-	    (when (stringp text)
-              (move-overlay icomplete-overlay (point) (point) (current-buffer))
-              ;; The current C cursor code doesn't know to use the overlay's
-              ;; marker's stickiness to figure out whether to place the cursor
-              ;; before or after the string, so let's spoon-feed it the pos.
-              (put-text-property 0 1 'cursor t text)
-              (overlay-put icomplete-overlay 'after-string text)))))))
-
-;;;_ > icomplete-completions (name candidates predicate require-match)
-(defun icomplete-completions (name candidates predicate require-match)
-  "Identify prospective candidates for minibuffer completion.
-
-The display is updated with each minibuffer keystroke during
-minibuffer completion.
-
-Prospective completion suffixes (if any) are displayed, bracketed by
-one of \(), \[], or \{} pairs.  The choice of brackets is as follows:
-
-  \(...) - a single prospect is identified and matching is enforced,
-  \[...] - a single prospect is identified but matching is optional, or
-  \{...} - multiple prospects, separated by commas, are indicated, and
-          further input is required to distinguish a single one.
-
-The displays for unambiguous matches have ` [Matched]' appended
-\(whether complete or not), or ` \[No matches]', if no eligible
-matches exist.  \(Keybindings for uniquely matched commands
-are exhibited within the square braces.)"
-
-  (let* ((md (completion--field-metadata (field-beginning)))
-	 (comps (completion-all-sorted-completions))
-         (last (if (consp comps) (last comps)))
-         (base-size (cdr last))
-         (open-bracket (if require-match "(" "["))
-         (close-bracket (if require-match ")" "]")))
-    ;; `concat'/`mapconcat' is the slow part.
-    (if (not (consp comps))
-        (format " %sNo matches%s" open-bracket close-bracket)
-      (if last (setcdr last nil))
-      (let* ((most-try
-              (if (and base-size (> base-size 0))
-                  (completion-try-completion
-                   name candidates predicate (length name) md)
-                ;; If the `comps' are 0-based, the result should be
-                ;; the same with `comps'.
-                (completion-try-completion
-                 name comps nil (length name) md)))
-	     (most (if (consp most-try) (car most-try)
-                     (if most-try (car comps) "")))
-             ;; Compare name and most, so we can determine if name is
-             ;; a prefix of most, or something else.
-	     (compare (compare-strings name nil nil
-				       most nil nil completion-ignore-case))
-	     (determ (unless (or (eq t compare) (eq t most-try)
-				 (= (setq compare (1- (abs compare)))
-				    (length most)))
-		       (concat open-bracket
-			       (cond
-				((= compare (length name))
-                                 ;; Typical case: name is a prefix.
-				 (substring most compare))
-				((< compare 5) most)
-				(t (concat "..." (substring most compare))))
-			       close-bracket)))
-	     ;;"-prospects" - more than one candidate
-	     (prospects-len (+ (length determ) 6 ;; take {,...} into account
-                               (string-width (buffer-string))))
-             (prospects-max
-              ;; Max total length to use, including the minibuffer content.
-              (* (+ icomplete-prospects-height
-                    ;; If the minibuffer content already uses up more than
-                    ;; one line, increase the allowable space accordingly.
-                    (/ prospects-len (window-width)))
-                 (window-width)))
-             (prefix-len
-              ;; Find the common prefix among `comps'.
-	      ;; We can't use the optimization below because its assumptions
-	      ;; aren't always true, e.g. when completion-cycling (bug#10850):
-	      ;; (if (eq t (compare-strings (car comps) nil (length most)
-	      ;; 			 most nil nil completion-ignore-case))
-	      ;;     ;; Common case.
-	      ;;     (length most)
-	      ;; Else, use try-completion.
-	      (let ((comps-prefix (try-completion "" comps)))
-		(and (stringp comps-prefix)
-		     (length comps-prefix)))) ;;)
-
-	     prospects most-is-exact comp limit)
-	(if (eq most-try t) ;; (or (null (cdr comps))
-	    (setq prospects nil)
-	  (while (and comps (not limit))
-	    (setq comp
-		  (if prefix-len (substring (car comps) prefix-len) (car comps))
-		  comps (cdr comps))
-	    (cond ((string-equal comp "") (setq most-is-exact t))
-		  ((member comp prospects))
-		  (t (setq prospects-len
-                           (+ (string-width comp) 1 prospects-len))
-		     (if (< prospects-len prospects-max)
-			 (push comp prospects)
-		       (setq limit t))))))
-        ;; Restore the base-size info, since completion-all-sorted-completions
-        ;; is cached.
-        (if last (setcdr last base-size))
-	(if prospects
-	    (concat determ
-		    "{"
-		    (and most-is-exact ",")
-		    (mapconcat 'identity (nreverse prospects) ",")
-		    (and limit ",...")
-		    "}")
-	  (concat determ
-		  " [Matched"
-		  (let ((keys (and icomplete-show-key-bindings
-				   (commandp (intern-soft most))
-				   (icomplete-get-keys most))))
-		    (if keys (concat "; " keys) ""))
-		  "]"))))))
+	(if (and (> (point-max) (minibuffer-prompt-end))
+		 buffer-undo-list	; Wait for some user input.
+		 (or
+		  ;; Don't bother with delay after certain number of chars:
+		  (> (- (point) (field-beginning)) icomplete-max-delay-chars)
+		  ;; Don't delay if alternatives number is small enough:
+		  (and (sequencep minibuffer-completion-table)
+		       (< (length minibuffer-completion-table)
+			  icomplete-delay-completions-threshold))
+		  ;; Delay - give some grace time for next keystroke, before
+		  ;; embarking on computing completions:
+		  (sit-for icomplete-compute-delay)))
+	    (let ((text (while-no-input
+			  (minibuffer-summarize-completions
+			   icomplete-separator
+			   (* icomplete-prospects-height (window-width))
+			   'icomplete-first-match
+			   icomplete-hide-common-prefix)))
+		  (buffer-undo-list t)
+		  deactivate-mark)
+	      ;; Do nothing if while-no-input was aborted.
+	      (when (stringp text)
+		(move-overlay icomplete-overlay (point) (point) (current-buffer))
+		;; The current C cursor code doesn't know to use the overlay's
+		;; marker's stickiness to figure out whether to place the cursor
+		;; before or after the string, so let's spoon-feed it the pos.
+		(put-text-property 0 1 'cursor t text)
+		(overlay-put icomplete-overlay 'after-string text)))))))
 
 ;;_* Local emacs vars.
 ;;Local variables:

=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el	2012-10-28 19:07:52 +0000
+++ lisp/minibuffer.el	2012-11-02 07:57:15 +0000
@@ -1108,12 +1108,68 @@
             (let ((hist (symbol-value minibuffer-history-variable)))
               (setq all (sort all (lambda (c1 c2)
                                     (> (length (member c1 hist))
-                                       (length (member c2 hist))))))))
+                                       (length (member c2 hist)))))))
+	    ;; Bring exact (but not unique) match to the front.
+	    (when (member string all)
+	      (push string all))
+
+	    ;; Delete duplicates.
+	    (delete-dups all))
           ;; Cache the result.  This is not just for speed, but also so that
           ;; repeated calls to minibuffer-force-complete can cycle through
           ;; all possibilities.
           (completion--cache-all-sorted-completions (nconc all base-size))))))
 
+(defun minibuffer-force-complete-and-exit ()
+  "Complete the minibuffer with first of the matches and exit."
+  (interactive)
+  ;; FIXME: Need to deal with the extra-size issue here as well.
+  ;; FIXME: ~/src/emacs/t<M-TAB>/lisp/minibuffer.el completes to
+  ;; ~/src/emacs/trunk/ and throws away lisp/minibuffer.el.
+  (let* ((start (copy-marker (field-beginning)))
+         (end (field-end))
+         ;; (md (completion--field-metadata start))
+         (all (completion-all-sorted-completions))
+         (base (+ start (or (cdr (last all)) 0))))
+    (cond
+     ((not (consp all))
+      (completion--message
+       (if all "No more completions" "No completions")))
+     ((not (consp (cdr all)))
+      (let ((done (equal (car all) (buffer-substring-no-properties base end))))
+        (unless done (completion--replace base end (car all)))
+        (completion--done (buffer-substring-no-properties start (point))
+                          'finished (when done "Sole completion"))
+	(exit-minibuffer)))
+     (t
+      (completion--replace base end (car all))
+      (completion--done (buffer-substring-no-properties start (point))
+			'sole)
+      (exit-minibuffer)))))
+
+(defun minibuffer-forward-sorted-completions ()
+  "Step forward completions by one entry.
+Second entry becomes the first and can be selected with
+`minibuffer-force-complete-and-exit'."
+  (interactive)
+  (let* ((comps (completion-all-sorted-completions))
+	 (last (last comps)))
+    (setcdr last (cons (car comps) (cdr last)))
+    (completion--cache-all-sorted-completions (cdr comps))))
+
+(defun minibuffer-backward-sorted-completions ()
+  "Step backward completions by one entry.
+Last entry becomes the first and can be selected with
+`minibuffer-force-complete-and-exit'."
+  (interactive)
+  (let* ((comps (completion-all-sorted-completions))
+	 (last-but-one (last comps 2))
+	 (last (cdr last-but-one)))
+    (when last
+      (setcdr last-but-one (cdr last))
+      (push (car last) comps)
+      (completion--cache-all-sorted-completions comps))))
+
 (defun minibuffer-force-complete ()
   "Complete the minibuffer to an exact match.
 Repeated uses step through the possible completions."


[-- Attachment #3: Type: text/plain, Size: 67 bytes --]


--8<---------------cut here---------------end--------------->8---

^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-11-02 11:49       ` Jambunathan K
@ 2012-11-02 12:12         ` Jambunathan K
  2012-11-09  1:53           ` Stefan Monnier
  2012-11-09  2:17         ` Stefan Monnier
  1 sibling, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-11-02 12:12 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

Jambunathan K <kjambunathan@gmail.com> writes:

> 1. `minibuffer-summarize-completions' is meant as a replacement for
>    `icomplete-exhibit'.  As the name suggests, it is meant to go in to
>     minibuffer.el.  It's presence in minibuffer.el proved problematic
>    (details in followup mail) and I had to move it to icomplete.el.

0. In my earlier patch, remove `minibuffer-summarize-completions' from
   icomplete.el and shift it enbloc to minibuffer.el.

   Compile and re-load.  emacs -Q

1. M-x icomplete-mode RET
2. M-x load-1
3. See the following error message echoed.

    ,----
    | Icomplete mode enabled
    | 
    | Error in post-command-hook (#[nil \301\302\303!)\207 [non-essential t
    | run-hooks icomplete-post-command-hook] 2]): (error "Attempt to modify
    | read-only object")
    `----

4. If I grep for the above error text, it happens in 

--8<---------------cut here---------------start------------->8---
-*- mode: grep; default-directory: "~/src/emacs/trunk/src/" -*-
Grep started at Fri Nov  2 17:35:44

find . -type d \( -path \*/SCCS -o -path \*/RCS -o -path \*/CVS -o -path \*/MCVS -o -path \*/.svn -o -path \*/.git -o -path \*/.hg -o -path \*/.bzr -o -path \*/_MTN -o -path \*/_darcs -o -path \*/\{arch\} \) -prune -o \! -type d \( -name .\#\* -o -name \*.o -o -name \*\~ -o -name \*.bin -o -name \*.lbin -o -name \*.so -o -name \*.a -o -name \*.ln -o -name \*.blg -o -name \*.bbl -o -name \*.elc -o -name \*.lof -o -name \*.glo -o -name \*.idx -o -name \*.lot -o -name \*.fmt -o -name \*.tfm -o -name \*.class -o -name \*.fas -o -name \*.lib -o -name \*.mem -o -name \*.x86f -o -name \*.sparcf -o -name \*.dfsl -o -name \*.pfsl -o -name \*.d64fsl -o -name \*.p64fsl -o -name \*.lx64fsl -o -name \*.lx32fsl -o -name \*.dx64fsl -o -name \*.dx32fsl -o -name \*.fx64fsl -o -name \*.fx32fsl -o -name \*.sx64fsl -o -name \*.sx32fsl -o -name \*.wx64fsl -o -name \*.wx32fsl -o -name \*.fasl -o -name \*.ufsl -o -name \*.fsl -o -name \*.dxl -o -name \*.lo -o -name \*.la -o -name \*.gmo -o -name \*.mo -o -name \*.toc -o -name \*.aux -o -name \*.cp -o -name \*.fn -o -name \*.ky -o -name \*.pg -o -name \*.tp -o -name \*.vr -o -name \*.cps -o -name \*.fns -o -name \*.kys -o -name \*.pgs -o -name \*.tps -o -name \*.vrs -o -name \*.pyc -o -name \*.pyo \) -prune -o  -type f \( -name \*.\[ch\] \) -exec grep  -nH -e Attempt\ to\ modify\ read-only\ object {} +
./data.c:104:  error ("Attempt to modify read-only object");

Grep finished (matches found) at Fri Nov  2 17:35:44
--8<---------------cut here---------------end--------------->8---

which is

--8<---------------cut here---------------start------------->8---
void
pure_write_error (void)
{
  error ("Attempt to modify read-only object");
}
--8<---------------cut here---------------end--------------->8---

this in turn leads to

--8<---------------cut here---------------start------------->8---

-*- mode: grep; default-directory: "~/src/emacs/trunk/src/" -*-
Grep started at Fri Nov  2 17:37:03

find . -type d \( -path \*/SCCS -o -path \*/RCS -o -path \*/CVS -o -path \*/MCVS -o -path \*/.svn -o -path \*/.git -o -path \*/.hg -o -path \*/.bzr -o -path \*/_MTN -o -path \*/_darcs -o -path \*/\{arch\} \) -prune -o \! -type d \( -name .\#\* -o -name \*.o -o -name \*\~ -o -name \*.bin -o -name \*.lbin -o -name \*.so -o -name \*.a -o -name \*.ln -o -name \*.blg -o -name \*.bbl -o -name \*.elc -o -name \*.lof -o -name \*.glo -o -name \*.idx -o -name \*.lot -o -name \*.fmt -o -name \*.tfm -o -name \*.class -o -name \*.fas -o -name \*.lib -o -name \*.mem -o -name \*.x86f -o -name \*.sparcf -o -name \*.dfsl -o -name \*.pfsl -o -name \*.d64fsl -o -name \*.p64fsl -o -name \*.lx64fsl -o -name \*.lx32fsl -o -name \*.dx64fsl -o -name \*.dx32fsl -o -name \*.fx64fsl -o -name \*.fx32fsl -o -name \*.sx64fsl -o -name \*.sx32fsl -o -name \*.wx64fsl -o -name \*.wx32fsl -o -name \*.fasl -o -name \*.ufsl -o -name \*.fsl -o -name \*.dxl -o -name \*.lo -o -name \*.la -o -name \*.gmo -o -name \*.mo -o -name \*.toc -o -name \*.aux -o -name \*.cp -o -name \*.fn -o -name \*.ky -o -name \*.pg -o -name \*.tp -o -name \*.vr -o -name \*.cps -o -name \*.fns -o -name \*.kys -o -name \*.pgs -o -name \*.tps -o -name \*.vrs -o -name \*.pyc -o -name \*.pyo \) -prune -o  -type f \( -name \*.\[ch\] \) -exec grep  -nH -e CHECK_IMPURE {} +
./puresize.h:74:#define CHECK_IMPURE(obj) \
./intervals.c:104:  CHECK_IMPURE (parent);
./keymap.c:350:	  CHECK_IMPURE (prev);
./keymap.c:852:		CHECK_IMPURE (elt);
./keymap.c:905:		CHECK_IMPURE (elt);
./keymap.c:949:      CHECK_IMPURE (insertion_point);
./data.c:490:  CHECK_IMPURE (cell);
./data.c:500:  CHECK_IMPURE (cell);
./data.c:2120:  CHECK_IMPURE (array);

Grep finished (matches found) at Fri Nov  2 17:37:03

--8<---------------cut here---------------end--------------->8---

I am pretty sure the error happens during `put-overlay' and my suspect
is intervals.c.

,---- from icomplete.el (See `icomplete-exhibit')
| (when (stringp text)
|   (move-overlay icomplete-overlay (point) (point) (current-buffer))
|   ;; The current C cursor code doesn't know to use the overlay's
|   ;; marker's stickiness to figure out whether to place the cursor
|   ;; before or after the string, so let's spoon-feed it the pos.
|   (put-text-property 0 1 'cursor t text) <===========================
|   (overlay-put icomplete-overlay 'after-string text))
`----

Since this happens when the file is in minibuffer.el and the return
values are "string constants", I am suspecting that some config value
has to be bumped somewhere.













^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-11-02 12:12         ` Jambunathan K
@ 2012-11-09  1:53           ` Stefan Monnier
  0 siblings, 0 replies; 42+ messages in thread
From: Stefan Monnier @ 2012-11-09  1:53 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> 0. In my earlier patch, remove `minibuffer-summarize-completions' from
>    icomplete.el and shift it enbloc to minibuffer.el.

minibuffer.el is preloaded, which implies among other things that some
of its data will be stored in the `pure' section, which is read-only.

We can try to track down which object is being modified this way (your
guess doesn't sound bad), but in any case I see no reason to move
minibuffer-summarize-completions to minibuffer.el.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-11-02 11:49       ` Jambunathan K
  2012-11-02 12:12         ` Jambunathan K
@ 2012-11-09  2:17         ` Stefan Monnier
  2012-11-09  4:25           ` Jambunathan K
  1 sibling, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-11-09  2:17 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> It is not that patch is final.  It is worthy of further discussion and
> experimentation.

Than you.  Comments below.

> 1. `minibuffer-summarize-completions' is meant as a replacement for
>    `icomplete-exhibit'.  As the name suggests, it is meant to go in to
>     minibuffer.el.  It's presence in minibuffer.el proved problematic
>    (details in followup mail) and I had to move it to icomplete.el.

Why would you want to put it in minibuffer.el?

>    TODO: Handle key binding hints for sole command matches?  Is it
>    really necessary.  It seems so `old school'.  May be it made sense
>    when Emacs /did not/ provide key binding hints.

Agreed.  I don't like it very much, especially because it is very ad-hoc
(and worse, the code can be triggered in cases where it makes no sense
to provide those shortcuts).

> 2. There is a display overlay that is used in minibuffer (see
>    `minibuffer-message').  There is a counterpart in icomplete.el named
>    `icomplete-overlay'?
>    I am wondering whether `icomplete-overlay' could be thrown away and
>    have it use, yet to be introduced `minibuffer-overlay'.  Should this
>    be buffer-local etc etc. I am not sure of.
>    I can exchange notes if there is some interest around this area.

I haven't had time to look into it.  Let's keep it for later.

> 3. Apropos `minibuffer-force-complete-and-exit' and `confirm' etc.

>>> +(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.
>   Can `test-completion' fail if the prompt is filled from valid
>   candidates?

Yes, very much so (think of completing a pdf filename, where the
completion will offer you directories, but those are only intermediate
states and are not valid final states).

> +(defvar icomplete-separator " | "
> +  "String used by icomplete to separate alternatives in the minibuffer.")

Better make it a defcustom.

> -(defcustom icomplete-minibuffer-setup-hook nil
> +(defcustom icomplete-minibuffer-setup-hook
> +  '((lambda ()
> +      (define-key icomplete-minibuffer-map
> +	"\C-j" 'minibuffer-force-complete-and-exit)
> +      (define-key icomplete-minibuffer-map
> +	"\C-s" 'minibuffer-forward-sorted-completions)
> +      (define-key icomplete-minibuffer-map
> +	"\C-r" 'minibuffer-backward-sorted-completions)))

Hooks should default to nil, with only *very* few exceptions.

Those define-key belong directly in the definition of
icomplete-minibuffer-map.

> -    (set (make-local-variable 'completion-show-inline-help) nil)
> +    ;; (set (make-local-variable 'completion-show-inline-help) nil)

Why?

> @@ -227,155 +259,192 @@
>    "Remove completions display \(if any) prior to new user input.
>  Should be run in on the minibuffer `pre-command-hook'.  See `icomplete-mode'
>  and `minibuffer-setup-hook'."
> -  (delete-overlay icomplete-overlay))
> +  (unless (memq this-command '(minibuffer-force-complete-and-exit minibuffer-forward-sorted-completions
> +						    minibuffer-backward-sorted-completions))
> +    ;; Current command does not belong to icomplete-mode.
> +    ;; Delete the overlay.
> +    (delete-overlay icomplete-overlay)))

Why do you need such ad-hoc special case for those commands?
things like `this-command' are always brittle, so while it's often
unavoidable, I want to make sure it really is so.

> +(defun minibuffer-summarize-completions (&optional separator max-width
> +						   first-match-face
> +						   strip-common-substring)

Please move this back to where it was (right after icomplete-exhibit),
so that `diff' can show something a bit more useful.

>  ;;;_ > icomplete-exhibit ()
>  (defun icomplete-exhibit ()
>    "Insert icomplete completions display.
>  Should be run via minibuffer `post-command-hook'.  See `icomplete-mode'
>  and `minibuffer-setup-hook'."
> -  (when (and icomplete-mode (icomplete-simple-completing-p))
> -    (save-excursion
> -      (goto-char (point-max))
> +  (if (and icomplete-mode (icomplete-simple-completing-p))
> +      (save-excursion
> +	(goto-char (point-max))

Why turn this `when' into an `if'?

> @@ -1108,12 +1108,68 @@
>              (let ((hist (symbol-value minibuffer-history-variable)))
>                (setq all (sort all (lambda (c1 c2)
>                                      (> (length (member c1 hist))
> -                                       (length (member c2 hist))))))))
> +                                       (length (member c2 hist)))))))
> +	    ;; Bring exact (but not unique) match to the front.
> +	    (when (member string all)
> +	      (push string all))

I see why this is often a good idea, but I'd be interested to hear
concrete use-cases where this was useful/needed.  The reason for it is
that the default ordering (shortest first) already should give you the
"exact but not unique is first".  And the other ordering rule (prefer
elements from the history) sounds just as valid as the one you suggest,
so I'm not sure why yours should take precedence.

Furthermore, the "exact but not unique first" is actually kind of
useless in the sense that the user can already just hit RET to get that
one, so she doesn't need much more help to access it.

> +	    ;; Delete duplicates.
> +	    (delete-dups all))

Sounds OK.

> +(defun minibuffer-force-complete-and-exit ()
> +  "Complete the minibuffer with first of the matches and exit."
> +  (interactive)
> +  ;; FIXME: Need to deal with the extra-size issue here as well.
> +  ;; FIXME: ~/src/emacs/t<M-TAB>/lisp/minibuffer.el completes to
> +  ;; ~/src/emacs/trunk/ and throws away lisp/minibuffer.el.
> +  (let* ((start (copy-marker (field-beginning)))
> +         (end (field-end))
> +         ;; (md (completion--field-metadata start))
> +         (all (completion-all-sorted-completions))
> +         (base (+ start (or (cdr (last all)) 0))))
> +    (cond
> +     ((not (consp all))
> +      (completion--message
> +       (if all "No more completions" "No completions")))
> +     ((not (consp (cdr all)))
> +      (let ((done (equal (car all) (buffer-substring-no-properties base end))))
> +        (unless done (completion--replace base end (car all)))
> +        (completion--done (buffer-substring-no-properties start (point))
> +                          'finished (when done "Sole completion"))
> +	(exit-minibuffer)))
> +     (t
> +      (completion--replace base end (car all))
> +      (completion--done (buffer-substring-no-properties start (point))
> +			'sole)
> +      (exit-minibuffer)))))

I'd suggest you simply call minibuffer-force-complete rather than
copying its code.  Also, after that call, just call
minibuffer-complete-and-exit (or rather, extract the body of
minibuffer-complete-and-exit to a new function which takes an addition
argument to know whether it should try to complete or not, and then call
this function from minibuffer-complete-and-exit (asking it to complete
if needed) as well as from minibuffer-force-complete-and-exit (asking
not to complete since we've just done it via minibuffer-force-complete).

> +(defun minibuffer-forward-sorted-completions ()

I think this should be in icomplete.el since this functionality doesn't
make much sense if completion-all-sorted-completions isn't displayed.

> +    (setcdr last (cons (car comps) (cdr last)))

You can use (push (car comps) (cdr last)) in Emacs-24.

> +      (setcdr last-but-one (cdr last))
> +      (push (car last) comps)

I think you can do (push (pop (cdr last-but-one)) comps).


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-11-09  2:17         ` Stefan Monnier
@ 2012-11-09  4:25           ` Jambunathan K
  2012-11-09 14:12             ` Stefan Monnier
  0 siblings, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-11-09  4:25 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638


Quick summary:

1. `minibuffer-summarize-completions' is a re-write of
   `icomplete-exhibit' and line-by-line diff is not possible.

2. `minibuffer-summarize-completions' belongs in minibuffer.el and not
   icomplete.

I need your feedback on 1 and 2 before I re-work the patch. 

Below, I present "arguments" in favor of (1) and (2).

>> 1. `minibuffer-summarize-completions' is meant as a replacement for
>>    `icomplete-exhibit'.  As the name suggests, it is meant to go in to
>>     minibuffer.el.  It's presence in minibuffer.el proved problematic
>>    (details in followup mail) and I had to move it to icomplete.el.
>
> Why would you want to put it in minibuffer.el?

It converts completion to a string much along the lines of
`minibuffer-completion-help'.  For example, when number of completions
hit the threshold one can arrange a call to summarize function.  I think
it would be useful.

It is also in the same spirit as `completion--done' - a
"completion--not-done" so to speak.

>> +(defun minibuffer-summarize-completions (&optional separator max-width
>> +						   first-match-face
>> +						   strip-common-substring)
>
> Please move this back to where it was (right after icomplete-exhibit),
> so that `diff' can show something a bit more useful.

It wouldn't because it is re-written.  I am not sure whether you like
the re-write but it was definitely begging for re-write.  (Please
confirm your preference here, so that I know what is acceptable.)

You may want to focus on the `compare-strings' part where the
conditional accounts for `ignore-case' logic.

FWIW,

 `minibuffer-summarize-completions' does what `icomplete-exhibit'
does. But it has the "same structure" as `completion--do-completion' and 

- It doesn't try to complete or change the minibuffer.

- It doesn't call `minibuffer-help-message', `completion--done' or
  `completion-message'.  Instead it builds it's own string.

The new re-write would make it easy to extract the summary builder to
it's own "message function" potentially along the lines of
`:exit-function' and `:annotation-function'.

> > +(defun minibuffer-forward-sorted-completions ()

> I think this should be in icomplete.el since this functionality doesn't
> make much sense if completion-all-sorted-completions isn't displayed.

Consistent if you are willing to accept my argument that
`minibuffer-summarize-completions' belongs and in minibuffer.el.


>>    TODO: Handle key binding hints for sole command matches?  Is it
>>    really necessary.  It seems so `old school'.  May be it made sense
>>    when Emacs /did not/ provide key binding hints.
>
> Agreed.  I don't like it very much, especially because it is very ad-hoc
> (and worse, the code can be triggered in cases where it makes no sense
> to provide those shortcuts).

I will remove it in the next round and obsolete the corresponding
variable.

>
>> 2. There is a display overlay that is used in minibuffer (see
>>    `minibuffer-message').  There is a counterpart in icomplete.el named
>>    `icomplete-overlay'?
>>    I am wondering whether `icomplete-overlay' could be thrown away and
>>    have it use, yet to be introduced `minibuffer-overlay'.  Should this
>>    be buffer-local etc etc. I am not sure of.
>>    I can exchange notes if there is some interest around this area.
>
> I haven't had time to look into it.  Let's keep it for later.
>
>> 3. Apropos `minibuffer-force-complete-and-exit' and `confirm' etc.
>
>> -    (set (make-local-variable 'completion-show-inline-help) nil)
>> +    ;; (set (make-local-variable 'completion-show-inline-help) nil)
>
> Why?

Just wanted to bring attention to the fact that icomplete uses it's own
overlay.  It is through this variable it shuts down minibuffer's very
own overlay.

>> @@ -227,155 +259,192 @@
>>    "Remove completions display \(if any) prior to new user input.
>>  Should be run in on the minibuffer `pre-command-hook'.  See `icomplete-mode'
>>  and `minibuffer-setup-hook'."
>> -  (delete-overlay icomplete-overlay))
>> +  (unless (memq this-command '(minibuffer-force-complete-and-exit minibuffer-forward-sorted-completions
>> +						    minibuffer-backward-sorted-completions))
>> +    ;; Current command does not belong to icomplete-mode.
>> +    ;; Delete the overlay.
>> +    (delete-overlay icomplete-overlay)))
>
> Why do you need such ad-hoc special case for those commands?
> things like `this-command' are always brittle, so while it's often
> unavoidable, I want to make sure it really is so.

Avoids "flicker" in the minibuffer.  More aesthetic than functional, not
strictly necessary.



>> @@ -1108,12 +1108,68 @@
>>              (let ((hist (symbol-value minibuffer-history-variable)))
>>                (setq all (sort all (lambda (c1 c2)
>>                                      (> (length (member c1 hist))
>> -                                       (length (member c2 hist))))))))
>> +                                       (length (member c2 hist)))))))
>> +	    ;; Bring exact (but not unique) match to the front.
>> +	    (when (member string all)
>> +	      (push string all))
>
> I see why this is often a good idea, but I'd be interested to hear
> concrete use-cases where this was useful/needed.  The reason for it is
> that the default ordering (shortest first) already should give you the
> "exact but not unique is first".  And the other ordering rule (prefer
> elements from the history) sounds just as valid as the one you suggest,
> so I'm not sure why yours should take precedence.

While in buffer, history takes precedence over the length rule.  It is
possible that the exact match gets stacked deep in the summary.

Also when common substring is stripped, the exact match looks weird - it
becomes an empty string and is easily recognized when it is a first
item.

> Furthermore, the "exact but not unique first" is actually kind of
> useless in the sense that the user can already just hit RET to get that
> one, so she doesn't need much more help to access it.
>> +	    ;; Delete duplicates.
>> +	    (delete-dups all))

The code merely moves the following condition in icomplete.el to it's
rightful place which is sorted completion itself. I have no opinion
whether the exact element should forcibly brought to the front.


-	    (cond ((string-equal comp "") (setq most-is-exact t))
-		  ((member comp prospects))
-		  (t (setq prospects-len
-                           (+ (string-width comp) 1 prospects-len))
-		     (if (< prospects-len prospects-max)
-			 (push comp prospects)
-		       (setq limit t))))))







^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-11-09  4:25           ` Jambunathan K
@ 2012-11-09 14:12             ` Stefan Monnier
  0 siblings, 0 replies; 42+ messages in thread
From: Stefan Monnier @ 2012-11-09 14:12 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> 1. `minibuffer-summarize-completions' is a re-write of
>    `icomplete-exhibit' and line-by-line diff is not possible.

Are there behavior differences?  If so, can you summarize them (pun
intended) before I try to understand the code?

> 2. `minibuffer-summarize-completions' belongs in minibuffer.el and not
>    icomplete.

Why?

> It wouldn't because it is re-written.  I am not sure whether you like
> the re-write but it was definitely begging for re-write.  (Please
> confirm your preference here, so that I know what is acceptable.)

I'm not necessarily opposed to a rewrite, but I think such a rewrite
should aim to split it into smaller functions: the main problem with
icomplete-exhibit is that it's too large.

> The new re-write would make it easy to extract the summary builder to
> it's own "message function" potentially along the lines of
> `:exit-function' and `:annotation-function'.

Is that intended to explain why you want to move it to minibuffer.el?
If so, I don't understand it.  Let's try to focus on improving icomplete
for now.  If/when some code in minibuffer.el could make use of some code
in icomplete.el it'll be easy enough to move the code.

If you move icomplete-exhibit (or its replacement) to minibuffer.el then
you may as well move the whole of icomplete.el to minibuffer.el, and I'm
not interested in doing that, for now.

>> Agreed.  I don't like it very much, especially because it is very ad-hoc
>> (and worse, the code can be triggered in cases where it makes no sense
>> to provide those shortcuts).
> I will remove it in the next round and obsolete the corresponding
> variable.

If the variable doesn't work at all, then just remove it.  "Obsolete" is
used for things that are planned for removal but that still work.

>>> -    (set (make-local-variable 'completion-show-inline-help) nil)
>>> +    ;; (set (make-local-variable 'completion-show-inline-help) nil)
>> Why?
> Just wanted to bring attention to the fact that icomplete uses it's own
> overlay.  It is through this variable it shuts down minibuffer's very
> own overlay.

I remember this part.  But if we keep such a reorganization for later,
we still need this `set' here, right?

>>> +  (unless (memq this-command '(minibuffer-force-complete-and-exit minibuffer-forward-sorted-completions
>>> +						    minibuffer-backward-sorted-completions))
>>> +    ;; Current command does not belong to icomplete-mode.
>>> +    ;; Delete the overlay.
>>> +    (delete-overlay icomplete-overlay)))
>> Why do you need such ad-hoc special case for those commands?
>> things like `this-command' are always brittle, so while it's often
>> unavoidable, I want to make sure it really is so.
> Avoids "flicker" in the minibuffer.

Hmm... there shouldn't be any such visible flicker, even if we
delete-overlay and then re-add the overlay in post-command-hook
(because there shouldn't be any redisplay between the two).
Do you have a test case showing this problem?

Also, why include minibuffer-force-complete-and-exit in this list, since
it will exit the minibuffer so the post-command-hook won't be run, so
there won't be flicker anyway.

>> I see why this is often a good idea, but I'd be interested to hear
>> concrete use-cases where this was useful/needed.  The reason for it is
>> that the default ordering (shortest first) already should give you the
>> "exact but not unique is first".  And the other ordering rule (prefer
>> elements from the history) sounds just as valid as the one you suggest,
>> so I'm not sure why yours should take precedence.
> While in buffer, history takes precedence over the length rule.  It is
> possible that the exact match gets stacked deep in the summary.
> Also when common substring is stripped, the exact match looks weird - it
> becomes an empty string and is easily recognized when it is a first
> item.

Right, which is why icomplete.el already displays it first, no matter
where it is in the list.

>> Furthermore, the "exact but not unique first" is actually kind of
>> useless in the sense that the user can already just hit RET to get that
>> one, so she doesn't need much more help to access it.
>>> +	    ;; Delete duplicates.
>>> +	    (delete-dups all))
> The code merely moves the following condition in icomplete.el to it's
> rightful place which is sorted completion itself.

Then I'd rather not do it.  If you have a specific use-case where that
solves a misbehavior, then we can discuss it, but otherwise I'd rather
focus on the main task at hand: incremental improvements to bring
icomplete.el a bit closer to ido.el.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-13 17:33 bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode Jambunathan K
                   ` (2 preceding siblings ...)
  2012-10-23 19:43 ` Stefan Monnier
@ 2012-11-29 21:34 ` Stefan Monnier
  2012-11-30  6:18   ` Jambunathan K
  2013-11-15  4:42 ` Jambunathan K
  4 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-11-29 21:34 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

I've installed part of your changes.  The most glaringly missing part is
the "highlight first element" feature.

Also I noticed that the minibuffer-force-complete command interacts
incorrectly with icomplete, so there's more work to do there.

I've resolved the flickering in a different way, BTW.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-11-29 21:34 ` Stefan Monnier
@ 2012-11-30  6:18   ` Jambunathan K
  2012-11-30 19:37     ` Stefan Monnier
  0 siblings, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-11-30  6:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

[-- Attachment #1: Type: text/plain, Size: 625 bytes --]


> I've installed part of your changes.  The most glaringly missing part is
> the "highlight first element" feature.

Thanks.  The attached patch should take care of this.

I would also like an option to show candidates in full.  But this after
the attached patch is applied.

,----
| +(defcustom icomplete-hide-common-prefix t
| +  "When non-nil, hide common prefix (if any) from completion candidates.
| +Default setting (typically) allows more candidates to be shown
| +for given `icomplete-prospects-height'.  When nil, show
| +candidates in full."
| +  :type 'boolean
| +  :version "24.3"
| +  :group 'icomplete)
`----


[-- Attachment #2: icomplete-first-match.diff --]
[-- Type: text/x-diff, Size: 2086 bytes --]

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2012-11-30 04:44:52 +0000
+++ lisp/ChangeLog	2012-11-30 06:08:20 +0000
@@ -1,3 +1,8 @@
+2012-11-30  Jambunathan K  <kjambunathan@gmail.com>
+
+	* icomplete.el (icomplete-first-match): New face.
+	(icomplete-completions): Use it (bug#12638).
+
 2012-11-30  OKAZAKI Tetsurou  <okazaki.tetsurou@gmail.com>  (tiny change)
 
 	* vc/vc.el (vc-register): Allow registering a file which is

=== modified file 'lisp/icomplete.el'
--- lisp/icomplete.el	2012-11-29 21:32:24 +0000
+++ lisp/icomplete.el	2012-11-30 06:01:14 +0000
@@ -76,6 +76,11 @@
   :type 'string
   :version "24.3")
 
+(defface icomplete-first-match  '((t :weight bold))
+  "Face used by icomplete for highlighting the first match."
+  :version "24.3"
+  :group 'icomplete)
+
 ;;;_* User Customization variables
 (defcustom icomplete-prospects-height
   ;; 20 is an estimated common size for the prompt + minibuffer content, to
@@ -377,6 +382,13 @@ are exhibited within the square braces.)
 		     (if (< prospects-len prospects-max)
 			 (push comp prospects)
 		       (setq limit t))))))
+	(setq prospects (nreverse prospects))
+	;; Highlight first of the prospects.
+	(when (and prospects (not most-is-exact))
+	  (let ((first (copy-sequence (pop prospects))))
+	    (put-text-property 0 (length first)
+			       'face 'icomplete-first-match first)
+	    (push first prospects)))
         ;; Restore the base-size info, since completion-all-sorted-completions
         ;; is cached.
         (if last (setcdr last base-size))
@@ -386,8 +398,7 @@ are exhibited within the square braces.)
 		    (and most-is-exact
                          (substring icomplete-separator
                                     (string-match "[^ ]" icomplete-separator)))
-		    (mapconcat 'identity (nreverse prospects)
-                               icomplete-separator)
+		    (mapconcat 'identity prospects icomplete-separator)
 		    (and limit (concat icomplete-separator "…"))
 		    "}")
 	  (concat determ " [Matched]"))))))


^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-11-30  6:18   ` Jambunathan K
@ 2012-11-30 19:37     ` Stefan Monnier
  2012-12-04 12:54       ` Jambunathan K
  0 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-11-30 19:37 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

>> I've installed part of your changes.  The most glaringly missing part is
>> the "highlight first element" feature.
> Thanks.  The attached patch should take care of this.
> I would also like an option to show candidates in full.  But this after
> the attached patch is applied.

Oh right, I also forgot that part.  Can you send a combined patch?


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-11-30 19:37     ` Stefan Monnier
@ 2012-12-04 12:54       ` Jambunathan K
  2012-12-04 15:02         ` Stefan Monnier
  0 siblings, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-12-04 12:54 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> I've installed part of your changes.  The most glaringly missing part is
>>> the "highlight first element" feature.
>> Thanks.  The attached patch should take care of this.
>> I would also like an option to show candidates in full.  But this after
>> the attached patch is applied.
>
> Oh right, I also forgot that part.  Can you send a combined patch?

I will.  

But couple of issues which you may want to clear.  (These were
introduced by you, so I will rather have you clear them up)

1. Do you adjust for (string-width icomplete-separator) while deciding on
   prospects.

2. M-x debbugs-gnu

   There is no way I can C-s C-j to the second or third candidates.  I
   normally use debbbugs-gnu-search which is the second candidate.

>         Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 12:54       ` Jambunathan K
@ 2012-12-04 15:02         ` Stefan Monnier
  2012-12-04 15:30           ` Jambunathan K
  2012-12-04 15:51           ` Jambunathan K
  0 siblings, 2 replies; 42+ messages in thread
From: Stefan Monnier @ 2012-12-04 15:02 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

>> Oh right, I also forgot that part.  Can you send a combined patch?
> I will.

Thanks.

> But couple of issues which you may want to clear.  (These were
> introduced by you, so I will rather have you clear them up)
> 1. Do you adjust for (string-width icomplete-separator) while deciding on
>    prospects.

No, currently I completely overlook this problem (I don't even take
(length icomplete-separator) into account).

> 2. M-x debbugs-gnu
>    There is no way I can C-s C-j to the second or third candidates.
>    I normally use debbbugs-gnu-search which is the second candidate.

Better ask this elsewhere, I'm not up to speed on debbugs-gnu.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 15:02         ` Stefan Monnier
@ 2012-12-04 15:30           ` Jambunathan K
  2012-12-04 15:45             ` Stefan Monnier
  2012-12-04 15:51           ` Jambunathan K
  1 sibling, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-12-04 15:30 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 2. M-x debbugs-gnu
>>    There is no way I can C-s C-j to the second or third candidates.
>>    I normally use debbbugs-gnu-search which is the second candidate.
>
> Better ask this elsewhere, I'm not up to speed on debbugs-gnu.

Why are you in such a haste? Am I really talking about debbugs-gnu?





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 15:30           ` Jambunathan K
@ 2012-12-04 15:45             ` Stefan Monnier
  2012-12-04 16:12               ` Jambunathan K
  0 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-12-04 15:45 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

>>> 2. M-x debbugs-gnu
>>> There is no way I can C-s C-j to the second or third candidates.
>>> I normally use debbbugs-gnu-search which is the second candidate.
>> Better ask this elsewhere, I'm not up to speed on debbugs-gnu.
> Why are you in such a haste? Am I really talking about debbugs-gnu?

Oh, sorry, I misunderstood (and yes, there's so much email to read in
this list and so little time to devote to it, that I always read it
in a hurry).

I'm not sure what's the best course of action here.
Maybe minibuffer-force-complete should always choose the first completion
not equal to the current content.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 15:02         ` Stefan Monnier
  2012-12-04 15:30           ` Jambunathan K
@ 2012-12-04 15:51           ` Jambunathan K
  2012-12-13 13:51             ` Jambunathan K
  1 sibling, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-12-04 15:51 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 2. M-x debbugs-gnu
>>    There is no way I can C-s C-j to the second or third candidates.
>>    I normally use debbbugs-gnu-search which is the second candidate.
>
> Better ask this elsewhere, I'm not up to speed on debbugs-gnu.

I am attaching screenshot of minibuffer. 

    1. M-x debbugs-gnu
    2. Immediately cycle with C-s

Cycling may happen internally but the UI presents the "most-is-exact"
always at the head.  This is a definite bug.


[-- Attachment #2: icomplete-1.png --]
[-- Type: image/png, Size: 1027 bytes --]

[-- Attachment #3: icomplete-2.png --]
[-- Type: image/png, Size: 1046 bytes --]

[-- Attachment #4: Type: text/plain, Size: 5 bytes --]


-- 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 15:45             ` Stefan Monnier
@ 2012-12-04 16:12               ` Jambunathan K
  2012-12-04 17:14                 ` Stefan Monnier
  0 siblings, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-12-04 16:12 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>> 2. M-x debbugs-gnu
>>>> There is no way I can C-s C-j to the second or third candidates.
>>>> I normally use debbbugs-gnu-search which is the second candidate.
>>> Better ask this elsewhere, I'm not up to speed on debbugs-gnu.
>> Why are you in such a haste? Am I really talking about debbugs-gnu?
>
> Oh, sorry, I misunderstood (and yes, there's so much email to read in
> this list and so little time to devote to it, that I always read it
> in a hurry).
>
> I'm not sure what's the best course of action here.
> Maybe minibuffer-force-complete should always choose the first completion
> not equal to the current content.

The problem is in the displayed string.  Why not push "" to prospects
when most-is-exact.  I think completions are cycling but the displayed
string is not.

Is removal of leading space really necessary?  Things would be simpler
otherwise.

,----
| (if prospects
|     (concat determ
| 	    "{"
| ,----
| |            (and most-is-exact
| |                 (substring icomplete-separator
| |                            (string-match "[^ ]" icomplete-separator)))
| `----
| 
| 	    (mapconcat 'identity (nreverse prospects)
|                        icomplete-separator)
| 	    (and limit (concat icomplete-separator "…"))
| 	    "}")
|   (concat determ " [Matched]"))
`----

>         Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 16:12               ` Jambunathan K
@ 2012-12-04 17:14                 ` Stefan Monnier
  2012-12-04 17:32                   ` Jambunathan K
  0 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-12-04 17:14 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> The problem is in the displayed string.  Why not push "" to prospects
> when most-is-exact.  I think completions are cycling but the displayed
> string is not.

The problem is that we want the display to indicate that the current
field is an exact match.  Currently this is visible thanks to the "{| ...}".
So if we want to let the empty string move elsewhere (and hence
potentially off-screen), we need another way to indicate that we have an
exact match.

> Is removal of leading space really necessary?

No, it just makes a lot of sense in the current situation.  Note that if
we let the empty string appear elsewhere, we may also want to use
"a | | b" instead of "a |  | b", tho it's clearly less important.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 17:14                 ` Stefan Monnier
@ 2012-12-04 17:32                   ` Jambunathan K
  2012-12-12  3:18                     ` Stefan Monnier
  0 siblings, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-12-04 17:32 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The problem is in the displayed string.  Why not push "" to prospects
>> when most-is-exact.  I think completions are cycling but the displayed
>> string is not.
>
> The problem is that we want the display to indicate that the current
> field is an exact match.  Currently this is visible thanks to the "{| ...}".
> So if we want to let the empty string move elsewhere (and hence
> potentially off-screen), we need another way to indicate that we have an
> exact match.

RET pushes the minibuffer contents.
C-j pushes the head of the list.

I think appending, "[Complete, but not unique]????" should serve as a
cue that one can simply RET.

>> Is removal of leading space really necessary?
>
> No, it just makes a lot of sense in the current situation.  Note that if
> we let the empty string appear elsewhere, we may also want to use
> "a | | b" instead of "a |  | b", tho it's clearly less important.

This could be an aside.  

I never could come up with a sensible explanation for magic number 5
down below.

,----
|  (determ (unless (or (eq t compare) (eq t most-try)
|     		 (= (setq compare (1- (abs compare)))
|     		    (length most)))
|            (concat open-bracket
|     	       (cond
|     		((= compare (length name))
|                      ;; Typical case: name is a prefix.
|     		 (substring most compare))
| ,----
| |     		((< compare 5) most)
| `----
| 
|     		(t (concat "..." (substring most compare))))
|   		       close-bracket)))
`----

>         Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 17:32                   ` Jambunathan K
@ 2012-12-12  3:18                     ` Stefan Monnier
  2012-12-12  3:42                       ` Drew Adams
  0 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-12-12  3:18 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

>>> The problem is in the displayed string.  Why not push "" to prospects
>>> when most-is-exact.  I think completions are cycling but the displayed
>>> string is not.
>> The problem is that we want the display to indicate that the current
>> field is an exact match.  Currently this is visible thanks to the "{| ...}".
>> So if we want to let the empty string move elsewhere (and hence
>> potentially off-screen), we need another way to indicate that we have an
>> exact match.

> RET pushes the minibuffer contents.
> C-j pushes the head of the list.

Right, so I was suggesting to make C-j skip to the second element if the
first is equal to the minibuffer contents, so that by choosing between RET
and C-j the user can choose which one she wants.

> I think appending, "[Complete, but not unique]????" should serve as a
> cue that one can simply RET.

You mean we'd display "foo[Complete but not unique]{bar | | baz}"?

> I never could come up with a sensible explanation for magic number 5
> down below.

Good question ... oh yes, it's because of the "..." on the line below:
we refrain from truncating the common prefix if the truncation would
gain us less than 2 columns.  Basically, it's a tradeoff between
losing information via truncation and losing screen real-estate if we
don't truncate.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-12  3:18                     ` Stefan Monnier
@ 2012-12-12  3:42                       ` Drew Adams
  2012-12-12  6:34                         ` Kevin Rodgers
  0 siblings, 1 reply; 42+ messages in thread
From: Drew Adams @ 2012-12-12  3:42 UTC (permalink / raw)
  To: 'Stefan Monnier', 'Jambunathan K'; +Cc: 12638

[-- Attachment #1: Type: text/plain, Size: 656 bytes --]

> > I think appending, "[Complete, but not unique]????" should 
> > serve as a cue that one can simply RET.
> 
> You mean we'd display "foo[Complete but not unique]{bar | | baz}"?

FWIW, in my own code (icomplete+.el), I have a separate user option for the
string that is displayed to indicate an exact match, as opposed to doing this
(in icomplete.el):

(and most-is-exact
     (substring icomplete-separator
                (string-match "[^ ]" icomplete-separator)))

The default value of the option is (string ?\u2605 ?\  ), i.e., a star.  See
attached.  (The number 5 shown is the total number of completions.  In this
particular case all are shown.)

[-- Attachment #2: throw-icomplete-star.png --]
[-- Type: image/png, Size: 3757 bytes --]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-12  3:42                       ` Drew Adams
@ 2012-12-12  6:34                         ` Kevin Rodgers
  2012-12-12 16:15                           ` Drew Adams
  0 siblings, 1 reply; 42+ messages in thread
From: Kevin Rodgers @ 2012-12-12  6:34 UTC (permalink / raw)
  To: 12638

On 12/11/12 8:42 PM, Drew Adams wrote:
>>> I think appending, "[Complete, but not unique]????" should
>>> serve as a cue that one can simply RET.
>>
>> You mean we'd display "foo[Complete but not unique]{bar | | baz}"?
>
> FWIW, in my own code (icomplete+.el), I have a separate user option for the
> string that is displayed to indicate an exact match, as opposed to doing this
> (in icomplete.el):
>
> (and most-is-exact
>       (substring icomplete-separator
>                  (string-match "[^ ]" icomplete-separator)))
>
> The default value of the option is (string ?\u2605 ?\  ), i.e., a star.  See
> attached.  (The number 5 shown is the total number of completions.  In this
> particular case all are shown.)

Just curious: Why isn't the exact match (represented by the star) separated from 
the other matches with a vertical bar?

-- 
Kevin Rodgers
Denver, Colorado, USA






^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-12  6:34                         ` Kevin Rodgers
@ 2012-12-12 16:15                           ` Drew Adams
  0 siblings, 0 replies; 42+ messages in thread
From: Drew Adams @ 2012-12-12 16:15 UTC (permalink / raw)
  To: 'Kevin Rodgers', 12638

> > (and most-is-exact
> >       (substring icomplete-separator
> >                  (string-match "[^ ]" icomplete-separator)))
> >
> > The default value of the option is (string ?\u2605 ?\  ), 
> > i.e., a star.  See attached.  (The number 5 shown is the total
> > number of completions.  In this particular case all are shown.)
> 
> Just curious: Why isn't the exact match (represented by the 
> star) separated from the other matches with a vertical bar?

If you are asking me about my code and not the vanilla version, the answer is:
(a) I didn't think it would be clearer (but yes, someone could interpret it as
belonging only to the first alternative), (b) a user can easily include the bar
in the option value (string), and (c) the bar is not hard-coded, but is from the
other separator used between the other alternatives.

If you want an initial bar, you can add it easily.  If you want no bars at all,
you can easily have that.  If you want `spaghetti' for the first separator and
`lasagne' for the others, you can easily have that.

But I'm open to other suggestions.  I'm not in favor (for my code) of
hard-coding a connection between the two separators.






^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-04 15:51           ` Jambunathan K
@ 2012-12-13 13:51             ` Jambunathan K
  2012-12-17 16:28               ` Stefan Monnier
  2013-01-11  5:47               ` Jambunathan K
  0 siblings, 2 replies; 42+ messages in thread
From: Jambunathan K @ 2012-12-13 13:51 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]

Jambunathan K <kjambunathan@gmail.com> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> 2. M-x debbugs-gnu
>>>    There is no way I can C-s C-j to the second or third candidates.
>>>    I normally use debbbugs-gnu-search which is the second candidate.
>>
>> Better ask this elsewhere, I'm not up to speed on debbugs-gnu.
>
> I am attaching screenshot of minibuffer. 
>
>     1. M-x debbugs-gnu
>     2. Immediately cycle with C-s
>
> Cycling may happen internally but the UI presents the "most-is-exact"
> always at the head.  This is a definite bug.

The case to be handled is "Complete, but not unique".  

The attached patch fixes the issue by sneaking a Ctrl-M in to the try
try completion field.

Attachments: 
M-x debbugs-gnu RET with icomplete-mode on.

icomplete-1.png:  
        Note the ^M and the empty first entry

icomplete-2.png:  
        After a C-s.  Note the ^M indicator and empty entry pushed to
        the last.

icomplete-3.png:
        This is the second invocation of M-x debbugs-gnu RET.  Note that
        the entry from the recent history is at the head.  Also note ^M
        and the empty entries.


[-- Attachment #2: icomplete-bug12638.diff --]
[-- Type: text/x-diff, Size: 5337 bytes --]

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2012-12-13 05:29:15 +0000
+++ lisp/ChangeLog	2012-12-13 13:32:58 +0000
@@ -1,3 +1,10 @@
+2012-12-13  Jambunathan K  <kjambunathan@gmail.com>
+
+	* icomplete.el (icomplete-hide-common-prefix): New user variable.
+	(icomplete-first-match): New user face.
+	(icomplete-completions): Honor above settings.  Correctly handle
+	the case when NAME is complete but not unique (Bug#12638).
+
 2012-12-13  Alan Mackenzie  <acm@muc.de>
 
 	Make CC Mode not hang when _some_ lines end in CRLF.  Bug #11841.

=== modified file 'lisp/icomplete.el'
--- lisp/icomplete.el	2012-11-30 17:09:55 +0000
+++ lisp/icomplete.el	2012-12-13 13:16:31 +0000
@@ -76,6 +76,18 @@
   :type 'string
   :version "24.4")
 
+(defcustom icomplete-hide-common-prefix t
+  "When non-nil, hide common prefix from completion candidates.
+When nil, show candidates in full."
+  :type 'boolean
+  :version "24.4"
+  :group 'icomplete)
+
+(defface icomplete-first-match  '((t :weight bold))
+  "Face used by icomplete for highlighting first match."
+  :version "24.4"
+  :group 'icomplete)
+
 ;;;_* User Customization variables
 (defcustom icomplete-prospects-height
   ;; 20 is an estimated common size for the prompt + minibuffer content, to
@@ -332,14 +344,12 @@ are exhibited within the square braces.)
 	     (determ (unless (or (eq t compare) (eq t most-try)
 				 (= (setq compare (1- (abs compare)))
 				    (length most)))
-		       (concat open-bracket
 			       (cond
 				((= compare (length name))
                                  ;; Typical case: name is a prefix.
 				 (substring most compare))
 				((< compare 5) most)
-				(t (concat "..." (substring most compare))))
-			       close-bracket)))
+			(t (concat "..." (substring most compare))))))
 	     ;;"-prospects" - more than one candidate
 	     (prospects-len (+ (length determ) 6 ;; take {,...} into account
                                (string-width (buffer-string))))
@@ -350,6 +360,8 @@ are exhibited within the square braces.)
                     ;; one line, increase the allowable space accordingly.
                     (/ prospects-len (window-width)))
                  (window-width)))
+	     (prefix (when icomplete-hide-common-prefix
+		       (try-completion "" comps)))
              (prefix-len
               ;; Find the common prefix among `comps'.
 	      ;; We can't use the optimization below because its assumptions
@@ -359,37 +371,58 @@ are exhibited within the square braces.)
 	      ;;     ;; Common case.
 	      ;;     (length most)
 	      ;; Else, use try-completion.
-	      (let ((comps-prefix (try-completion "" comps)))
-		(and (stringp comps-prefix)
-		     (length comps-prefix)))) ;;)
-
+	      (and (stringp prefix) (length prefix))) ;;)
 	     prospects most-is-exact comp limit)
 	(if (eq most-try t) ;; (or (null (cdr comps))
 	    (setq prospects nil)
+	  (when (and (cdr comps) (member name comps))
+	    ;; NAME is complete but not unique.  This scenario, poses
+	    ;; following UI issues:
+	    ;;
+	    ;; - When common prefix is hidden, NAME is stripped empty.
+	    ;;   This would make the entry inconspicuous.
+	    ;; - Due to sorting of completions, NAME may not be the
+	    ;;   first of the prospects and could be hidden deep in the
+	    ;;   displayed string.
+	    ;; - Because of truncation, NAME may not even be displayed
+	    ;;   to the user.
+	    ;;
+	    ;; To circumvent all the above problems, provide a visual
+	    ;; cue to the user via try completion field.
+	    (setq determ "\r"))
 	  (while (and comps (not limit))
 	    (setq comp
 		  (if prefix-len (substring (car comps) prefix-len) (car comps))
 		  comps (cdr comps))
-	    (cond ((string-equal comp "") (setq most-is-exact t))
-		  ((member comp prospects))
-		  (t (setq prospects-len
+	    (setq prospects-len
                            (+ (string-width comp) 1 prospects-len))
 		     (if (< prospects-len prospects-max)
 			 (push comp prospects)
-		       (setq limit t))))))
+	      (setq limit t))))
+	(setq prospects (nreverse prospects))
+	;; Decorate the first of prospects.
+	(when prospects
+	  (let ((first (copy-sequence (pop prospects))))
+	    (put-text-property 0 (length first)
+			       'face 'icomplete-first-match first)
+	    (push first prospects)))
         ;; Restore the base-size info, since completion-all-sorted-completions
         ;; is cached.
         (if last (setcdr last base-size))
+	(when determ
+	  (setq determ (concat open-bracket determ close-bracket)))
 	(if prospects
 	    (concat determ
+		    (if (not (cdr prospects))
+			;; NAME is not complete but will complete to
+			;; an EXACT match on next try.
+			""
+		      (concat
 		    "{"
-		    (and most-is-exact
-                         (substring icomplete-separator
-                                    (string-match "[^ ]" icomplete-separator)))
-		    (mapconcat 'identity (nreverse prospects)
-                               icomplete-separator)
+		       (mapconcat 'identity prospects icomplete-separator)
 		    (and limit (concat icomplete-separator "…"))
-		    "}")
+		       "}"
+		       )))
 	  (concat determ " [Matched]"))))))
 
 ;;_* Local emacs vars.


[-- Attachment #3: icomplete-3.png --]
[-- Type: image/png, Size: 5177 bytes --]

[-- Attachment #4: icomplete-2.png --]
[-- Type: image/png, Size: 2134 bytes --]

[-- Attachment #5: icomplete-1.png --]
[-- Type: image/png, Size: 2081 bytes --]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-13 13:51             ` Jambunathan K
@ 2012-12-17 16:28               ` Stefan Monnier
  2012-12-17 19:22                 ` Jambunathan K
  2013-01-11  5:47               ` Jambunathan K
  1 sibling, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-12-17 16:28 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> The attached patch fixes the issue by sneaking a Ctrl-M in to the try
> try completion field.

That looks ugly and I don't find the ^M to be very intuitive.

Basically there are 2 options:
- Let "" be anywhere in the {...} list rather than only at the beginning.
  This would seem inconvenient if it's not compensated by some other
  "Complete, but not unique" marker.  Your ^M is one such option.
  We could also keep an empty () (or []), which is fairly logical and
  might be vaguely intuitive.
- Force "" to be at the beginning and have minibuffer-force-complete
  skip the empty completion, so the user would have to hit RET rather
  than C-j to select the exact text he typed.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-17 16:28               ` Stefan Monnier
@ 2012-12-17 19:22                 ` Jambunathan K
  2012-12-17 20:12                   ` Stefan Monnier
  0 siblings, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-12-17 19:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638


>> The attached patch fixes the issue by sneaking a Ctrl-M in to the try
>> try completion field.
>
> That looks ugly and I don't find the ^M to be very intuitive.
>
> Basically there are 2 options:
> - Let "" be anywhere in the {...} list rather than only at the beginning.
>   This would seem inconvenient if it's not compensated by some other
>   "Complete, but not unique" marker.  Your ^M is one such option.
>   We could also keep an empty () (or []), which is fairly logical and
>   might be vaguely intuitive.

My vote goes for option 1.

> - Force "" to be at the beginning and have minibuffer-force-complete
>   skip the empty completion, so the user would have to hit RET rather
>   than C-j to select the exact text he typed.





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-17 19:22                 ` Jambunathan K
@ 2012-12-17 20:12                   ` Stefan Monnier
  2012-12-17 20:58                     ` Jambunathan K
  0 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2012-12-17 20:12 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

>> - Let "" be anywhere in the {...} list rather than only at the beginning.
>> This would seem inconvenient if it's not compensated by some other
>> "Complete, but not unique" marker.  Your ^M is one such option.
>> We could also keep an empty () (or []), which is fairly logical and
>> might be vaguely intuitive.
> My vote goes for option 1.

This one has several sub-options.  Which representation for "Complete,
but not unique" do you favor then (taking into account that I don't
think ^M is going to fly)?


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-17 20:12                   ` Stefan Monnier
@ 2012-12-17 20:58                     ` Jambunathan K
  2012-12-18  1:26                       ` Stefan Monnier
  0 siblings, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2012-12-17 20:58 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> - Let "" be anywhere in the {...} list rather than only at the beginning.
>>> This would seem inconvenient if it's not compensated by some other
>>> "Complete, but not unique" marker.  Your ^M is one such option.
>>> We could also keep an empty () (or []), which is fairly logical and
>>> might be vaguely intuitive.
>> My vote goes for option 1.
>
> This one has several sub-options.  Which representation for "Complete,
> but not unique" do you favor then (taking into account that I don't
> think ^M is going to fly)?

I will just settle for whatever is chosen by you - empty space within
braces is good.

How about subtly fontifying the minibuffer contents itself. I mean the
whole `field-string', the string that is typed by the user which is
/not/ the display overlay.

We can also have:
    1. an empty space with some background fontification
    2. a non-breaking space 

Both (1) and (2) without surrouding `()'.

As a side note:

Drew suggested unicode '*'.  

I went with a control character because:
    1. It suggests what my fingers should do 
    2. Control characters are less like to occur in completions.

ps: Is ^M as a literal two character text more palatable than ^M as an
    escape glyph.





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-17 20:58                     ` Jambunathan K
@ 2012-12-18  1:26                       ` Stefan Monnier
  2012-12-18  3:09                         ` Drew Adams
  2012-12-18 14:40                         ` Jambunathan K
  0 siblings, 2 replies; 42+ messages in thread
From: Stefan Monnier @ 2012-12-18  1:26 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

> How about subtly fontifying the minibuffer contents itself.

The problem is that this can already be fontified by various other
options and packages, so there's a good chance that it will
interact poorly.  Also it might not work at all in a tty.

> We can also have:
>     1. an empty space with some background fontification
>     2. a non-breaking space
> Both (1) and (2) without surrounding `()'.

An empty space is actually not a bad idea.  I also considered
a checkmark ✓, but a space is simple and works everywhere.

> Drew suggested unicode '*'.

Yes, that could work as well.  Also he suggested putting it within the
{...} rather than before it.

> I went with a control character because:
>     1. It suggests what my fingers should do

Aaaaahhhhhh!!!!!! That's where it came from!!  OK, it makes more sense, now.
So, there is logic to it, but I completely missed it.

> ps: Is ^M as a literal two character text more palatable than ^M as an
>     escape glyph.

Same difference, since it's only the visual appearance that
matters here.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-18  1:26                       ` Stefan Monnier
@ 2012-12-18  3:09                         ` Drew Adams
  2012-12-18 14:40                         ` Jambunathan K
  1 sibling, 0 replies; 42+ messages in thread
From: Drew Adams @ 2012-12-18  3:09 UTC (permalink / raw)
  To: 'Stefan Monnier', 'Jambunathan K'; +Cc: 12638

> An empty space is actually not a bad idea.  I also considered
> a checkmark ?, but a space is simple and works everywhere.
> 
> > Drew suggested unicode '*'.

Actually, it's a user option, with Unicode star (not asterisk) as the default
value.  I experimented with Unicode checkmark also.  In principle it is the
clearest symbol, in terms of conveying the meaning of completeness.  But it
didn't look so good (not so clear visually), at least with the font that I
tried.  My defcustom has these commented lines indicating other symbols that
might be useful:

;; (string ?\u2714 ?\  ) ; check mark
;; (string ?\u29eb ?\  ) ; diamond
;; (string ?\u2205 ?\  ) ; empty set

The empty set too has good meaning here, but the Unicode symbol is tiny and
unclear (and some people might confuse it with zero if unfamiliar with the
symbol).  An alternative empty-set representation is of course {}, but braces
are already used for another purpose here.

> Yes, that could work as well.  Also he suggested putting it within the
> {...} rather than before it.
> 
> > I went with a control character because:
> >     1. It suggests what my fingers should do
> 
> Aaaaahhhhhh!!!!!! That's where it came from!!  OK, it makes 
> more sense, now.  So, there is logic to it, but I completely missed it.
> 
> > ps: Is ^M as a literal two character text more palatable 
> > than ^M as an escape glyph.
> 
> Same difference, since it's only the visual appearance that
> matters here.

Except that they do not have the same visual appearance.  The latter uses face
`escape-glyph', no?  Of course that could be overridden.

I would advise against confusing the symbol for complete-but-not-unique with
^M/RET as the key for accepting the value.  The fact that many users are not
aware that ^M and RET (the Enter key) are at all related is only one reason.
Another reason is that RET is always possible - it just behaves differently if
the input is already complete.

There is one advantage to using a control char (or its text representation):
users are unlikely to think that it is a char that is part of a completion
candidate.  Although not common, it is possible for a Unicode star or whatever
to be part of a completion candidate.  I discounted this problem as unimportant,
but it is at least real.  (The same is true for any character, however,
including control-M.)

FWIW, another reason I went with Unicode star as the default was that in my case
the code should work for pre-Unicode Emacs versions, and in that case the
default is an ordinary asterisk - related to the star.  That reason won't apply
to icomplete.el, of course.






^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-18  1:26                       ` Stefan Monnier
  2012-12-18  3:09                         ` Drew Adams
@ 2012-12-18 14:40                         ` Jambunathan K
  1 sibling, 0 replies; 42+ messages in thread
From: Jambunathan K @ 2012-12-18 14:40 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> We can also have:
>>     1. an empty space with some background fontification
>>     2. a non-breaking space
>> Both (1) and (2) without surrounding `()'.
>
> An empty space is actually not a bad idea.  I also considered
> a checkmark ✓, but a space is simple and works everywhere.

OK, let's go with `()'.  If we need filler characters let us sneak a
full-stop or a dot operator in between.





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-12-13 13:51             ` Jambunathan K
  2012-12-17 16:28               ` Stefan Monnier
@ 2013-01-11  5:47               ` Jambunathan K
  2013-01-11 14:17                 ` Stefan Monnier
  1 sibling, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2013-01-11  5:47 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

Stefan

> The case to be handled is "Complete, but not unique".  

You need to take some action on this issue.

icomplete.el has marched ahead atleast by two versions, since I floated
these changes.

My only concern is that, I am totally confused about what I should do
with the patch.  (I am no reader of your mind - Will you re-work it
yourself or apply it as such etc.)

I desire (or rather insist!) a firm closure to this stuff.
-- 





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2013-01-11  5:47               ` Jambunathan K
@ 2013-01-11 14:17                 ` Stefan Monnier
  2013-02-13 13:56                   ` Jambunathan K
  0 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2013-01-11 14:17 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

I'd like to go for the () annotation, but I'm pretty swamped, so
progress is slow.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2013-01-11 14:17                 ` Stefan Monnier
@ 2013-02-13 13:56                   ` Jambunathan K
  2013-02-13 15:15                     ` Stefan Monnier
  0 siblings, 1 reply; 42+ messages in thread
From: Jambunathan K @ 2013-02-13 13:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638

[-- Attachment #1: Type: text/plain, Size: 139 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I'd like to go for the () annotation, but I'm pretty swamped, so
> progress is slow.


[-- Attachment #2: icomplete.el.diff --]
[-- Type: text/plain, Size: 4989 bytes --]

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2013-02-13 13:40:00 +0000
+++ lisp/ChangeLog	2013-02-13 13:52:00 +0000
@@ -1,3 +1,10 @@
+2013-02-13  Jambunathan K  <kjambunathan@gmail.com>
+
+	* icomplete.el (icomplete-hide-common-prefix):  New user option.
+	(icomplete-first-match): New face.
+	(icomplete-completions): Correct handling of "complete but not
+	unique" (Bug#12638).
+
 2013-02-13  Stefan Monnier  <monnier@iro.umontreal.ca>
 
 	* tmm.el: Use lexical-binding and current-active-maps.

=== modified file 'lisp/icomplete.el'
--- lisp/icomplete.el	2013-02-08 07:53:55 +0000
+++ lisp/icomplete.el	2013-02-13 13:31:04 +0000
@@ -76,6 +76,18 @@
   :type 'string
   :version "24.4")
 
+(defcustom icomplete-hide-common-prefix t
+  "When non-nil, hide common prefix from completion candidates.
+When nil, show candidates in full."
+  :type 'boolean
+  :version "24.4"
+  :group 'icomplete)
+
+(defface icomplete-first-match  '((t :weight bold))
+  "Face used by icomplete for highlighting first match."
+  :version "24.4"
+  :group 'icomplete)
+
 ;;;_* User Customization variables
 (defcustom icomplete-prospects-height
   ;; 20 is an estimated common size for the prompt + minibuffer content, to
@@ -344,7 +356,8 @@ are exhibited within the square braces.)
 				(t (concat "…" (substring most compare))))
 			       close-bracket)))
 	     ;;"-prospects" - more than one candidate
-	     (prospects-len (+ (length determ)
+	     (prospects-len (+ (string-width
+				(or determ (concat open-bracket close-bracket)))
 			       (string-width icomplete-separator)
 			       3 ;; take {…} into account
 			       (string-width (buffer-string))))
@@ -355,6 +368,8 @@ are exhibited within the square braces.)
                     ;; one line, increase the allowable space accordingly.
                     (/ prospects-len (window-width)))
                  (window-width)))
+	     (prefix (when icomplete-hide-common-prefix
+		       (try-completion "" comps)))
              (prefix-len
               ;; Find the common prefix among `comps'.
 	      ;; We can't use the optimization below because its assumptions
@@ -364,37 +379,55 @@ are exhibited within the square braces.)
 	      ;;     ;; Common case.
 	      ;;     (length most)
 	      ;; Else, use try-completion.
-	      (let ((comps-prefix (try-completion "" comps)))
-		(and (stringp comps-prefix)
-		     (length comps-prefix)))) ;;)
-
-	     prospects most-is-exact comp limit)
+	      (and (stringp prefix) (length prefix))) ;;)
+	     prospects comp limit)
 	(if (eq most-try t) ;; (or (null (cdr comps))
 	    (setq prospects nil)
+	  (when (member name comps)
+	    ;; NAME is complete but not unique.  This scenario poses
+	    ;; following UI issues:
+	    ;;
+	    ;; - When `icomplete-hide-common-prefix' is non-nil, NAME
+	    ;;   is stripped empty.  This would make the entry
+	    ;;   inconspicuous.
+	    ;;
+	    ;; - Due to sorting of completions, NAME may not be the
+	    ;;   first of the prospects and could be hidden deep in
+	    ;;   the displayed string.
+	    ;;
+	    ;; - Because of `icomplete-prospects-height' , NAME may
+	    ;;   not even be displayed to the user.
+	    ;;
+	    ;; To circumvent all the above problems, provide a visual
+	    ;; cue to the user via an "empty string" in the try
+	    ;; completion field.
+	    (setq determ (concat open-bracket "" close-bracket)))
+	  ;; Compute prospects for display.
 	  (while (and comps (not limit))
 	    (setq comp
 		  (if prefix-len (substring (car comps) prefix-len) (car comps))
 		  comps (cdr comps))
-	    (cond ((string-equal comp "") (setq most-is-exact t))
-		  ((member comp prospects))
-		  (t (setq prospects-len
+	    (setq prospects-len
                            (+ (string-width comp)
 			      (string-width icomplete-separator)
 			      prospects-len))
 		     (if (< prospects-len prospects-max)
 			 (push comp prospects)
-		       (setq limit t))))))
+	      (setq limit t))))
+	(setq prospects (nreverse prospects))
+	;; Decorate first of the prospects.
+	(when prospects
+	  (let ((first (copy-sequence (pop prospects))))
+	    (put-text-property 0 (length first)
+			       'face 'icomplete-first-match first)
+	    (push first prospects)))
         ;; Restore the base-size info, since completion-all-sorted-completions
         ;; is cached.
         (if last (setcdr last base-size))
 	(if prospects
 	    (concat determ
 		    "{"
-		    (and most-is-exact
-                         (substring icomplete-separator
-                                    (string-match "[^ ]" icomplete-separator)))
-		    (mapconcat 'identity (nreverse prospects)
-                               icomplete-separator)
+		    (mapconcat 'identity prospects icomplete-separator)
 		    (and limit (concat icomplete-separator "…"))
 		    "}")
 	  (concat determ " [Matched]"))))))


[-- Attachment #3: Type: text/plain, Size: 4 bytes --]

-- 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2013-02-13 13:56                   ` Jambunathan K
@ 2013-02-13 15:15                     ` Stefan Monnier
  2013-02-13 17:18                       ` Jambunathan K
  0 siblings, 1 reply; 42+ messages in thread
From: Stefan Monnier @ 2013-02-13 15:15 UTC (permalink / raw)
  To: Jambunathan K; +Cc: 12638

>> I'd like to go for the () annotation, but I'm pretty swamped, so
>> progress is slow.

> [2. icomplete.el.diff --- text/plain]

Thank you, installed.  BTW, how 'bout we give you write access so you
can commit those things yourself?  If that sounds good to you, then log
into your account on savannah and request membership in the `emacs' group.


        Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2013-02-13 15:15                     ` Stefan Monnier
@ 2013-02-13 17:18                       ` Jambunathan K
  0 siblings, 0 replies; 42+ messages in thread
From: Jambunathan K @ 2013-02-13 17:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 12638


This bug can now be closed.

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> I'd like to go for the () annotation, but I'm pretty swamped, so
>>> progress is slow.
>
>> [2. icomplete.el.diff --- text/plain]
>
> Thank you, installed.  BTW, how 'bout we give you write access so you
> can commit those things yourself?  If that sounds good to you, then log
> into your account on savannah and request membership in the `emacs' group.

Thank you for the invitation.  I am honored :-) 

Let me pop in soon.

>         Stefan





^ permalink raw reply	[flat|nested] 42+ messages in thread

* bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode
  2012-10-13 17:33 bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode Jambunathan K
                   ` (3 preceding siblings ...)
  2012-11-29 21:34 ` Stefan Monnier
@ 2013-11-15  4:42 ` Jambunathan K
  4 siblings, 0 replies; 42+ messages in thread
From: Jambunathan K @ 2013-11-15  4:42 UTC (permalink / raw)
  To: 12638-done


OP here.  Closed.





^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2013-11-15  4:42 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-13 17:33 bug#12638: 24.2.50; FR: Some suggestions for icomplete-mode Jambunathan K
2012-10-13 22:14 ` Stefan Monnier
2012-10-14  4:18 ` Drew Adams
2012-10-23 19:43 ` Stefan Monnier
2012-10-23 20:08   ` Jambunathan K
2012-10-23 20:17     ` Jambunathan K
2012-10-24 13:09     ` Stefan Monnier
2012-11-02 11:49       ` Jambunathan K
2012-11-02 12:12         ` Jambunathan K
2012-11-09  1:53           ` Stefan Monnier
2012-11-09  2:17         ` Stefan Monnier
2012-11-09  4:25           ` Jambunathan K
2012-11-09 14:12             ` Stefan Monnier
2012-11-29 21:34 ` Stefan Monnier
2012-11-30  6:18   ` Jambunathan K
2012-11-30 19:37     ` Stefan Monnier
2012-12-04 12:54       ` Jambunathan K
2012-12-04 15:02         ` Stefan Monnier
2012-12-04 15:30           ` Jambunathan K
2012-12-04 15:45             ` Stefan Monnier
2012-12-04 16:12               ` Jambunathan K
2012-12-04 17:14                 ` Stefan Monnier
2012-12-04 17:32                   ` Jambunathan K
2012-12-12  3:18                     ` Stefan Monnier
2012-12-12  3:42                       ` Drew Adams
2012-12-12  6:34                         ` Kevin Rodgers
2012-12-12 16:15                           ` Drew Adams
2012-12-04 15:51           ` Jambunathan K
2012-12-13 13:51             ` Jambunathan K
2012-12-17 16:28               ` Stefan Monnier
2012-12-17 19:22                 ` Jambunathan K
2012-12-17 20:12                   ` Stefan Monnier
2012-12-17 20:58                     ` Jambunathan K
2012-12-18  1:26                       ` Stefan Monnier
2012-12-18  3:09                         ` Drew Adams
2012-12-18 14:40                         ` Jambunathan K
2013-01-11  5:47               ` Jambunathan K
2013-01-11 14:17                 ` Stefan Monnier
2013-02-13 13:56                   ` Jambunathan K
2013-02-13 15:15                     ` Stefan Monnier
2013-02-13 17:18                       ` Jambunathan K
2013-11-15  4:42 ` Jambunathan K

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).