bug#28439: suggestion: support case-independent xref-find-definitions prompt TAB

bug#28439: suggestion: support case-independent xref-find-definitions prompt TAB

[Winston]

xref-find-definitions UI suggestion:

   Since xref-find-definitions is willing to match names typed in all
   lower case to mixed case names, TAB (show alternatives) should, too.
   If there's not already a way to make TAB do so, then a way should be
   added.

Discussion:

   Suppose I have Foo1() and Foo2().  Entering "foo1" or "foo2" at the
   prompt works if the name is complete and unique, even though I used
   all lower case.  TAB, however will only show alternatives if I've
   typed "Foo" TAB, while "foo" TAB will say "No matches".

   While it's obvious why the code works that way, I claim users are
   going to expect TAB to offer completions for any string with which
   xref-find-definitions would succeed, and thus that TAB should be
   capable of presenting a less case-dependent alternatives list.

   I was surprised when it didn't.  I had typed in enough of a tag name
(in all lower case) to match uniquely, and then typed TAB expecting the
single completion (perhaps in the prompt line itself rather than a
separate window, possibly with the string I had entered being converted
to its correct mixed case), and was surprised when it said No match.

   This is also another difference compared to find-tag, where one only
needs to uniquely partial match a name to get to it.  With
xref-find-definitions, I can't go to the partial name and TAB doesn't
complete the name even when there's only one match (because the typed
string had the wrong case), so it takes more typing (or correct use of
the shift key) than find-tag did.
 -WBE

bug#28439: suggestion: support case-independent xref-find-definitions prompt TAB

[Stephen Berman]

On Tue, 12 Sep 2017 18:37 EDT Winston <wbe@psr.com> wrote:

> xref-find-definitions UI suggestion:
>
>    Since xref-find-definitions is willing to match names typed in all
>    lower case to mixed case names, TAB (show alternatives) should, too.
>    If there's not already a way to make TAB do so, then a way should be
>    added.

There is: (setq completion-ignore-case t)

Steve Berman

bug#28439: suggestion: support case-independent xref-find-definitions

[Winston]

Steve Berman kindly replied:
> There is: (setq completion-ignore-case t)

Wow, that was easy, and it does just what I described.

Thanks!
 -WBE

[You're welcome to close this suggestion/bug report.]

bug#28439: suggestion: support case-independent xref-find-definitions prompt TAB

[Eli Zaretskii]

> Date: Tue, 12 Sep 2017 18:37 EDT
> From: Winston <wbe@psr.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, Dmitry Gutov <dgutov@yandex.ru>
> 
> xref-find-definitions UI suggestion:
> 
>    Since xref-find-definitions is willing to match names typed in all
>    lower case to mixed case names, TAB (show alternatives) should, too.
>    If there's not already a way to make TAB do so, then a way should be
>    added.

Dmitry, how about the patch below?

--- lisp/progmodes/xref.el~0	2017-06-01 07:00:20.000000000 +0300
+++ lisp/progmodes/xref.el	2017-09-13 07:30:19.779611200 +0300
@@ -762,22 +762,25 @@
           (not (memq command (cdr xref-prompt-for-identifier)))
         (memq command xref-prompt-for-identifier))))
 
-(defun xref--read-identifier (prompt)
-  "Return the identifier at point or read it from the minibuffer."
+(defun xref--read-identifier (prompt &optional caseless)
+  "Return the identifier at point or read it from the minibuffer.
+Optional argument CASELESS means ignore letter-case when completing
+on user input."
   (let* ((backend (xref-find-backend))
          (id (xref-backend-identifier-at-point backend)))
     (cond ((or current-prefix-arg
                (not id)
                (xref--prompt-p this-command))
-           (completing-read (if id
-                                (format "%s (default %s): "
-                                        (substring prompt 0 (string-match
-                                                             "[ :]+\\'" prompt))
-                                        id)
-                              prompt)
-                            (xref-backend-identifier-completion-table backend)
-                            nil nil nil
-                            'xref--read-identifier-history id))
+           (let ((completion-ignore-case caseless))
+             (completing-read (if id
+                                  (format "%s (default %s): "
+                                          (substring prompt 0 (string-match
+                                                               "[ :]+\\'" prompt))
+                                          id)
+                                prompt)
+                              (xref-backend-identifier-completion-table backend)
+                              nil nil nil
+                              'xref--read-identifier-history id)))
           (t id))))
 
 \f
@@ -804,19 +807,19 @@
 definition for IDENTIFIER, display it in the selected window.
 Otherwise, display the list of the possible definitions in a
 buffer where the user can select from the list."
-  (interactive (list (xref--read-identifier "Find definitions of: ")))
+  (interactive (list (xref--read-identifier "Find definitions of: " t)))
   (xref--find-definitions identifier nil))
 
 ;;;###autoload
 (defun xref-find-definitions-other-window (identifier)
   "Like `xref-find-definitions' but switch to the other window."
-  (interactive (list (xref--read-identifier "Find definitions of: ")))
+  (interactive (list (xref--read-identifier "Find definitions of: " t)))
   (xref--find-definitions identifier 'window))
 
 ;;;###autoload
 (defun xref-find-definitions-other-frame (identifier)
   "Like `xref-find-definitions' but switch to the other frame."
-  (interactive (list (xref--read-identifier "Find definitions of: ")))
+  (interactive (list (xref--read-identifier "Find definitions of: " t)))
   (xref--find-definitions identifier 'frame))
 
 ;;;###autoload

bug#28439: suggestion: support case-independent xref-find-definitions prompt TAB

[Dmitry Gutov]

Hi Eli,

On 9/13/17 6:30 PM, Eli Zaretskii wrote:

>> xref-find-definitions UI suggestion:
>>
>>     Since xref-find-definitions is willing to match names typed in all
>>     lower case to mixed case names, TAB (show alternatives) should, too.
>>     If there's not already a way to make TAB do so, then a way should be
>>     added.
> 
> Dmitry, how about the patch below?

Sorry, what don't we want to follow the value of completion-ignore-case?

The fact that we have the option tags-case-fold-search (used in 
find-tag-tag, among other places) probably means that some users prefer 
not to ignore case.

Maybe we could add a similar xref-specific option on top, but I'm not 
sure why completion-ignore-case is not good enough.

We could change its default to t, though.

bug#28439: suggestion: support case-independent xref-find-definitions prompt TAB

[Eli Zaretskii]

> Cc: bug-gnu-emacs@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 14 Sep 2017 02:04:10 +0300
> 
> >>     Since xref-find-definitions is willing to match names typed in all
> >>     lower case to mixed case names, TAB (show alternatives) should, too.
> >>     If there's not already a way to make TAB do so, then a way should be
> >>     added.
> > 
> > Dmitry, how about the patch below?
> 
> Sorry, what don't we want to follow the value of completion-ignore-case?

Because it has a much broader effect.  And because the issue here is
inconsistency between completion and actual matching in
xref-find-definitions.  They should be consistent, IMO.

> The fact that we have the option tags-case-fold-search (used in 
> find-tag-tag, among other places) probably means that some users prefer 
> not to ignore case.

Then maybe an alternative is to make tags-case-fold-search nil by
default?  Or make xref--read-identifier be case-insensitive if
case-fold-search is non-nil?

> Maybe we could add a similar xref-specific option on top, but I'm not 
> sure why completion-ignore-case is not good enough.
> 
> We could change its default to t, though.

Emacs-wide?  Or just when completing on identifiers?

bug#28439: suggestion: support case-independent xref-find-definitions prompt TAB

[Dmitry Gutov]

On 9/14/17 8:01 PM, Eli Zaretskii wrote:

>> Sorry, what don't we want to follow the value of completion-ignore-case?
> 
> Because it has a much broader effect. 

I guessed so. But do you have a particular problem in mind?

> And because the issue here is
> inconsistency between completion and actual matching in
> xref-find-definitions.  They should be consistent, IMO.

I think I agree, because xref--read-identifier calls completing-read 
with nil require-match argument (so we can't rely on the completion 
table to correct the input), but your patch doesn't fix the 
inconsistency. It only moves the identifier read toward the 
case-fold-search default.

>> The fact that we have the option tags-case-fold-search (used in
>> find-tag-tag, among other places) probably means that some users prefer
>> not to ignore case.
> 
> Then maybe an alternative is to make tags-case-fold-search nil by
> default? Or make xref--read-identifier be case-insensitive if
> case-fold-search is non-nil?

The other way around: etags--xref-find-definitions can bind 
tags-case-fold-search to the value of completion-ignore-case. Or 
whichever xref-specific variable we add.

We'd also need to add case-insensitive search support to 
elisp--xref-find-definitions, I suppose. So far, 
find-function-search-for-symbol always performs case-sensitive search. 
It's rarely a problem, though, because Elisp uses capital letters very 
infrequently.

>> Maybe we could add a similar xref-specific option on top, but I'm not
>> sure why completion-ignore-case is not good enough.
>>
>> We could change its default to t, though.
> 
> Emacs-wide?  Or just when completing on identifiers?

Either is fine, as far as I'm concerned.

bug#28439: suggestion: support case-independent xref-find-definitions prompt TAB

[Eli Zaretskii]

> Cc: wbe@psr.com, bug-gnu-emacs@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 19 Sep 2017 03:55:34 +0300
> 
> On 9/14/17 8:01 PM, Eli Zaretskii wrote:
> 
> >> Sorry, what don't we want to follow the value of completion-ignore-case?
> > 
> > Because it has a much broader effect. 
> 
> I guessed so. But do you have a particular problem in mind?

No, just a general concern.

> > Then maybe an alternative is to make tags-case-fold-search nil by
> > default? Or make xref--read-identifier be case-insensitive if
> > case-fold-search is non-nil?
> 
> The other way around: etags--xref-find-definitions can bind 
> tags-case-fold-search to the value of completion-ignore-case. Or 
> whichever xref-specific variable we add.

Fine with me.

> We'd also need to add case-insensitive search support to 
> elisp--xref-find-definitions, I suppose. So far, 
> find-function-search-for-symbol always performs case-sensitive search. 
> It's rarely a problem, though, because Elisp uses capital letters very 
> infrequently.

Agreed, on both counts.

> >> Maybe we could add a similar xref-specific option on top, but I'm not
> >> sure why completion-ignore-case is not good enough.
> >>
> >> We could change its default to t, though.
> > 
> > Emacs-wide?  Or just when completing on identifiers?
> 
> Either is fine, as far as I'm concerned.

Emacs-wide is too radical, I think, and I don't think we have a good
case for justifying that.

Thanks.

bug#28439: suggestion: support case-independent xref-find-definitions prompt

[Winston]

[I've corrected the Subject and Cc lists above to refer to bug#28439
 instead of sending to bug-gnu-emacs@gnu.org.]

>> Sorry, what don't we want to follow the value of completion-ignore-case?

> Because it has a much broader effect.  And because the issue here is
> inconsistency between completion and actual matching in
> xref-find-definitions.  They should be consistent, IMO.

   Thank you, Eli.  IMHO, too.

   For now, I'm using completion-ignore-case because it solves the
immediate issue, but I was concerned that it could have side-effects
elsewhere that I won't discover until they happen.

   Sure, since I like it here, I might like it elsewhere, but I won't
know until those situations come up, and if I don't like it elsewhere,
then I'd be stuck.

   I liked your patch when I saw it, because it directly addressed the
issue.  Other options obviously include:
* xref-completion-ignore-case, default t;
* creating something that applies to both completions and find-definition
  (I didn't save the patch, but my vague recollection is that it just
  defaulted to using t for completions).

JMO, FWIW.
 -WBE

bug#28439: suggestion: support case-independent xref-find-definitions prompt

[Dmitry Gutov]

On 9/14/17 9:20 PM, Winston wrote:

>     Sure, since I like it here, I might like it elsewhere, but I won't
> know until those situations come up, and if I don't like it elsewhere,
> then I'd be stuck.

I'd like to know of one such example, if it happens.

>     I liked your patch when I saw it, because it directly addressed the
> issue.  Other options obviously include:
> * xref-completion-ignore-case, default t;

Yes, that's my alternative suggestion.

> * creating something that applies to both completions and find-definition
>    (I didn't save the patch, but my vague recollection is that it just
>    defaulted to using t for completions).

Not sure what this means, need details.

bug#28439: suggestion: support case-independent

[Winston]

>>     Sure, since I like it here, I might like it elsewhere, but I won't
>> know until those situations come up, and if I don't like it elsewhere,
>> then I'd be stuck.

> I'd like to know of one such example, if it happens.

OK, but if it's a couple years from now before it happens, I'm likely to
assume you're not still interested.  :)

>> * xref-completion-ignore-case, default t;

>> * creating something that applies to both completions and
>>    find-definition (I didn't save the patch, but my vague
>>    recollection is that it just defaulted to using t for
>>    completions).

> Not sure what this means, need details.

The first was a variable that only affects completions.

The second controls both completions and what xref-find-definitions
matches, so either both would allow case folding or both would not.
 -WBE

bug#28439: suggestion: support case-independent

[Dmitry Gutov]

On 9/15/17 1:32 AM, Winston wrote:

> OK, but if it's a couple years from now before it happens, I'm likely to
> assume you're not still interested.  :)

I probably would. :)

>>> * xref-completion-ignore-case, default t;
> 
>>> * creating something that applies to both completions and
>>>     find-definition (I didn't save the patch, but my vague
>>>     recollection is that it just defaulted to using t for
>>>     completions).
> 
>> Not sure what this means, need details.
> 
> The first was a variable that only affects completions.
> 
> The second controls both completions and what xref-find-definitions
> matches, so either both would allow case folding or both would not.

The second option is something to consider indeed. Since we don't 
require a match during completion, the user can enter a string that 
would have completions, but gets no case-sensitive matches.

We can do that without an extra variable, though, by making definitions 
search be controlled by completion-ignore-case.

bug#28439: suggestion: support case-independent xref-find-definitions prompt

[Winston]

Re: using completion-ignore-case to control both completions and
    xref-find-definitions 

Dmitry previously commented:
> We can do that without an extra variable, though, by making
> definitions search be controlled by completion-ignore-case.

   Unless you want to argue that xref-find-definitions is a
"completion", doing that would mean that completion-ignore-case has an
unexpected side-effect (namely, that it affects a function that is not a
"completion").  On programming aesthetic grounds, then, I'd argue that
xref-ignore-case (or whatever you want to call it) would be better.
 -WBE

bug#28439: suggestion: support case-independent xref-find-definitions prompt

[Dmitry Gutov]

On 9/28/17 2:06 AM, Winston wrote:

>     Unless you want to argue that xref-find-definitions is a
> "completion", doing that would mean that completion-ignore-case has an
> unexpected side-effect (namely, that it affects a function that is not a
> "completion").  On programming aesthetic grounds, then, I'd argue that
> xref-ignore-case (or whatever you want to call it) would be better.

This is just splitting hairs. xref-ignore-case will default to 
completion-ignore-case, and you'd be able to give the same complaint.

bug#28439: suggestion: support case-independent xref-find-definitions prompt

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 9/28/17 2:06 AM, Winston wrote:
>
>>     Unless you want to argue that xref-find-definitions is a
>> "completion", doing that would mean that completion-ignore-case has an
>> unexpected side-effect (namely, that it affects a function that is not a
>> "completion").  On programming aesthetic grounds, then, I'd argue that
>> xref-ignore-case (or whatever you want to call it) would be better.
>
> This is just splitting hairs. xref-ignore-case will default to
> completion-ignore-case, and you'd be able to give the same complaint.

Skimming this thread, it seems that there was concern that xref is
inconsistent here somehow, but no actual use case was shown where this
happens.  So I'm closing this bug report -- if this is a problem seen in
the wild, please respond to the debbugs address and we'll reopen.

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