bug#12615: 24.2.50; Non-ignored case in insert-char

classic Classic list List threaded Threaded
17 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Harald Hanche-Olsen
Starting with emacs -Q:

Evaluate the following in the *scratch* buffer:
(make-local-variable 'completion-ignore-case)
Then type: C-x 8 C-m a TAB

Expected result:
  A completion list of unicode names starting with the letter A.
Actual result:
  [no match]

Some observations: C-x 8 C-m is bound to insert-char, a C function.
And insert-char calls read-char-by-name, which let-binds
completion-ignore-case to t. The clear intention is that unicode name
searches should always be case insensitive, and this seems always to
be the case if completion-ignore-case is not buffer local.

Setting completion-ignore-case to t in the *scratch* buffer still does
not help.

In GNU Emacs 24.2.50.1 (x86_64-apple-darwin11.4.0, NS apple-appkit-1138.47)
 of 2012-09-24 on airy
Bzr revision: 110175 [hidden email]-20120924063102-7nllu1xpqi4f24n7
Windowing system distributor `Apple', version 10.3.1138
Configured using:
 `configure '--with-ns''

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

I have also seen this on a quite recent emacs on x86_64-unknown-linux-gnu.

- Harald



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Harald Hanche-Olsen
Thinking about this on my way home from work, I think I figured out
the reason for this problem:

The let binding of completion-ignore-case takes place in the current
buffer, and so overrides the buffer-local binding. However, the
variable is used in the minibuffer, in which the buffer-local binding
(and with it, the let binding) is not visible; it uses the global
binding instead.

I am not totally sure about this; however, a simple experiment shows
that buffer-local bindings and let bindings interact in the way
described.

If I am right, then making completion-ignore-case buffer-local makes
no sense, as it can't have any effect. (If you're curious, mew does
this. I should contact the author to find out why.)

As to whether this is an emacs bug, I am no longer sure.

But I can't find anything in the elisp manual about how let operates
on buffer-local variables.

- Harald



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Andreas Schwab-2
Harald Hanche-Olsen <[hidden email]> writes:

> But I can't find anything in the elisp manual about how let operates
> on buffer-local variables.

*Note (elisp) Intro to Buffer-Local::

Andreas.

--
Andreas Schwab, [hidden email]
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Glenn Morris-3
In reply to this post by Harald Hanche-Olsen
Harald Hanche-Olsen wrote:

> If I am right, then making completion-ignore-case buffer-local makes
> no sense, as it can't have any effect. (If you're curious, mew does
> this. I should contact the author to find out why.)

I notice that progmodes/idlwave.el and progmodes/idlw-shell.el in Emacs
do this too.

> But I can't find anything in the elisp manual about how let operates
> on buffer-local variables.

As Andreas said, it has an explicit example of this kind of issue with a
bold "Warning" notice.

http://www.gnu.org/software/emacs/manual/html_node/elisp/Intro-to-Buffer_002dLocal.html



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Stefan Monnier
In reply to this post by Harald Hanche-Olsen
> Some observations: C-x 8 C-m is bound to insert-char, a C function.
> And insert-char calls read-char-by-name, which let-binds
> completion-ignore-case to t.  The clear intention is that unicode name
> searches should always be case insensitive, and this seems always to
> be the case if completion-ignore-case is not buffer local.

Indeed, a buffer-local setting of completion-ignore-case can
bring surprises.  I think the patch below will fix this problem for this
particular case.


        Stefan


=== modified file 'lisp/international/mule-cmds.el'
--- lisp/international/mule-cmds.el 2012-09-25 18:47:18 +0000
+++ lisp/international/mule-cmds.el 2012-10-11 00:54:27 +0000
@@ -2958,13 +2958,14 @@
 This function also accepts a hexadecimal number of Unicode code
 point or a number in hash notation, e.g. #o21430 for octal,
 #x2318 for hex, or #10r8984 for decimal."
-  (let* ((completion-ignore-case t)
- (input (completing-read
+  (let ((input
+         (completing-read
                  prompt
                  (lambda (string pred action)
+            (let ((completion-ignore-case t))
                    (if (eq action 'metadata)
                        '(metadata (category . unicode-name))
-                     (complete-with-action action (ucs-names) string pred))))))
+                (complete-with-action action (ucs-names) string pred)))))))
     (cond
      ((string-match-p "\\`[0-9a-fA-F]+\\'" input)
       (string-to-number input 16))




Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Harald Hanche-Olsen
In reply to this post by Glenn Morris-3
[Glenn Morris <[hidden email]> (2012-10-10 21:31:02 UTC)]

> Harald Hanche-Olsen wrote:
>
> > But I can't find anything in the elisp manual about how let operates
> > on buffer-local variables.
>
> As Andreas said, it has an explicit example of this kind of issue with a
> bold "Warning" notice.

Ah, so it does. How on earth did I miss that? My guess: The anemic
scroll bars on the Mac making me think I was seeing the whole buffer.

- Harald



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Harald Hanche-Olsen
In reply to this post by Stefan Monnier
[Stefan Monnier <[hidden email]> (2012-10-11 00:56:00 UTC)]

> Indeed, a buffer-local setting of completion-ignore-case can
> bring surprises.  I think the patch below will fix this problem for this
> particular case.

Indeed it does. I think that patch is worth committing.

- Harald



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Stefan Monnier
>> Indeed, a buffer-local setting of completion-ignore-case can
>> bring surprises.  I think the patch below will fix this problem for this
>> particular case.
> Indeed it does.

Thanks, installed,


        Stefan



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Juri Linkov
In reply to this post by Stefan Monnier
>> Some observations: C-x 8 C-m is bound to insert-char, a C function.
>> And insert-char calls read-char-by-name, which let-binds
>> completion-ignore-case to t.  The clear intention is that unicode name
>> searches should always be case insensitive, and this seems always to
>> be the case if completion-ignore-case is not buffer local.
>
> Indeed, a buffer-local setting of completion-ignore-case can
> bring surprises.  I think the patch below will fix this problem for this
> particular case.

I just stumbled upon the case where this fix causes the regression:
typing `C-x 8 RET *acc TAB' results in "[No match]".

I don't know why this case disobeys the let-binding of
`completion-ignore-case' and whether a simpler fix is possible,
but at least this patch fixes it for the emacs-24 branch:

=== modified file 'lisp/international/mule-cmds.el'
--- lisp/international/mule-cmds.el 2012-10-11 20:05:47 +0000
+++ lisp/international/mule-cmds.el 2012-12-29 22:45:12 +0000
@@ -2946,10 +2946,12 @@ (defun read-char-by-name (prompt)
 point or a number in hash notation, e.g. #o21430 for octal,
 #x2318 for hex, or #10r8984 for decimal."
   (let ((input
+         (minibuffer-with-setup-hook
+     (lambda ()
+       (set (make-local-variable 'completion-ignore-case) t))
          (completing-read
           prompt
           (lambda (string pred action)
-            (let ((completion-ignore-case t))
               (if (eq action 'metadata)
                   '(metadata (category . unicode-name))
                 (complete-with-action action (ucs-names) string pred)))))))




Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Juri Linkov
>>> Some observations: C-x 8 C-m is bound to insert-char, a C function.
>>> And insert-char calls read-char-by-name, which let-binds
>>> completion-ignore-case to t.  The clear intention is that unicode name
>>> searches should always be case insensitive, and this seems always to
>>> be the case if completion-ignore-case is not buffer local.
>>
>> Indeed, a buffer-local setting of completion-ignore-case can
>> bring surprises.  I think the patch below will fix this problem for this
>> particular case.
>
> I just stumbled upon the case where this fix causes the regression:
> typing `C-x 8 RET *acc TAB' results in "[No match]".
>
> I don't know why this case disobeys the let-binding of
> `completion-ignore-case' and whether a simpler fix is possible,
> but at least this patch fixes it for the emacs-24 branch:

Please ignore this patch.  Just search the source tree with
grep "completion-ignore-case t" and see the remaining 100 places
that have exactly the same problem.  Take for example the first grep
hit in bookmark.el.  Evaluate the following in the *scratch* buffer:
(make-local-variable 'completion-ignore-case)
Then type `C-x r b' (`bookmark-jump')
followed by a lower-case letter and TAB.
Completion is not case insensitive.

Maybe to fix all them at once, `read_minibuf' should make
a local variable `completion-ignore-case' in the minibuffer
and copy its value from the original buffer?



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Juri Linkov
In light of the incoming pretest I propose to fix the regression now in
the emacs-24 branch (the test case for the regression is `C-x 8 RET *acc TAB')
with the patch below, and later think about another fix for trunk
(I still have no idea for a better fix) that doesn't cause a regression.

=== modified file 'lisp/international/mule-cmds.el'
--- lisp/international/mule-cmds.el 2013-01-01 09:11:05 +0000
+++ lisp/international/mule-cmds.el 2013-01-06 00:19:17 +0000
@@ -2945,14 +2945,13 @@ (defun read-char-by-name (prompt)
 This function also accepts a hexadecimal number of Unicode code
 point or a number in hash notation, e.g. #o21430 for octal,
 #x2318 for hex, or #10r8984 for decimal."
-  (let ((input
-         (completing-read
+  (let* ((completion-ignore-case t)
+ (input (completing-read
           prompt
           (lambda (string pred action)
-            (let ((completion-ignore-case t))
               (if (eq action 'metadata)
                   '(metadata (category . unicode-name))
-                (complete-with-action action (ucs-names) string pred)))))))
+                     (complete-with-action action (ucs-names) string pred))))))
     (cond
      ((string-match-p "\\`[0-9a-fA-F]+\\'" input)
       (string-to-number input 16))




Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Stefan Monnier
In reply to this post by Juri Linkov
> Maybe to fix all them at once, `read_minibuf' should make
> a local variable `completion-ignore-case' in the minibuffer
> and copy its value from the original buffer?

That might be a good idea.
I'd guess only completing-read would need to do it, rather than
read_minibuf.

Tho we might still want to make it possible to specify case-irrelevance
in the completion table itself.


        Stefan



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Juri Linkov
>> Maybe to fix all them at once, `read_minibuf' should make
>> a local variable `completion-ignore-case' in the minibuffer
>> and copy its value from the original buffer?
>
> That might be a good idea.
> I'd guess only completing-read would need to do it, rather than
> read_minibuf.

Then `completing-read-default' would be a good place too
(anyone who overrides `completing-read-function' have to copy most of
code from `completing-read-default' anyway).

> Tho we might still want to make it possible to specify case-irrelevance
> in the completion table itself.

I suppose you mean specifying `completion-ignore-case' in `metadata'.
This could help to avoid problems with buffer-local bindings of
`completion-ignore-case'.



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Stefan Monnier
>> Tho we might still want to make it possible to specify case-irrelevance
>> in the completion table itself.
> I suppose you mean specifying `completion-ignore-case' in `metadata'.

I didn't intend to specify a particular way to do it.  It could be via
the `metadata', indeed.  But in Emacs<23 it was done simply be having
try-completion and all-completions ignore the case, without having to
tell anyone else.  If this "old style" (i.e. the style currently used in
read-char-by-name) can be made to work, that'd be even better.


        Stefan



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Juri Linkov
In reply to this post by Juri Linkov
reopen 12615
thanks

>> I'd guess only completing-read would need to do it, rather than
>> read_minibuf.
>
> Then `completing-read-default' would be a good place too
> (anyone who overrides `completing-read-function' have to copy most of
> code from `completing-read-default' anyway).

I tried to do this in `completing-read-default', and it seems to fix the
reported problem, and (make-local-variable 'completion-ignore-case) in
the *scratch* buffer doesn't override the let-binding in `read-char-by-name':

=== modified file 'lisp/minibuffer.el'
--- lisp/minibuffer.el 2013-01-03 00:36:36 +0000
+++ lisp/minibuffer.el 2013-01-10 00:23:05 +0000
@@ -3202,8 +3202,16 @@ (defun completing-read-default (prompt c
                     ;; in minibuffer-local-filename-completion-map can
                     ;; override bindings in base-keymap.
                     base-keymap)))
-         (result (read-from-minibuffer prompt initial-input keymap
-                                       nil hist def inherit-input-method)))
+         ;; Get the value of `completion-ignore-case' from the original
+         ;; buffer where it is either buffer-local or let-bound.
+         (c-i-c completion-ignore-case)
+         (result
+          (minibuffer-with-setup-hook
+              (lambda ()
+                ;; Copy the value from original buffer to the minibuffer.
+                (set (make-local-variable 'completion-ignore-case) c-i-c))
+            (read-from-minibuffer prompt initial-input keymap
+                                  nil hist def inherit-input-method))))
     (when (and (equal result "") def)
       (setq result (if (consp def) (car def) def)))
     result))




Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Stefan Monnier
> I tried to do this in `completing-read-default', and it seems to fix the
> reported problem, and (make-local-variable 'completion-ignore-case) in
> the *scratch* buffer doesn't override the let-binding in `read-char-by-name':

Let's keep it around as a possible fallback workaround, but I'd rather
avoid using it if possible.


        Stefan



Reply | Threaded
Open this post in threaded view
|

bug#12615: 24.2.50; Non-ignored case in insert-char

Lars Ingebrigtsen
Stefan Monnier <[hidden email]> writes:

>> I tried to do this in `completing-read-default', and it seems to fix the
>> reported problem, and (make-local-variable 'completion-ignore-case) in
>> the *scratch* buffer doesn't override the let-binding in `read-char-by-name':
>
> Let's keep it around as a possible fallback workaround, but I'd rather
> avoid using it if possible.

Amazingly enough, the patch still applies seven years later, and it
still fixes the bug reported by Harald.

It is a bit of a hack, but...  Any further opinions?  I'm in favour of
applying (after modernising a teensy bit).

--
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no