Disabling imenu default of thing-at-point

Disabling imenu default of thing-at-point

[Felipe Ochoa]

imenu currently uses (thing-at-point 'symbol) to offer a default in
completing read. It's very helpful when symbol at point is one of the
options, but not really useful when not. It's particularly inconvenient
when using ido for completing read (e.g., with ido-ubiquitous), since it
makes M-x imenu RET not do anything instead of jumping to the first
option.

The fix is one-line (below), but the unhelpful (for me) default seems to
be coded explicitly. Would this patch break anyone else's use of imenu?

--- a/lisp/imenu.el
+++ b/lisp/imenu.el
@@ -876,7 +876,7 @@ Return one of the entries in index-alist or nil."
 		    (cdr item)))
 	    index-alist))))
     (when (stringp name)
-      (setq name (or (imenu-find-default name prepared-index-alist) name)))
+      (setq name (imenu-find-default name prepared-index-alist)))
     (cond (prompt)
 	  ((and name (imenu--in-alist name prepared-index-alist))
 	   (setq prompt (format "Index item (default %s): " name)))

RE: Disabling imenu default of thing-at-point

[Drew Adams]

> imenu currently uses (thing-at-point 'symbol) to offer a default in
> completing read. It's very helpful when symbol at point is one of the
> options, but not really useful when not. It's particularly inconvenient
> when using ido for completing read (e.g., with ido-ubiquitous), since it
> makes M-x imenu RET not do anything instead of jumping to the first
> option.

Sounds like Ido (or Ido Ubiquitous) needs to be fixed.  There
should not be a problem with providing a default value, even
when that default value might not always be helpful.

> The fix is one-line (below), but the unhelpful (for me) default seems to
> be coded explicitly. Would this patch break anyone else's use of imenu?

It breaks everyone's ability to pick up what was previously the
default value as a default value.

> -      (setq name (or (imenu-find-default name prepared-index-alist) name)))
> +      (setq name (imenu-find-default name prepared-index-alist)))
>      (cond (prompt)
>  	  ((and name (imenu--in-alist name prepared-index-alist))
>  	   (setq prompt (format "Index item (default %s): " name)))

If you make that change then what is the sense of binding `name' to
`(thing-at-point 'symbol)' in the first place?  It's only purpose
could then be to return a string so that `imenu-find-default' is
used at all.  This doesn't make any sense (to me).

Re: Disabling imenu default of thing-at-point

[Noam Postavsky]

On Mon, Jul 24, 2017 at 1:39 PM, Drew Adams <drew.adams@oracle.com> wrote:
>> imenu currently uses (thing-at-point 'symbol) to offer a default in
>> completing read. It's very helpful when symbol at point is one of the
>> options, but not really useful when not. It's particularly inconvenient
>> when using ido for completing read (e.g., with ido-ubiquitous), since it
>> makes M-x imenu RET not do anything instead of jumping to the first
>> option.
>
> Sounds like Ido (or Ido Ubiquitous) needs to be fixed.  There
> should not be a problem with providing a default value, even
> when that default value might not always be helpful.

You can press C-j instead of RET at an ido to prompt to return only
what you've explicitly typed (i.e., without any completion to default
automagic).

Re: Disabling imenu default of thing-at-point

[Felipe Ochoa]

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

> You can press C-j instead of RET at an ido to prompt to return only
> what you've explicitly typed (i.e., without any completion to default
> automagic).

C-j brings up the `minibuffer-completion-help' dialog. I think because
imenu let-binds `minibuffer-setup-hook' around `completing-read', but I'm
not sure

[-- Attachment #2: Type: text/html, Size: 411 bytes --]

Re: Disabling imenu default of thing-at-point

[Felipe Ochoa]

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

(Apologies, forgot to CC the list)

> Sounds like Ido (or Ido Ubiquitous) needs to be fixed.  There
> should not be a problem with providing a default value, even
> when that default value might not always be helpful.

I think this could also be an option, but note that ido follows
completing-read-default in its handling of invalid defaults. E.g.,
evaluate the following and hit RET without selecting anything:

(completing-read-default
 "Complete: " '("abc" "def" "ghi")
 nil t nil nil "jkl")

The result will be "jkl".

One thing that cannot be fixed within ido (or completing-read)
is the prompt. Currently all users see "Index item (default %s): ",
even when the default is bogus, instead of "Index item: ".

> It breaks everyone's ability to pick up what was previously the
> default value as a default value.
>
> > -      (setq name (or (imenu-find-default name prepared-index-alist)
name)))
> > +      (setq name (imenu-find-default name prepared-index-alist)))
> >      (cond (prompt)
> >         ((and name (imenu--in-alist name prepared-index-alist))
> >          (setq prompt (format "Index item (default %s): " name)))
>
> If you make that change then what is the sense of binding `name' to
> `(thing-at-point 'symbol)' in the first place?  It's only purpose
> could then be to return a string so that `imenu-find-default' is
> used at all.  This doesn't make any sense (to me).

The code does not do away with defaults. To me, the new approach
would mean in words:

1. Grab the symbol at point.
2. Check if it matches one of the items in the index.
3. If so, offer it as a default. Otherwise, ignore it.

[-- Attachment #2: Type: text/html, Size: 2356 bytes --]

Re: Disabling imenu default of thing-at-point

[Felipe Ochoa]

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

---------- Forwarded message ----------
From: Drew Adams <drew.adams@oracle.com>
Date: 25 July 2017 at 00:57
Subject: RE: Disabling imenu default of thing-at-point
To: Felipe Ochoa <felipe.nospam.ochoa@gmail.com>



> > `completing-read' has multiple uses. It is not just for picking one of
a set of completion candidates. When `t' is specified as the REQUIRED arg
it means that either (a) one of those candidates must be picked or (2) the
default value can be picked. The list of candidates might be predefined,
and the default value might be computed dynamically (e.g. thing at point),
for example.

> The documentation doesn't say this, but since it has worked this way for
15+ years, I believe you :-)



The doc (`C-h f') says:



REQUIRE-MATCH can take the following values:



- t means that the user is not allowed to exit unless the input is (or
completes to) an element of COLLECTION or is null.



If the input is null, ‘completing-read’ returns DEF, or the first element
of the list of default values, or an empty string if DEF is nil, regardless
of the value of REQUIRE-MATCH.


> > Why is it bogus? If a different behavior were desired, where a user
could not pick the default value but had to pick one of the completion
candidates, then the `completing-read' call would be different.

> I think this is exactly what I'm suggesting (That we use a different
`completing-read' call).

> I think this type of default is bogus, because of what imenu is supposed
to do. From *info*: "The Imenu facility offers a way to find the major
definitions in a file by name... If you type ‘M-x imenu’, it reads the name
of a definition using the minibuffer, then moves point to that definition."
Offering as default a non-existent definition is bogus in my mind, since
where should point move to in that case?

OK, that makes sense.  You can do that easier, with clearer code - just
test whether the default returned by thing-at-point is one of the
completion candidates.

> > Perhaps Ido Ubiquitous does not provide for or handle such a use case
(?). But it is a common and useful use case of `completing-read'.

> To the contrary, ido works just like `completing-read'. I can see how it
might be useful in general for completing-read, but I just don't think it's
useful for its use imenu. (And hence why I think imenu should be fixed)



I'm OK with that. I think that a previous thread with the Ido Ubiquitous
developer expressed the desire to remove that use case from
`completing-read' because I.U. does *not* support it (jumps through some
hoops to work around it). But perhaps I misunderstood.


> > But is that what was intended for the Imenu code? Does it make no sense
(in this case) for a user to pick the default value if it is not one of the
completion candidates?

> Stefan would have to comment on the code's intent (from 2001). As
mentioned above, I don't think this makes sense. But others may have uses
for imenu that I don't know of where this behavior is useful.

I guess that's the question: is there any use here for a value that is not
one of the completion candidates. If not, what you suggest sounds good.

- Drew

On 25 July 2017 at 11:05, Felipe Ochoa <felipe.nospam.ochoa@gmail.com>
wrote:

> (Apologies, forgot to CC the list)
>
>
> > Sounds like Ido (or Ido Ubiquitous) needs to be fixed.  There
> > should not be a problem with providing a default value, even
> > when that default value might not always be helpful.
>
> I think this could also be an option, but note that ido follows
> completing-read-default in its handling of invalid defaults. E.g.,
> evaluate the following and hit RET without selecting anything:
>
> (completing-read-default
>  "Complete: " '("abc" "def" "ghi")
>  nil t nil nil "jkl")
>
> The result will be "jkl".
>
> One thing that cannot be fixed within ido (or completing-read)
> is the prompt. Currently all users see "Index item (default %s): ",
> even when the default is bogus, instead of "Index item: ".
>
> > It breaks everyone's ability to pick up what was previously the
> > default value as a default value.
> >
> > > -      (setq name (or (imenu-find-default name prepared-index-alist)
> name)))
> > > +      (setq name (imenu-find-default name prepared-index-alist)))
> > >      (cond (prompt)
> > >         ((and name (imenu--in-alist name prepared-index-alist))
> > >          (setq prompt (format "Index item (default %s): " name)))
> >
> > If you make that change then what is the sense of binding `name' to
> > `(thing-at-point 'symbol)' in the first place?  It's only purpose
> > could then be to return a string so that `imenu-find-default' is
> > used at all.  This doesn't make any sense (to me).
>
> The code does not do away with defaults. To me, the new approach
> would mean in words:
>
> 1. Grab the symbol at point.
> 2. Check if it matches one of the items in the index.
> 3. If so, offer it as a default. Otherwise, ignore it.
>
>
>

[-- Attachment #2: Type: text/html, Size: 10228 bytes --]

Re: Disabling imenu default of thing-at-point

[Felipe Ochoa]

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

---------- Forwarded message ----------
From: Drew Adams <drew.adams@oracle.com>
Date: 24 July 2017 at 20:24
Subject: RE: Disabling imenu default of thing-at-point
To: Felipe Ochoa <felipe.nospam.ochoa@gmail.com>
> > Sounds like Ido (or Ido Ubiquitous) needs to be fixed.  There
> > should not be a problem with providing a default value, even
> > when that default value might not always be helpful.

> I think this could also be an option, but note that ido follows
> completing-read-default in its handling of invalid defaults. E.g.,
> evaluate the following and hit RET without selecting anything:

> (completing-read-default "Complete: " '("abc" "def" "ghi") nil t nil nil
"jkl")

> The result will be "jkl".

That's the case also for the Imenu code you cited, no? NAME is the default
value to `completing-read', and you get it in that case by hitting RET with
no input.

And there is nothing "invalid" about the "jkl" value.

`completing-read' has multiple uses. It is not just for picking one of a
set of completion candidates. When `t' is specified as the REQUIRED arg it
means that either (a) one of those candidates must be picked *or* (2) the
default value can be picked. The list of candidates might be predefined,
and the default value might be computed dynamically (e.g. thing at point),
for example.
> One thing that cannot be fixed within ido (or completing-read)
> is the prompt. Currently all users see "Index item (default %s): ",
> even when the default is bogus, instead of "Index item: ".

Why is it bogus? If a different behavior were desired, where a user could
not pick the default value but had to pick one of the completion
candidates, then the `completing-read' call would be different.



Perhaps Ido Ubiquitous does not provide for or handle such a use case (?).
But it is a common and useful use case of `completing-read'.


> > > It breaks everyone's ability to pick up what was previously the
> > > default value as a default value.
> > >
> > > > -      (setq name (or (imenu-find-default name
prepared-index-alist) name)))
> > > > +      (setq name (imenu-find-default name prepared-index-alist)))
> > > >      (cond (prompt)
> > > >         ((and name (imenu--in-alist name prepared-index-alist))
> > > >          (setq prompt (format "Index item (default %s): " name)))
> > >
> > > If you make that change then what is the sense of binding `name' to
> > > `(thing-at-point 'symbol)' in the first place?  It's only purpose
> > > could then be to return a string so that `imenu-find-default' is
> > > used at all.  This doesn't make any sense (to me).


>
> The code does not do away with defaults. To me, the new approach
> would mean in words:
>
> 1. Grab the symbol at point.
> 2. Check if it matches one of the items in the index.
> 3. If so, offer it as a default. Otherwise, ignore it.


I understand that that's what you proposed. (There's a simpler way to code
that, if that's what's desired.) But is that what was intended for the
Imenu code? Does it make no sense (in this case) for a user to pick the
default value if it is not one of the completion candidates?

On 25 July 2017 at 11:05, Felipe Ochoa <felipe.nospam.ochoa@gmail.com>
wrote:

> (Apologies, forgot to CC the list)
>
>
> > Sounds like Ido (or Ido Ubiquitous) needs to be fixed.  There
> > should not be a problem with providing a default value, even
> > when that default value might not always be helpful.
>
> I think this could also be an option, but note that ido follows
> completing-read-default in its handling of invalid defaults. E.g.,
> evaluate the following and hit RET without selecting anything:
>
> (completing-read-default
>  "Complete: " '("abc" "def" "ghi")
>  nil t nil nil "jkl")
>
> The result will be "jkl".
>
> One thing that cannot be fixed within ido (or completing-read)
> is the prompt. Currently all users see "Index item (default %s): ",
> even when the default is bogus, instead of "Index item: ".
>
> > It breaks everyone's ability to pick up what was previously the
> > default value as a default value.
> >
> > > -      (setq name (or (imenu-find-default name prepared-index-alist)
> name)))
> > > +      (setq name (imenu-find-default name prepared-index-alist)))
> > >      (cond (prompt)
> > >         ((and name (imenu--in-alist name prepared-index-alist))
> > >          (setq prompt (format "Index item (default %s): " name)))
> >
> > If you make that change then what is the sense of binding `name' to
> > `(thing-at-point 'symbol)' in the first place?  It's only purpose
> > could then be to return a string so that `imenu-find-default' is
> > used at all.  This doesn't make any sense (to me).
>
> The code does not do away with defaults. To me, the new approach
> would mean in words:
>
> 1. Grab the symbol at point.
> 2. Check if it matches one of the items in the index.
> 3. If so, offer it as a default. Otherwise, ignore it.
>
>
>

[-- Attachment #2: Type: text/html, Size: 8340 bytes --]

Re: Disabling imenu default of thing-at-point

[Felipe Ochoa]

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

I realize this thread is probably hard to follow, so wanted to offer a
brief recap (Drew, please correct me where necessary).

Core issue:
Felipe> imenu currently uses (thing-at-point 'symbol) to offer a default in
completing read. It's very helpful when symbol at point is one of the
options, but not really useful when not. It's particularly inconvenient
when using ido for completing read

Possible workaround:
Noam suggested I use C-j to skip the default automagic in ido, but this
does not work because of some imenu trickery (plus I think imenu is the
place to fix this)

Drew and I had a back-and-forth about whether the hypothetical fix belongs
in ido/completing-read or imenu. The conclusion was it would belong in
imenu, but he's still unclear whether a fix is necessary at all.

Drew> But is that what was intended for the Imenu code? Does it make no
sense (in this case) for a user to pick the default value if it is not one
of the completion candidates?


On 25 July 2017 at 11:21, Felipe Ochoa <felipe.nospam.ochoa@gmail.com>
wrote:

>
> ---------- Forwarded message ----------
> From: Drew Adams <drew.adams@oracle.com>
> Date: 24 July 2017 at 20:24
> Subject: RE: Disabling imenu default of thing-at-point
> To: Felipe Ochoa <felipe.nospam.ochoa@gmail.com>
> > > Sounds like Ido (or Ido Ubiquitous) needs to be fixed.  There
> > > should not be a problem with providing a default value, even
> > > when that default value might not always be helpful.
>
> > I think this could also be an option, but note that ido follows
> > completing-read-default in its handling of invalid defaults. E.g.,
> > evaluate the following and hit RET without selecting anything:
>
> > (completing-read-default "Complete: " '("abc" "def" "ghi") nil t nil nil
> "jkl")
>
> > The result will be "jkl".
>
> That's the case also for the Imenu code you cited, no? NAME is the default
> value to `completing-read', and you get it in that case by hitting RET with
> no input.
>
> And there is nothing "invalid" about the "jkl" value.
>
> `completing-read' has multiple uses. It is not just for picking one of a
> set of completion candidates. When `t' is specified as the REQUIRED arg it
> means that either (a) one of those candidates must be picked *or* (2) the
> default value can be picked. The list of candidates might be predefined,
> and the default value might be computed dynamically (e.g. thing at point),
> for example.
> > One thing that cannot be fixed within ido (or completing-read)
> > is the prompt. Currently all users see "Index item (default %s): ",
> > even when the default is bogus, instead of "Index item: ".
>
> Why is it bogus? If a different behavior were desired, where a user could
> not pick the default value but had to pick one of the completion
> candidates, then the `completing-read' call would be different.
>
>
>
> Perhaps Ido Ubiquitous does not provide for or handle such a use case (?).
> But it is a common and useful use case of `completing-read'.
>
>
> > > > It breaks everyone's ability to pick up what was previously the
> > > > default value as a default value.
> > > >
> > > > > -      (setq name (or (imenu-find-default name
> prepared-index-alist) name)))
> > > > > +      (setq name (imenu-find-default name prepared-index-alist)))
> > > > >      (cond (prompt)
> > > > >         ((and name (imenu--in-alist name prepared-index-alist))
> > > > >          (setq prompt (format "Index item (default %s): " name)))
> > > >
> > > > If you make that change then what is the sense of binding `name' to
> > > > `(thing-at-point 'symbol)' in the first place?  It's only purpose
> > > > could then be to return a string so that `imenu-find-default' is
> > > > used at all.  This doesn't make any sense (to me).
>
>
> >
> > The code does not do away with defaults. To me, the new approach
> > would mean in words:
> >
> > 1. Grab the symbol at point.
> > 2. Check if it matches one of the items in the index.
> > 3. If so, offer it as a default. Otherwise, ignore it.
>
>
> I understand that that's what you proposed. (There's a simpler way to code
> that, if that's what's desired.) But is that what was intended for the
> Imenu code? Does it make no sense (in this case) for a user to pick the
> default value if it is not one of the completion candidates?
>
> On 25 July 2017 at 11:05, Felipe Ochoa <felipe.nospam.ochoa@gmail.com>
> wrote:
>
>> (Apologies, forgot to CC the list)
>>
>>
>> > Sounds like Ido (or Ido Ubiquitous) needs to be fixed.  There
>> > should not be a problem with providing a default value, even
>> > when that default value might not always be helpful.
>>
>> I think this could also be an option, but note that ido follows
>> completing-read-default in its handling of invalid defaults. E.g.,
>> evaluate the following and hit RET without selecting anything:
>>
>> (completing-read-default
>>  "Complete: " '("abc" "def" "ghi")
>>  nil t nil nil "jkl")
>>
>> The result will be "jkl".
>>
>> One thing that cannot be fixed within ido (or completing-read)
>> is the prompt. Currently all users see "Index item (default %s): ",
>> even when the default is bogus, instead of "Index item: ".
>>
>> > It breaks everyone's ability to pick up what was previously the
>> > default value as a default value.
>> >
>> > > -      (setq name (or (imenu-find-default name prepared-index-alist)
>> name)))
>> > > +      (setq name (imenu-find-default name prepared-index-alist)))
>> > >      (cond (prompt)
>> > >         ((and name (imenu--in-alist name prepared-index-alist))
>> > >          (setq prompt (format "Index item (default %s): " name)))
>> >
>> > If you make that change then what is the sense of binding `name' to
>> > `(thing-at-point 'symbol)' in the first place?  It's only purpose
>> > could then be to return a string so that `imenu-find-default' is
>> > used at all.  This doesn't make any sense (to me).
>>
>> The code does not do away with defaults. To me, the new approach
>> would mean in words:
>>
>> 1. Grab the symbol at point.
>> 2. Check if it matches one of the items in the index.
>> 3. If so, offer it as a default. Otherwise, ignore it.
>>
>>
>>
>

[-- Attachment #2: Type: text/html, Size: 10208 bytes --]