bug#22692: 25.0.91; xref-find-definitions fails to prompt

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Mike Kupfer]

If point is on a token that isn't defined in the current tags file,
M-. gives me an error (look for "Cygwin" further down in this bug
report).  I expect to get a prompt.

Ditto if point is at the "#" of "#include" or "#ifdef".

More broadly, I think the default for xref-prompt-for-identifier is a
usability regression.  When I'm examining code, point is rarely at the
token I want to find the definition for.  So with the default behavior,
I either have to move my hand to the mouse to set point and then move it
back to the keyboard for the "." in M-., or I have to type in a bunch of
navigation keystrokes, or I have to add a C-u prefix.  Either way, the
default behavior is getting in my way, not helping.

In GNU Emacs 25.0.91.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars)
 of 2016-02-14 built on allegro
Windowing system distributor 'The X.Org Foundation', version 11.0.11604000
System Description:	Debian GNU/Linux 8.3 (jessie)

Configured using:
 'configure --prefix=/usr/new'

Configured features:
XPM JPEG TIFF GIF PNG SOUND NOTIFY LIBXML2 FREETYPE XFT ZLIB
TOOLKIT_SCROLL_BARS LUCID X11

Important settings:
  value of $LC_TIME: C
  value of $LANG: en_US.utf8
  locale-coding-system: utf-8-unix

Major mode: Help

Minor modes in effect:
  diff-auto-refine-mode: t
  shell-dirtrack-mode: t
  delete-selection-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  transient-mark-mode: t

Recent messages:
Please type y or n, or C-v to scroll: y [3 times]
user-error: No definitions found for: Cygwin
Type "q" in help window to delete it
Quit

find-tag is not on any key
Mark set
GNU Emacs 25.0.91.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars) of 2016-02-14
You can run the command ‘emacs-version’ with M-x em-v RET
GNU Emacs 25.0.91.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars) of 2016-02-14

Load-path shadows:
None found.

Features:
(shadow emacsbug pulse etags xref project cc-mode cc-fonts cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs eieio-opt
speedbar sb-image ezimage dframe ediff-merg ediff-wind ediff-diff
ediff-mult ediff-help ediff-init ediff-util ediff mh-limit sgml-mode
diff-mode pp gnus-mh mh-alias crm shr-color color shr seq dom subr-x
browse-url mh-search qp mh-thread sort gnus-async gnus-bcklg gnus-kill
gnus-dup gnus-ml disp-table timezone url-http url-gw url-cache url-auth
url-handlers nnrss xml mm-url url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf url-util url-parse
url-vars nndraft nnmh utf-7 rfc2104 network-stream nsm starttls
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg nntp
gnus-cache flow-fill mm-archive mail-extr character-fold misearch
multi-isearch mh-mime mh-identity mh-letter mh-show goto-addr thingatpt
gnus-cite gnus-art mm-uu mml2015 gnus-sum gnus-group gnus-undo
gnus-start gnus-cloud nnimap nnmail mail-source tls gnutls utf7 netrc
nnoo parse-time gnus-spec gnus-int gnus-win gnus-range gnus gnus-ems
nnheader wid-edit mh-comp mh-gnus mm-view mml-smime smime dig mailcap
vc-hg org-element org-rmail org-mhe org-irc org-info org-gnus
org-docview doc-view jka-compr image-mode org-bibtex bibtex org-bbdb
org-w3m org org-macro org-footnote org-pcomplete org-list org-faces
org-entities org-version ob-emacs-lisp ob ob-tangle ob-ref ob-lob
ob-table ob-exp org-src ob-keys ob-comint ob-core ob-eval org-compat
org-macs org-loaddefs find-func cal-menu calendar cal-loaddefs
pcmpl-unix mh-inc hl-line mh-tool-bar mh-seq mh-xface mh-utils mh-folder
which-func imenu mh-scan mh-e mh-compat mh-acros cl mh-buffers
mh-loaddefs shell pcomplete mdk-mail smtpmail auth-source cl-seq eieio
byte-opt bytecomp byte-compile cl-extra cconv eieio-core cl-macs gv
password-cache sendmail message format-spec rfc822 mml mml-sec epg
epg-config gnus-util mm-decode mm-bodies mm-encode mail-parse rfc2231
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode mail-prsvr
mailabbrev mail-utils gmm-utils mailheader server noutline outline
easy-mmode comint ansi-color ring xcscope easymenu advice ispell delsel
vc cl-loaddefs pcase cl-lib vc-dispatcher dired timeclock mdk-hacks
time-date mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel x-win term/common-win x-dnd tool-bar dnd fontset
image regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cl-generic cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote inotify dynamic-setting
font-render-setting x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 509163 47425)
 (symbols 48 47223 0)
 (miscs 40 492 1325)
 (strings 32 119455 22990)
 (string-bytes 1 3536292)
 (vectors 16 42300)
 (vector-slots 8 881800 34425)
 (floats 8 791 405)
 (intervals 56 48671 33)
 (buffers 976 47)
 (heap 1024 56510 27603))

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Eli Zaretskii]

> From: Mike Kupfer <m.kupfer@acm.org>
> Date: Mon, 15 Feb 2016 17:02:03 -0800
> 
> More broadly, I think the default for xref-prompt-for-identifier is a
> usability regression.  When I'm examining code, point is rarely at the
> token I want to find the definition for.

It's exactly the opposite here: point is almost always on the token.

> So with the default behavior,
> I either have to move my hand to the mouse to set point and then move it
> back to the keyboard for the "." in M-., or I have to type in a bunch of
> navigation keystrokes, or I have to add a C-u prefix.  Either way, the
> default behavior is getting in my way, not helping.

Perhaps we could have a defcustom for this use case, but making the
behavior you want the default sounds wrong to me.

Thanks.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Dmitry Gutov]

On 02/16/2016 05:44 AM, Eli Zaretskii wrote:

>> So with the default behavior,
>> I either have to move my hand to the mouse to set point and then move it
>> back to the keyboard for the "." in M-., or I have to type in a bunch of
>> navigation keystrokes, or I have to add a C-u prefix.  Either way, the
>> default behavior is getting in my way, not helping.
>
> Perhaps we could have a defcustom for this use case,

Don't we have it already? The aforementioned xref-prompt-for-identifier.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Dmitry Gutov]

On 02/16/2016 03:02 AM, Mike Kupfer wrote:
> If point is on a token that isn't defined in the current tags file,
> M-. gives me an error (look for "Cygwin" further down in this bug
> report).  I expect to get a prompt.

That doesn't sound right to me: "XXX is not found" is a valid piece of 
information. Simply showing a prompt would obscure it.

If we accept that the command searches for the thing at point by 
default, the current behavior seems appropriate. And whether or not it 
prompts, is customizable.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Eli Zaretskii]

> Cc: 22692@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 16 Feb 2016 12:10:42 +0200
> 
> On 02/16/2016 05:44 AM, Eli Zaretskii wrote:
> 
> >> So with the default behavior,
> >> I either have to move my hand to the mouse to set point and then move it
> >> back to the keyboard for the "." in M-., or I have to type in a bunch of
> >> navigation keystrokes, or I have to add a C-u prefix.  Either way, the
> >> default behavior is getting in my way, not helping.
> >
> > Perhaps we could have a defcustom for this use case,
> 
> Don't we have it already? The aforementioned xref-prompt-for-identifier.

Oops!  Sorry, you are right.  So I guess Michael should just use it.

Thanks.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Mike Kupfer]

Dmitry Gutov wrote:

> On 02/16/2016 03:02 AM, Mike Kupfer wrote:
> > If point is on a token that isn't defined in the current tags file,
> > M-. gives me an error (look for "Cygwin" further down in this bug
> > report).  I expect to get a prompt.
> 
> That doesn't sound right to me: "XXX is not found" is a valid piece of
> information. Simply showing a prompt would obscure it.

The Help string for xref-find-definitions says

> With prefix argument or when there’s no identifier at point,
> prompt for it.

I guess you could argue that if point is on a token that's not in the
tags table, it's still on an "identifier".  But "#" is hardly an
identifier.

And the Info page "Xref Commands" says

> With a prefix argument, or if there’s no valid
> identifier at point, it prompts for the identifier.

Does "valid identifier" mean syntatically correct, or does it mean that
the identifier is in the tags table.  Please clarify the documentation.

> If we accept that the command searches for the thing at point by
> default, the current behavior seems appropriate. And whether or not it
> prompts, is customizable.

Yes, I have already have

  (setq xref-prompt-for-identifier t)

in my .emacs.

mike

bug#22692: docstring for xref-find-definitions

[Mike Kupfer]

Based on the behavior that I observe (using "emacs -Q" to visit
src/frame.c), the docstring for xref-find-definitions would be more
accurate if the first paragraph were replaced by

  Find the definition of the identifier at point.
  If there is no identifier at point, use an identifier (or numeric
  literal) near point on the same line.  With a prefix argument or when
  there’s no identifier on the current line, prompt for the identifier
  to look up.

Trying to look up the definition for a numeric literal strikes me as
odd, but that's the behavior I'm seeing.

mike

bug#22692: docstring for xref-find-definitions

[Eli Zaretskii]

> From: Mike Kupfer <m.kupfer@acm.org>
> Date: Wed, 17 Feb 2016 19:45:21 -0800
> 
> Based on the behavior that I observe (using "emacs -Q" to visit
> src/frame.c), the docstring for xref-find-definitions would be more
> accurate if the first paragraph were replaced by
> 
>   Find the definition of the identifier at point.
>   If there is no identifier at point, use an identifier (or numeric
>   literal) near point on the same line.

We cannot be this specific, not as long as the doc string is literal
text defined as part of 'defun'.  The exact definition of what is "the
identifier at point" depends on the xref backend set up by the major
mode.  That definition could be very smart or it could be less smart.
The command doesn't know.

If we want to be accurate here, we will need to come up with a way for
the back-end to supply its definition, and incorporate that in the doc
string that is created dynamically.  Do we have infrastructure for
that?

Failing that, the only band-aid I can offer is something like

  Find the definition of the identifier at or near point.

If you think it's better, we can make that change now.

> Trying to look up the definition for a numeric literal strikes me as
> odd, but that's the behavior I'm seeing.

Did you use M-. in Emacs 24 and before?  Because that's exactly what
it did in this case, it would say this in the echo area:

  Find tag (default 1):

The reason is that this is what etags.el does when asked to find "the
identifier at or near point".  Patches to make it smarter are welcome.
(The relevant function is find-tag-default-bounds.)

Thanks.

bug#22692: docstring for xref-find-definitions

[Mike Kupfer]

Eli Zaretskii wrote:

> > From: Mike Kupfer <m.kupfer@acm.org>
> > Date: Wed, 17 Feb 2016 19:45:21 -0800
> > 
> > Based on the behavior that I observe (using "emacs -Q" to visit
> > src/frame.c), the docstring for xref-find-definitions would be more
> > accurate if the first paragraph were replaced by
> > 
> >   Find the definition of the identifier at point.
> >   If there is no identifier at point, use an identifier (or numeric
> >   literal) near point on the same line.
> 
> We cannot be this specific, not as long as the doc string is literal
> text defined as part of 'defun'.  The exact definition of what is "the
> identifier at point" depends on the xref backend set up by the major
> mode.  That definition could be very smart or it could be less smart.
> The command doesn't know.

Okay.  Is there an easy way for the user to tell which backend is in
use?  If there is, then maybe the docstring could say something general
and then note that the user could also check the backend's
documentation.

> If we want to be accurate here, we will need to come up with a way for
> the back-end to supply its definition, and incorporate that in the doc
> string that is created dynamically.  Do we have infrastructure for
> that?

Not that I know of, but I don't know much about Emacs's documentation
infrastructure.

> Failing that, the only band-aid I can offer is something like
> 
>   Find the definition of the identifier at or near point.
> 
> If you think it's better, we can make that change now.

Yes, I think that would be an improvement.  Thanks.

> > Trying to look up the definition for a numeric literal strikes me as
> > odd, but that's the behavior I'm seeing.
> 
> Did you use M-. in Emacs 24 and before?  Because that's exactly what
> it did in this case, it would say this in the echo area:
> 
>   Find tag (default 1):

Interesting.  I use M-. a lot, and I never noticed this.  I suppose it's
because I know I'm going to type in the name that I'm looking for, so I
ignore the default.

> The reason is that this is what etags.el does when asked to find "the
> identifier at or near point".  Patches to make it smarter are welcome.
> (The relevant function is find-tag-default-bounds.)

Okay.  I'll add this to my spare-time projects list.

thanks,
mike

bug#22692: docstring for xref-find-definitions

[Dmitry Gutov]

On 02/18/2016 06:50 PM, Eli Zaretskii wrote:

> Failing that, the only band-aid I can offer is something like
>
>    Find the definition of the identifier at or near point.
>
> If you think it's better, we can make that change now.

Do we really want to codify that behavior? I've switched to use 
find-tag--default because it seemed appropriate for the etags backend, 
but the "near point" aspect looks fairly awkward to me, and I imagine 
third-party backends might choose to omit it.

I'd prefer to use the more precise behavior in find-tag-default-bounds 
as well. And if there's general agreement here, I wouldn't mind taking 
care of that patch.

Also note how careful the find-tag-default is in avoiding prescribing 
the exact behavior.

> Did you use M-. in Emacs 24 and before?  Because that's exactly what
> it did in this case, it would say this in the echo area:
>
>    Find tag (default 1):
>
> The reason is that this is what etags.el does when asked to find "the
> identifier at or near point".  Patches to make it smarter are welcome.
> (The relevant function is find-tag-default-bounds.)

Not necessarily. Every major mode that knows better should define its 
own find-tag-default-function (but none do, so far). See the dispatch 
inside find-tag--default.

bug#22692: docstring for xref-find-definitions

[Dmitry Gutov]

On 02/18/2016 08:48 PM, Mike Kupfer wrote:
> Is there an easy way for the user to tell which backend is in use?

You can evaluate (xref-find-backend). But that won't help with the 
docstrings, I think.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Dmitry Gutov]

On 02/17/2016 03:55 AM, Mike Kupfer wrote:

> The Help string for xref-find-definitions says
>
>> With prefix argument or when there’s no identifier at point,
>> prompt for it.
>
> I guess you could argue that if point is on a token that's not in the
> tags table, it's still on an "identifier".

Whatever identifier the backend says is there. Which, in the case of 
etags, delegates to find-tag--default.

Validating identifiers against the tags table is a sensible suggestion, 
but that might backfire in several scenarios:

- What if the tags table only contains fully qualified names? We have 
multiple ways the backend can match tags, defined by 
etags-xref-find-definitions-tag-order, which might apply even if the 
identifier is not literally one of the elements.

And I'm not sure performing that matching logic just when the default 
identifier to use is returned, is a good idea.

- Either xref-backend-identifier-at-point impl for etags will have to be 
able to use different logic based on the command currently being invoked 
(that doesn't really fit in the current design), or you won't be able to 
look for e.g. occurrences of the local variable at point (using 
xref-find-references), because etags doesn't parse local variables.

> But "#" is hardly an identifier.

Apparently it is, as far as find-tag-default-bounds is concerned. In the 
following sense: find-tag-default-bounds uses the notion of "symbol" as 
defined by the current syntax table.

If "#" can't be a part of an identifier name in the programming language 
in question (C, right?), maybe you should request that character's 
syntax class to be changed to something like "punctuation" there.

> Does "valid identifier" mean syntatically correct, or does it mean that
> the identifier is in the tags table.  Please clarify the documentation.

We probably should just remove the word "valid" from there.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Mike Kupfer]

Dmitry Gutov wrote:

> On 02/17/2016 03:55 AM, Mike Kupfer wrote:
> > Does "valid identifier" mean syntatically correct, or does it mean that
> > the identifier is in the tags table.  Please clarify the documentation.
> 
> We probably should just remove the word "valid" from there.

Yes, I think that's the right fix.

I'll review and respond to the other points you made, but it will
probably take me a couple of days to get to it.

thanks,
mike

bug#22692: docstring for xref-find-definitions

[Eli Zaretskii]

> Cc: 22692@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 19 Feb 2016 15:01:46 +0200
> 
> On 02/18/2016 06:50 PM, Eli Zaretskii wrote:
> 
> > Failing that, the only band-aid I can offer is something like
> >
> >    Find the definition of the identifier at or near point.
> >
> > If you think it's better, we can make that change now.
> 
> Do we really want to codify that behavior? I've switched to use 
> find-tag--default because it seemed appropriate for the etags backend, 
> but the "near point" aspect looks fairly awkward to me, and I imagine 
> third-party backends might choose to omit it.
> 
> I'd prefer to use the more precise behavior in find-tag-default-bounds 
> as well. And if there's general agreement here, I wouldn't mind taking 
> care of that patch.

Making such a change is fine with me, thanks.

> > Did you use M-. in Emacs 24 and before?  Because that's exactly what
> > it did in this case, it would say this in the echo area:
> >
> >    Find tag (default 1):
> >
> > The reason is that this is what etags.el does when asked to find "the
> > identifier at or near point".  Patches to make it smarter are welcome.
> > (The relevant function is find-tag-default-bounds.)
> 
> Not necessarily. Every major mode that knows better should define its 
> own find-tag-default-function (but none do, so far). See the dispatch 
> inside find-tag--default.

If find-tag-default-function is also used by xref-find-references,
then it won't be TRT to reject constants up front.  A request to find
all the places where a certain constant is used is a valid use case.
It is indeed unlikely to have such a request for the constant 1, but
think about constants like 3.14159 or 3.0e+8.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 19 Feb 2016 15:43:57 +0200
> Cc: 22692@debbugs.gnu.org
> 
> Validating identifiers against the tags table is a sensible suggestion, 

It is a valid use case to try to find out whether a symbol already has
some definition.  In that case, saying "no definition" is what the
user will expect for an undefined symbol, so asking for a symbol will
confuse.

> > Does "valid identifier" mean syntatically correct, or does it mean that
> > the identifier is in the tags table.  Please clarify the documentation.
> 
> We probably should just remove the word "valid" from there.

That depends on how you will change the behavior ;-)

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Dmitry Gutov]

On 02/19/2016 05:59 PM, Eli Zaretskii wrote:

> It is a valid use case to try to find out whether a symbol already has
> some definition.  In that case, saying "no definition" is what the
> user will expect for an undefined symbol, so asking for a symbol will
> confuse.

Then we don't need to change anything there, do we?

>> We probably should just remove the word "valid" from there.
>
> That depends on how you will change the behavior ;-)

Zero change is best change. :-)

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Eli Zaretskii]

> Cc: 22692@debbugs.gnu.org, m.kupfer@acm.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 19 Feb 2016 20:08:37 +0200
> 
> On 02/19/2016 05:59 PM, Eli Zaretskii wrote:
> 
> > It is a valid use case to try to find out whether a symbol already has
> > some definition.  In that case, saying "no definition" is what the
> > user will expect for an undefined symbol, so asking for a symbol will
> > confuse.
> 
> Then we don't need to change anything there, do we?

Yes, I think so.

> >> We probably should just remove the word "valid" from there.
> >
> > That depends on how you will change the behavior ;-)
> 
> Zero change is best change. :-)

No, I meant the change you wanted to make in finding the symbol at
point, to avoid looking for it anywhere on the same line.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Dmitry Gutov]

On 02/19/2016 08:37 PM, Eli Zaretskii wrote:

> No, I meant the change you wanted to make in finding the symbol at
> point, to avoid looking for it anywhere on the same line.

Isn't that orthogonal to the question of using the word "valid"?

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Eli Zaretskii]

> Cc: 22692@debbugs.gnu.org, m.kupfer@acm.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 19 Feb 2016 20:52:28 +0200
> 
> On 02/19/2016 08:37 PM, Eli Zaretskii wrote:
> 
> > No, I meant the change you wanted to make in finding the symbol at
> > point, to avoid looking for it anywhere on the same line.
> 
> Isn't that orthogonal to the question of using the word "valid"?

Maybe.  I don't tend thinking of documentation as a collection of
words.  When you change how the symbol is determined, I could reason
about describing that.  I agree that the chances of having "valid"
there are rather small.

bug#22692: docstring for xref-find-definitions

[Dmitry Gutov]

On 02/19/2016 05:34 PM, Eli Zaretskii wrote:

>> I'd prefer to use the more precise behavior in find-tag-default-bounds
>> as well. And if there's general agreement here, I wouldn't mind taking
>> care of that patch.
>
> Making such a change is fine with me, thanks.

The patch is trivial (*). But can we really make this change? And can we 
do it in emacs-25?

It's a breaking one, but it primarily affects xref (aside from the 
"obsolete" etags commands). And by doing it now, we can avoid similar 
breakage in the next Emacs release.

The only other place find-tag-default-bounds is used, is in 
isearch-forward-symbol-at-point. Yuri, was there a reason you chose to 
use it instead of `bounds-of-thing-at-point'?

> If find-tag-default-function is also used by xref-find-references,
> then it won't be TRT to reject constants up front.  A request to find
> all the places where a certain constant is used is a valid use case.
> It is indeed unlikely to have such a request for the constant 1, but
> think about constants like 3.14159 or 3.0e+8.

That's a valid argument. I also don't think numeric literals will create 
too many false positives, even for xref-find-definitions.

(*)

diff --git a/lisp/subr.el b/lisp/subr.el
index c685f95..5f88d99 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2626,29 +2626,8 @@ find-tag-default-bounds
    "Determine the boundaries of the default tag, based on text at point.
  Return a cons cell with the beginning and end of the found tag.
  If there is no plausible default, return nil."
-  (let (from to bound)
-    (when (or (progn
-		;; Look at text around `point'.
-		(save-excursion
-		  (skip-syntax-backward "w_") (setq from (point)))
-		(save-excursion
-		  (skip-syntax-forward "w_") (setq to (point)))
-		(> to from))
-	      ;; Look between `line-beginning-position' and `point'.
-	      (save-excursion
-		(and (setq bound (line-beginning-position))
-		     (skip-syntax-backward "^w_" bound)
-		     (> (setq to (point)) bound)
-		     (skip-syntax-backward "w_")
-		     (setq from (point))))
-	      ;; Look between `point' and `line-end-position'.
-	      (save-excursion
-		(and (setq bound (line-end-position))
-		     (skip-syntax-forward "^w_" bound)
-		     (< (setq from (point)) bound)
-		     (skip-syntax-forward "w_")
-		     (setq to (point)))))
-      (cons from to))))
+  (require 'thingatpt)
+  (bounds-of-thing-at-point 'symbol))

  (defun find-tag-default ()
    "Determine default tag to search for, based on text at point.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Dmitry Gutov]

On 02/19/2016 10:24 PM, Eli Zaretskii wrote:
>> Cc: 22692@debbugs.gnu.org, m.kupfer@acm.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Fri, 19 Feb 2016 20:52:28 +0200
>>
>> On 02/19/2016 08:37 PM, Eli Zaretskii wrote:
>>
>>> No, I meant the change you wanted to make in finding the symbol at
>>> point, to avoid looking for it anywhere on the same line.
>>
>> Isn't that orthogonal to the question of using the word "valid"?
>
> Maybe.  I don't tend thinking of documentation as a collection of
> words.  When you change how the symbol is determined, I could reason
> about describing that.  I agree that the chances of having "valid"
> there are rather small.

Collection of behaviors, then. We're thinking of two axes. The one 
touched on in this subthread is "valid" (i.e. whether the symbol must be 
tested against the completion table before it can be used as the default 
value; apparently we've settled on "no").

The other is whether we look for the symbol "at" point, or are we also 
allowed to look "near" point (within the current line), if there's no 
symbol at point. So far we've got two votes for "no" as well.

bug#22692: docstring for xref-find-definitions

[Eli Zaretskii]

> Cc: m.kupfer@acm.org, 22692@debbugs.gnu.org, Juri Linkov <juri@linkov.net>
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 20 Feb 2016 03:24:18 +0200
> 
> On 02/19/2016 05:34 PM, Eli Zaretskii wrote:
> 
> >> I'd prefer to use the more precise behavior in find-tag-default-bounds
> >> as well. And if there's general agreement here, I wouldn't mind taking
> >> care of that patch.
> >
> > Making such a change is fine with me, thanks.
> 
> The patch is trivial (*). But can we really make this change? And can we 
> do it in emacs-25?

Yes and yes, IMO.

> It's a breaking one, but it primarily affects xref (aside from the 
> "obsolete" etags commands). And by doing it now, we can avoid similar 
> breakage in the next Emacs release.

Exactly my thoughts.

So I think you should push your change, unless Juri objects.

Thanks.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Eli Zaretskii]

> Cc: 22692@debbugs.gnu.org, m.kupfer@acm.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 20 Feb 2016 03:28:44 +0200
> 
> The other is whether we look for the symbol "at" point, or are we also 
> allowed to look "near" point (within the current line), if there's no 
> symbol at point. So far we've got two votes for "no" as well.

I think it's okay to find the symbol before or after point in the same
manner thingatpt does that.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Mike Kupfer]

Hmm, I'm afraid I've gotten lost in all the recent discussion.

If the documentation is made consistent with the (default) behavior,
that would address my core concern.  But it sounds like finding the
right wording ("at" versus "at or near") is a problem because different
backends could have different behavior.  If that's true, would "at, or
possibly near" (or something like that) work?

I'd still like for "valid" to be removed from "valid identifier" because
of the ambiguity about what counts as "valid" in this context.

mike

bug#22692: docstring for xref-find-definitions

[Mike Kupfer]

Dmitry Gutov wrote:

> On 02/19/2016 05:34 PM, Eli Zaretskii wrote:
> > If find-tag-default-function is also used by xref-find-references,
> > then it won't be TRT to reject constants up front.  A request to find
> > all the places where a certain constant is used is a valid use case.
> > It is indeed unlikely to have such a request for the constant 1, but
> > think about constants like 3.14159 or 3.0e+8.
> 
> That's a valid argument.

Agreed.

I suppose find-tag-default-function could be enhanced to take an
optional flag that says whether the request is for finding references or
for finding the definition.  Though I'm not sure the additional
complexity is worth it.  And it would certainly not be part of
addressing this bug.

mike

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Dmitry Gutov]

On 02/21/2016 05:36 AM, Mike Kupfer wrote:
> But it sounds like finding the
> right wording ("at" versus "at or near") is a problem because different
> backends could have different behavior.  If that's true, would "at, or
> possibly near" (or something like that) work?

Hopefully not. After all, the generic function that the backends 
implement is called xref-backend-identifier-at-point.

> I'd still like for "valid" to be removed from "valid identifier" because
> of the ambiguity about what counts as "valid" in this context.

Right.

bug#22692: docstring for xref-find-definitions

[Juri Linkov]

>> It's a breaking one, but it primarily affects xref (aside from the
>> "obsolete" etags commands). And by doing it now, we can avoid similar
>> breakage in the next Emacs release.
>
> Exactly my thoughts.
>
> So I think you should push your change, unless Juri objects.

I'm fine with using bounds-of-thing-at-point if you think it's OK
to require thingatpt in core packages such as isearch.

bug#22692: docstring for xref-find-definitions

[Dmitry Gutov]

On 02/23/2016 02:04 AM, Juri Linkov wrote:

> I'm fine with using bounds-of-thing-at-point if you think it's OK
> to require thingatpt in core packages such as isearch.

Great, I've pushed that change. Requiring thingatpt doesn't seem to be 
something significant, as long as we don't have to preload it (and we 
don't, in this case).

The new behavior also seems to fit the related docstrings better: 
find-tag-default-bounds, find-tag-default, find-tag-default-as-regexp, 
find-tag-default-as-symbol-regexp all say "at point", not "near" or "on 
the same line", so it seems we don't even need a NEWS entry here.

bug#22692: docstring for xref-find-definitions

[Drew Adams]

> >> It's a breaking one, but it primarily affects xref (aside from the
> >> "obsolete" etags commands). And by doing it now, we can avoid similar
> >> breakage in the next Emacs release.
> >
> > Exactly my thoughts.
> > So I think you should push your change, unless Juri objects.
> 
> I'm fine with using bounds-of-thing-at-point if you think it's OK
> to require thingatpt in core packages such as isearch.

Wrt `bounds-of-thing-at-point', please consider fixing bug #9300.
The fix was provided, and it is trivial.

See also `bounds-of-thing-at-point' bug #8667.

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Dmitry Gutov]

On 02/19/2016 05:59 PM, Eli Zaretskii wrote:

>>> Does "valid identifier" mean syntatically correct, or does it mean that
>>> the identifier is in the tags table.  Please clarify the documentation.
>>
>> We probably should just remove the word "valid" from there.
>
> That depends on how you will change the behavior ;-)

I've changed it like we discussed.

However, after reading the quoted text, I'm not quite sure we need to 
remove "valid". To my eyes, "valid identifier" in that text means 
"syntactically valid", rather than "an identifier defined somewhere in 
the current project". The latter would be "an existing identifier", I guess?

But that's my impression. If that word raises questions, maybe it doe 
need clarification, or a replacement. The only option that came to my 
mind, however, is more unwieldy: "discernible identifier", and it 
doesn't mean the same thing.

The two other uses of "valid identifier" we have inside Emacs (in 
cc-vars.el and idlwave.el) also mean "syntactically valid".

Mike, thoughts?

bug#22692: 25.0.91; xref-find-definitions fails to prompt

[Mike Kupfer]

Dmitry Gutov wrote:

> I've changed it like we discussed.

Thanks!

> However, after reading the quoted text, I'm not quite sure we need to
> remove "valid". To my eyes, "valid identifier" in that text means
> "syntactically valid", rather than "an identifier defined somewhere in
> the current project". The latter would be "an existing identifier", I
> guess?
[...]
> The two other uses of "valid identifier" we have inside Emacs (in
> cc-vars.el and idlwave.el) also mean "syntactically valid".

In cc-vars.el, "syntactically valid identifiers" appears earlier in the
paragraph, so when "valid identifiers" appears later, the interpretation
is clear.

In idlwave.el, "valid identifier" is used in a comment.  I think it's
ambiguous (on its own, without studying the surrounding code), but since
it's just a comment, I don't care so much about it.

(This is as of Emacs 25.0.91; I didn't check the latest in the emacs-25
git branch.)

> Mike, thoughts?

I have a slight preference for removing "valid" from the Info text,
since that change makes the text a little more concise.  But I'd also be
fine with changing to it to "syntactically valid identifier".

cheers,
mike

bug#22692: docstring for xref-find-definitions

[Eli Zaretskii]

> Cc: m.kupfer@acm.org, 22692@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 23 Feb 2016 02:35:12 +0200
> 
> On 02/23/2016 02:04 AM, Juri Linkov wrote:
> 
> > I'm fine with using bounds-of-thing-at-point if you think it's OK
> > to require thingatpt in core packages such as isearch.
> 
> Great, I've pushed that change. Requiring thingatpt doesn't seem to be 
> something significant, as long as we don't have to preload it (and we 
> don't, in this case).
> 
> The new behavior also seems to fit the related docstrings better: 
> find-tag-default-bounds, find-tag-default, find-tag-default-as-regexp, 
> find-tag-default-as-symbol-regexp all say "at point", not "near" or "on 
> the same line", so it seems we don't even need a NEWS entry here.

The current doc string looks OK to me: it describes the behavior very
well.

Thanks.

bug#22692: docstring for xref-find-definitions

[Dmitry Gutov]

On 02/27/2016 12:32 PM, Eli Zaretskii wrote:

> The current doc string looks OK to me: it describes the behavior very
> well.

Great. I believe the only thing holding this bug is making the choice 
about the "valid" in the manual.

bug#22692: docstring for xref-find-definitions

[Eli Zaretskii]

> Cc: 22692@debbugs.gnu.org, juri@linkov.net, m.kupfer@acm.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 27 Feb 2016 14:35:20 +0200
> 
> On 02/27/2016 12:32 PM, Eli Zaretskii wrote:
> 
> > The current doc string looks OK to me: it describes the behavior very
> > well.
> 
> Great. I believe the only thing holding this bug is making the choice 
> about the "valid" in the manual.

You mean, whether to remove the "valid" part from the 2nd sentence
reproduced below?

     ‘M-.’ (‘xref-find-definitions’) shows the definitions of the
  identifier at point.  With a prefix argument, or if there’s no valid
  identifier at point, it prompts for the identifier.            ^^^^^

I'd say let's remove it.

bug#22692: docstring for xref-find-definitions

[Dmitry Gutov]

On 02/27/2016 02:43 PM, Eli Zaretskii wrote:

> You mean, whether to remove the "valid" part from the 2nd sentence
> reproduced below?
>
>      ‘M-.’ (‘xref-find-definitions’) shows the definitions of the
>   identifier at point.  With a prefix argument, or if there’s no valid
>   identifier at point, it prompts for the identifier.            ^^^^^
>
> I'd say let's remove it.

Indeed, that seems the easier choice. Pushed, and marking this done.