bug#4219: 23.1; case insensitive + partial completions

bug#4219: 23.1; case insensitive + partial completions

[Eli Barzilay]

Start with a default Emacs, and

  (setq read-file-name-completion-ignore-case t)
  (setq completion-styles '(partial-completion))

Now go to a directory that has two files called

  INSTALL
  install-sh

Hit `C-x C-f ins TAB' -- it will be completed to "insTALL".


In GNU Emacs 23.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.10.14)
 of 2009-08-01 on winooski.ccs.neu.edu
Windowing system distributor `The X.Org Foundation', version 11.0.10300000
configured using `configure  '--prefix=/home/eli/bin/local/emacs-dir''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: POSIX
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US
  value of $XMODIFIERS: nil
  locale-coding-system: iso-latin-1-unix
  default-enable-multibyte-characters: t

Major mode: Dired by name

Minor modes in effect:
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

bug#4219: 23.1; case insensitive + partial completions

[Chong Yidong]

> Start with a default Emacs, and
>
>   (setq read-file-name-completion-ignore-case t)
>   (setq completion-styles '(partial-completion))
>
> Now go to a directory that has two files called
>
>   INSTALL
>   install-sh
>
> Hit `C-x C-f ins TAB' -- it will be completed to "insTALL".

Stefan, could you take a look at this?  I glanced through your partial
completion code, and it's clear where the problem arises.  In
completion-pcm--merge-try,

  (completion-pcm--merge-completions all pattern)

returns

  (all "TALL" "ins")

and so completion-pcm--pattern->string naively joins the result into
"insTALL".  However, I don't know how to fix this.  The default
completion style actually does the right thing, because it can just
replace the string wholesale, but I'm not sure if the pcm style can do
the same thing.

bug#4219: 23.1; case insensitive + partial completions

[Stefan Monnier]

>>>>> "Eli" == Eli Barzilay <eli@barzilay.org> writes:

> Start with a default Emacs, and
>   (setq read-file-name-completion-ignore-case t)
>   (setq completion-styles '(partial-completion))

> Now go to a directory that has two files called

>   INSTALL
>   install-sh

> Hit `C-x C-f ins TAB' -- it will be completed to "insTALL".

I think the patch below will help out.  You may still get such
inconsistent results in different parts of a completion (e.g. completing
"fo-ba" against "FOO-BAR" and "foo-bar" may return "FOO-bar"), but at
least the above case should be handled better.


        Stefan


--- minibuffer.el.~1.83.~	2009-09-02 20:35:02.000000000 -0400
+++ minibuffer.el	2009-09-13 23:31:10.000000000 -0400
@@ -1670,28 +1670,32 @@
           (unless (string-match re str)
             (error "Internal error: %s doesn't match %s" str re))
           (let ((chopped ())
-                (i 1))
-            (while (match-beginning i)
-              (push (match-string i str) chopped)
+                (last 0)
+                (i 1)
+                next)
+            (while (setq next (match-end i))
+              (push (substring str last next) chopped)
+              (setq last next)
               (setq i (1+ i)))
             ;; Add the text corresponding to the implicit trailing `any'.
-            (push (substring str (match-end 0)) chopped)
+            (push (substring str last) chopped)
             (push (nreverse chopped) ccs))))
 
       ;; Then for each of those non-constant elements, extract the
       ;; commonality between them.
-      (let ((res ()))
-        ;; Make the implicit `any' explicit.  We could make it explicit
+      (let ((res ())
+            (fixed ""))
+        ;; Make the implicit trailing `any' explicit.  We could make it explicit
         ;; everywhere, but it would slow down regexp-matching a little bit.
         (dolist (elem (append pattern '(any)))
           (if (stringp elem)
-              (push elem res)
+              (setq fixed (concat fixed elem))
             (let ((comps ()))
               (dolist (cc (prog1 ccs (setq ccs nil)))
                 (push (car cc) comps)
                 (push (cdr cc) ccs))
-              (let* ((prefix (try-completion "" comps))
-                     (unique (or (and (eq prefix t) (setq prefix ""))
+              (let* ((prefix (try-completion fixed comps))
+                     (unique (or (and (eq prefix t) (setq prefix fixed))
                                  (eq t (try-completion prefix comps)))))
                 (unless (equal prefix "") (push prefix res))
                 ;; If there's only one completion, `elem' is not useful
@@ -1700,7 +1704,8 @@
                 ;; `any' into a `star' because the surrounding context has
                 ;; changed such that string->pattern wouldn't add an `any'
                 ;; here any more.
-                (unless unique (push elem res))))))
+                (unless unique (push elem res))
+                (setq fixed "")))))
         ;; We return it in reverse order.
         res)))))
 

bug#4219: 23.1; case insensitive + partial completions

[Eli Barzilay]

On Sep 13, Stefan Monnier wrote:
> >>>>> "Eli" == Eli Barzilay <eli@barzilay.org> writes:
> 
> >   INSTALL
> >   install-sh
> 
> > Hit `C-x C-f ins TAB' -- it will be completed to "insTALL".
> 
> I think the patch below will help out.  You may still get such
> inconsistent results in different parts of a completion
> (e.g. completing "fo-ba" against "FOO-BAR" and "foo-bar" may return
> "FOO-bar"), but at least the above case should be handled better.

I had a hacked version of `PC-do-completion' which worked until v23
came out that did a proper case-insensitive match.  In my version I'd
never get "FOO-bar" for the above.  Would it help to look at it?

(It's been a while since I did it, and all I have a single comment at
the top of that change saying: "If there are possibility prefixes that
match the basestr exactly then replace the basestr part of prefix by
the apropriate one".  So I'm not sure if this can be helpful or not.)

-- 
          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:
                    http://barzilay.org/                   Maze is Life!