bug#23179: 25.0.92; Restore `M-,' to continue etags search

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

Hi!

I often use "etags" to search in files. The key binding `M-,' used to be
bound to `tags-loop-continue', a generic command used to continue the last
tags operation, like `tags-search' and `tags-query-replace'.

In Emacs 25.0.92, `M-,' is bound to `xref-pop-marker-stack', whereas
`tags-loop-continue' is unbound.

Clearly, this breaks things for existing etags users and brings very little
in return. (I expect `xref-pop-marker-stack' to be used relatively seldom.)

I suggest that we change the key layout to the following:

 * Bind `xref-pop-marker-stack' to another location, say, `C-x M-.',
alternatively make `C-u M-.' pop the state. (This is modeled after the key
binding used to pop the mark.)

 * Restore `M-,' to allow continuing the last tags command. (Of course,
this doesn't have to be `tags-loop-continue', it could also be an
equivalent xref command, should one exist.)

Sincerely,
    Anders Lindgren



In GNU Emacs 25.0.92.1 (i686-w64-mingw32)
 of 2016-03-21 built on LAPHROAIG
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Configured using:
 'configure --host=i686-w64-mingw32 --without-dbus
 --without-compress-install CFLAGS=-static'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS

Important settings:
  value of $LANG: SVE
  locale-coding-system: cp1252

Major mode: C++/l

Minor modes in effect:
  subword-mode: t
  doxygen-mode: t
  c-align-operands-electric-mode: t
  shell-dirtrack-mode: t
  dynamic-spaces-global-mode: t
  dynamic-spaces-mode: t
  char-font-lock-global-mode: t
  char-font-lock-mode: t
  global-auto-revert-mode: t
  global-cwarn-mode: t
  cwarn-mode: t
  preproc-font-lock-global-mode: t
  preproc-font-lock-mode: t
  highlight-doxygen-global-mode: t
  highlight-doxygen-mode: t
  lisp-extra-font-lock-global-mode: t
  global-edit-server-edit-mode: t
  highlight2clipboard-mode: t
  minibuffer-electric-file-mode: t
  recentf-mode: t
  msb-mode: t
  multicolumn-global-mode: t
  display-time-mode: t
  tooltip-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
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Recent messages:
Quit

Scanning file e:/src/Mystro-430/430/src/ibe/TaEnterLeaveGenerator.h...
Scanning file e:/src/Mystro-430/430/src/ibe/TaFloat.h...
Scanning file e:/src/Mystro-430/430/src/ibe/TaFunctionGenerator.cpp...
Scanning file e:/src/Mystro-430/430/src/ibe/TaFunctionGenerator.h...
Scanning file e:/src/Mystro-430/430/src/ibe/TaGoSetup.cpp...found
 [2 times]
Quit
Making completion list... [2 times]

Load-path shadows:
e:/home/AndersL/emacs/lisp/table hides e:/Program
Files/emacs-25.0.92/share/emacs/25.0.92/lisp/textmodes/table
e:/home/AndersL/emacs/src/asm-mode-new/src/asm-mode hides e:/Program
Files/emacs-25.0.92/share/emacs/25.0.92/lisp/progmodes/asm-mode
e:/home/AndersL/.emacs.d/elpa/25.0.92.x/helm-core-20160331.118/helm-multi-match
hides
e:/home/AndersL/.emacs.d/elpa/25.0.92.x/helm-20160331.118/helm-multi-match
e:/home/AndersL/emacs/src/misc/c-clean-buffer hides
e:/src/emacs-modules/IAR/c-clean-buffer
e:/home/AndersL/emacs/lisp/wikipedia-mode hides
e:/src/emacs-modules/lisp/wikipedia-mode
e:/home/AndersL/emacs/src/misc/stdify hides e:/src/emacs-modules/lisp/stdify
e:/Program Files/emacs-25.0.92/share/emacs/25.0.92/lisp/progmodes/ruby-mode
hides e:/src/emacs-modules/lisp/ruby-mode
e:/home/AndersL/emacs/src/misc/preproc hides
e:/src/emacs-modules/lisp/preproc
e:/home/AndersL/emacs/src/misc/preproc-indent hides
e:/src/emacs-modules/lisp/preproc-indent
e:/home/AndersL/emacs/lisp/gnuserv hides e:/src/emacs-modules/lisp/gnuserv
e:/home/AndersL/emacs/lisp/dsvn hides e:/src/emacs-modules/lisp/dsvn
e:/home/AndersL/emacs/src/misc/ctypes hides e:/src/emacs-modules/lisp/ctypes
e:/home/AndersL/emacs/lisp/column-marker hides
e:/src/emacs-modules/lisp/column-marker
e:/home/AndersL/emacs/lisp/cmake-mode hides
e:/src/emacs-modules/lisp/cmake-mode
e:/home/AndersL/emacs/src/misc/c-indent-operator hides
e:/src/emacs-modules/lisp/c-indent-operator
e:/home/AndersL/emacs/src/misc/c-electric-operator hides
e:/src/emacs-modules/lisp/c-electric-operator

Features:
(shadow sort mail-extr emacsbug message format-spec rfc822 mml mml-sec
epg epg-config gnus-util mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils tags-extra iartags-visit-tags
t2-config cperl-mode eieio-opt speedbar sb-image ezimage dframe
find-func help-fns dabbrev macrostep-c subr-x cmacexp macrostep pp
end-of-buffer-log cap-words superword subword doxygen c-align-operands
shell pcomplete grep compile thingatpt etags xref project eieio byte-opt
bytecomp byte-compile cconv eieio-core ruby-mode smie dired misearch
multi-isearch cl-extra help-mode cl-seq follow vc-dispatcher asm-mode
cmake-cache ps-print ps-def lpr iaremacs-init t2-log-mode
t2-show-config-mode lockdir project-name view-all-targets edg-mode
site-start c-electric-operator vc-svn warnings server dynamic-spaces
char-font-lock autorevert filenotify folding-isearch folding tail-mode
view cwarn cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs preproc-font-lock objc-font-lock
highlight-doxygen lisp-extra-font-lock edit-server highlight2clipboard
htmlize ange-ftp comint ansi-color ring paren mic-paren iso-insert
minibuf-elfile recentf tree-widget wid-edit msb multicolumn edmacro
kmacro easy-mmode autoload lisp-mnt finder-inf package easymenu time
lindydancer-theme old-emacs-support cl-macs derived advice cl gv
cl-loaddefs pcase cl-lib time-date mule-util tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp
disp-table w32-win w32-vars term/common-win 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 w32notify w32 multi-tty
make-network-process emacs)

Memory information:
((conses 8 355561 29597)
 (symbols 32 34706 32)
 (miscs 32 367 1596)
 (strings 16 69697 11069)
 (string-bytes 1 2545738)
 (vectors 8 34379)
 (vector-slots 4 1413731 35946)
 (floats 8 581 507)
 (intervals 28 22154 12)
 (buffers 520 39))

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

We've discussed this issue a few times already.

On 04/01/2016 11:55 AM, Anders Lindgren wrote:

> Clearly, this breaks things for existing etags users and brings very
> little in return. (I expect `xref-pop-marker-stack' to be used
> relatively seldom.)

I use it all the time.

>  * Bind `xref-pop-marker-stack' to another location, say, `C-x M-.',
> alternatively make `C-u M-.' pop the state. (This is modeled after the
> key binding used to pop the mark.)

That sounds much less convenient than the current binding.

>  * Restore `M-,' to allow continuing the last tags command. (Of course,
> this doesn't have to be `tags-loop-continue', it could also be an
> equivalent xref command, should one exist.)

There's no such command.

If you use `find-tag' often, you've probably rebound `M-.', and doing 
the same with `M-,' in your personal init file shouldn't be a problem.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

Hi!


>  * Restore `M-,' to allow continuing the last tags command. (Of course,
>> this doesn't have to be `tags-loop-continue', it could also be an
>> equivalent xref command, should one exist.)
>>
>
> There's no such command.
>

Then, how do you search through a set of files using `xref'? Is that even
possible?


If you use `find-tag' often, you've probably rebound `M-.', and doing the
> same with `M-,' in your personal init file shouldn't be a problem.


No, I haven't rebound `M-.', the default binding (`xref-find-definitions')
works perfectly well. For me, personally, it would not be a problem to
rebind `M-,' (After using Emacs for 25 years, I could teach it to dance if
I wanted to).

The problem is that we should provide a decent default value for the rest
of the world. Currently, there are no suitable key binding to continue a
tags search, and the built-in documentation doesn't offer any help.


 * Bind `xref-pop-marker-stack' to another location, say, `C-x M-.',
>> alternatively make `C-u M-.' pop the state. (This is modeled after the
>> key binding used to pop the mark.)
>>
>
> That sounds much less convenient than the current binding.
>

It all boils down to which commands should get access to the premium key
bindings. I don't think `xref-pop-marker-stack' qualifies, if it means that
continuing a tags search no longer has a key.

Of course, if you feel otherwise, you can always bind it in you personal
init file.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Fri, 1 Apr 2016 12:35:22 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: 23179@debbugs.gnu.org
> 
>  * Restore `M-,' to allow continuing the last tags command. (Of course,
>  this doesn't have to be `tags-loop-continue', it could also be an
>  equivalent xref command, should one exist.)
> 
>  There's no such command.
> 
> Then, how do you search through a set of files using `xref'? Is that even possible?

You do that by using the XREF buffer and the special commands
available there.

> The problem is that we should provide a decent default value for the rest of the world. Currently, there are no
> suitable key binding to continue a tags search, and the built-in documentation doesn't offer any help.

I think the only viable solution is to provide a replacement for
tags-search that uses the xref UI to browse the results.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/01/2016 02:03 PM, Eli Zaretskii wrote:

>> Then, how do you search through a set of files using `xref'? Is that even possible?
>
> You do that by using the XREF buffer and the special commands
> available there.

You can also use next-error and previous-error. If the question is about 
navigating through search results, of course.

> I think the only viable solution is to provide a replacement for
> tags-search that uses the xref UI to browse the results.

By now, we have both project-find-regexp and dired-do-find-regexp, which 
provide different takes on the issue of "search through a set of files".

If you want to have an xref-find-regexp as well, we should first 
understand clearly how it would differ from those two commands. And its 
specification cannot refer to tags files directly.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Cc: 23179@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 2 Apr 2016 02:44:57 +0300
> 
> > I think the only viable solution is to provide a replacement for
> > tags-search that uses the xref UI to browse the results.
> 
> By now, we have both project-find-regexp and dired-do-find-regexp, which 
> provide different takes on the issue of "search through a set of files".

Depending on the specific use case (i.e. what is being looked for by
using tags-search), xref-find-references or xref-find-apropos or
xref-query-replace-in-results might also be relevant.

> If you want to have an xref-find-regexp as well, we should first 
> understand clearly how it would differ from those two commands. And its 
> specification cannot refer to tags files directly.

But we could have a tags-only command that presented an xref UI, I
think.  (Its name could be "tags-search" ;-)

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/02/2016 09:58 AM, Eli Zaretskii wrote:

> But we could have a tags-only command that presented an xref UI, I
> think.  (Its name could be "tags-search" ;-)

Probably not tags-search; we're only deprecating some etags commands, 
but not removing them (yet?). And why is etags so special that it needs 
its own find-regexp command, but other backends don't?

Anyway, this should get you going:

(defun tags-find-regexp (regexp)
   (interactive "sTags search (regexp): ")
   (let* ((files
           (save-excursion
             (let ((enable-recursive-minibuffers t))
               (visit-tags-table-buffer))
             (mapcar #'expand-file-name (tags-table-files))))
          (xrefs (cl-mapcan
                  (lambda (file)
                    (xref-collect-matches regexp "*" file nil))
                  files)))
     (unless xrefs
       (user-error "No matches for: %s" regexp))
     (xref--show-xrefs xrefs nil t)))

(I don't know if calling 'find' once per file is an actual problem, but 
it's likely suboptimal; we'll fix that in a unified fashion later).

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Cc: 23179@debbugs.gnu.org, andlind@gmail.com
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 3 Apr 2016 02:39:47 +0300
> 
> On 04/02/2016 09:58 AM, Eli Zaretskii wrote:
> 
> > But we could have a tags-only command that presented an xref UI, I
> > think.  (Its name could be "tags-search" ;-)
> 
> Probably not tags-search; we're only deprecating some etags commands, 
> but not removing them (yet?).

Fine with me.

> And why is etags so special that it needs its own find-regexp
> command, but other backends don't?

Etags isn't special, I just remembered that you said at some point you
couldn't see how other back-ends will be able to implement a similar
functionality.  But if I misremembered, or if you now see a way to do
this with other back-ends, by all means let's do that.

> Anyway, this should get you going:

Thanks, this looks good to me.

Anders, can you see if this provides a solution to your problem?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/03/2016 06:32 PM, Eli Zaretskii wrote:

> Etags isn't special, I just remembered that you said at some point you
> couldn't see how other back-ends will be able to implement a similar
> functionality.  But if I misremembered, or if you now see a way to do
> this with other back-ends, by all means let's do that.

Not sure what I said previously, but first and foremost, we don't have a 
backend-agnostic spec for that similar functionality.

When we have it, I'm sure other backends can implement regexp-search to 
the best of their ability.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Cc: 23179@debbugs.gnu.org, andlind@gmail.com
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 3 Apr 2016 20:21:13 +0300
> 
> On 04/03/2016 06:32 PM, Eli Zaretskii wrote:
> 
> > Etags isn't special, I just remembered that you said at some point you
> > couldn't see how other back-ends will be able to implement a similar
> > functionality.  But if I misremembered, or if you now see a way to do
> > this with other back-ends, by all means let's do that.
> 
> Not sure what I said previously, but first and foremost, we don't have a 
> backend-agnostic spec for that similar functionality.

That's probably what you said.

> When we have it, I'm sure other backends can implement regexp-search to 
> the best of their ability.

Searching is not the problem, indeed.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

On Sun, Apr 3, 2016 at 1:39 AM, Dmitry Gutov <dgutov@yandex.ru> wrote:

> On 04/02/2016 09:58 AM, Eli Zaretskii wrote:
>
> But we could have a tags-only command that presented an xref UI, I
>> think.  (Its name could be "tags-search" ;-)
>>
>
> Probably not tags-search; we're only deprecating some etags commands, but
> not removing them (yet?). And why is etags so special that it needs its own
> find-regexp command, but other backends don't?
>

Ideally, xref should define a regexp search command that work with all
backends. The only thing tags-specific in your code is getting the list of
files -- if you would expand the xref backend interface with a
corresponding function it would work with all backends.

Dmitry:

> Anyway, this should get you going: [tags-find-regexp]
>

Eli:
> Anders, can you see if this provides a solution to your problem?

I gave the code a test. It's a step in the right direction. However, I
found some problems with the approach (even though not all are caused by
xref):

* Unlike `tags-search', it search through all source files before
presenting the first match. The traditional `tags-search' stop of the first
match, and continue searching when the used pressed `M-,'. The effect is
that it becomes much, much slower to find the first match [+++]. I would
suggest that xref should provide two kinds of searches: one incremental
(like `tags-search') and one `find-all' (like the provided function). You
could think of `isearch' vs. `occur'. It would be fine with me if
`next-error' would be used to restart the incremental search (even though I
would probably bind it to `M-,').

* There is no need for a xref UI window when doing an incremental search or
query-replace. It just occupies precious screen real estate.

* The xref UI window is not updated to reflect the current location. For
example, in a *grep* buffer, the cursor move and an arrow in the left
fringe reflect the current location.

* I like the touch that the matches in the *xref* buffer are syntax
highlighted. Unfortunately, not all matches are highlighted. It appears as
though only matches in previously viewed parts of source files retain
syntax highlighting.

* `next-error' issued from an *xref* search don't reuse the source windows
(whereas a `next-error' issued from a grep buffer does). I use six
side-by-side windows, and if I step through matches found across several
source files, after a while all windows will be occupied by source files
where matches were found.

* `next-error' in ChangeLog buffers cause Emacs to go to the corresponding
change. This makes it hard to step past irrelevant xref matches if they
occur a ChangeLog file.


+++ Using "etags *.h *.m *.c" in the Emacs "src" directory, `(tags-search
"nstrace")' find the first occurrence in 0.7 seconds, whereas the new
`tags-find-regexp' takes over 8 seconds to perform a full search.


John:

> and what is the equivalent of tags-query-replace?
>

Dmtry:
> xref-query-replace-in-results

This is not quite true. `tags-query-replace' do an incremental search and
replace in the source files referred to by a tags file directly.
`xref-query-replace-in-results' requires a user to first do a search (to
get stuff into the xref buffer), then run the command to replace. It sounds
very awkward to me.

In other words, I would prefer if we would add an incremental xref
query-replace command along the lines I suggested for the search command
above.


Finally, if all the tags commands should be made obsolete, their
documentations should be updated with references to the commands that are
intended to replace them.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Sun, 3 Apr 2016 20:32:17 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 23179@debbugs.gnu.org
> 
> > Anders, can you see if this provides a solution to your problem?
> 
> I gave the code a test. It's a step in the right direction. However, I found some problems with the approach

Are these problems grave enough that you'd prefer not to use such a
new command?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

On Sun, Apr 3, 2016 at 8:42 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Sun, 3 Apr 2016 20:32:17 +0200
> > From: Anders Lindgren <andlind@gmail.com>
> > Cc: Eli Zaretskii <eliz@gnu.org>, 23179@debbugs.gnu.org
> >
> > > Anders, can you see if this provides a solution to your problem?
> >
> > I gave the code a test. It's a step in the right direction. However, I
> found some problems with the approach
>
> Are these problems grave enough that you'd prefer not to use such a
> new command?
>

Without a doubt, yes.

I might use some other features of xref, but I would still be using the old
tags-search and tags-query-replace commands, as the situation is right now.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Sun, 3 Apr 2016 20:49:11 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Brief Busters <dgutov@yandex.ru>, 23179@debbugs.gnu.org
> 
>  > I gave the code a test. It's a step in the right direction. However, I found some problems with the
>  approach
> 
>  Are these problems grave enough that you'd prefer not to use such a
>  new command?
> 
> Without a doubt, yes.
> 
> I might use some other features of xref, but I would still be using the old tags-search and tags-query-replace
> commands, as the situation is right now.

What is the minimum set of changes that will cause you to change your
mind?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

>
> What is the minimum set of changes that will cause you to change your
> mind?
>

Incremental search and query-replace commands (that doesn't show the xref
UI).

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Sun, 3 Apr 2016 21:11:28 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Brief Busters <dgutov@yandex.ru>, 23179@debbugs.gnu.org
> 
>  What is the minimum set of changes that will cause you to change your
>  mind?
> 
> Incremental search and query-replace commands (that doesn't show the xref UI).

Why is showing the UI a problem?  You can always delete the window if
you don't want to see it.  "C-x 0" is all it takes.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Andy Moreton]

On Sun 03 Apr 2016, Eli Zaretskii wrote:

>> Date: Sun, 3 Apr 2016 21:11:28 +0200
>> From: Anders Lindgren <andlind@gmail.com>
>> Cc: Brief Busters <dgutov@yandex.ru>, 23179@debbugs.gnu.org
>> 
>>  What is the minimum set of changes that will cause you to change your
>>  mind?
>> 
>> Incremental search and query-replace commands (that doesn't show the xref UI).
>
> Why is showing the UI a problem?  You can always delete the window if
> you don't want to see it.  "C-x 0" is all it takes.

Why should this extra UI appear ? Emacs users who are accustomed to the
existing facilities will find it annoying, and start filing bug reports
about an obvious regression.

By all means add new facilites with xref, but without loss of existing
keybindings that many people have ingrained into muscle memory.

    AndyM

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Sun, 03 Apr 2016 21:15:01 +0100
> 
> Why should this extra UI appear ?

Because it is an integral part of the feature.

> Emacs users who are accustomed to the existing facilities will find
> it annoying, and start filing bug reports about an obvious
> regression.

So you are saying that any new features that present a UI never used
before is a bug?

> By all means add new facilites with xref, but without loss of existing
> keybindings that many people have ingrained into muscle memory.

That's impossible, and you know it.  Not with features that are
explicitly meant to replace the old ones.

Anyway, all these opinions should have been brought up many moons ago,
when these features were added to the development sources, and perhaps
even earlier, when their design and implementation was discussed here.
Coming up now, after so much efforts was invested in improving this
and documenting it, it's really too late, unless we want to delay the
release of Emacs 25.1 by another year or so.  If you don't like some
aspects of this feature, the constructive way forward is to submit
patches.

Thanks.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Andy Moreton]

On Mon 04 Apr 2016, Eli Zaretskii wrote:

>> From: Andy Moreton <andrewjmoreton@gmail.com>
>> Date: Sun, 03 Apr 2016 21:15:01 +0100
>> 
>> Why should this extra UI appear ?
>
> Because it is an integral part of the feature.
>
>> Emacs users who are accustomed to the existing facilities will find
>> it annoying, and start filing bug reports about an obvious
>> regression.
>
> So you are saying that any new features that present a UI never used
> before is a bug?

Of course not. The existing etags based facilities allow search for a
tag and then subsequent matches without showing any additional windows.
The new xref based stuff should keep this workflow.

>> By all means add new facilites with xref, but without loss of existing
>> keybindings that many people have ingrained into muscle memory.
>
> That's impossible, and you know it.  Not with features that are
> explicitly meant to replace the old ones.

Why ever not ? The interface stays the same, with a replacement
implementation. If that is not possible then the new design need to be
reworked.

> Anyway, all these opinions should have been brought up many moons ago,
> when these features were added to the development sources, and perhaps
> even earlier, when their design and implementation was discussed here.
> Coming up now, after so much efforts was invested in improving this
> and documenting it, it's really too late, unless we want to delay the
> release of Emacs 25.1 by another year or so.  If you don't like some
> aspects of this feature, the constructive way forward is to submit
> patches.

New features are fine, as long as the existing keybindings are retained
with similar functionality. M-, should continue to function as it did
for etags, and the new xref functionality should be moved to a different
binding. Changing long-standing bindings is a disservice to users.

    AndyM

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> From: Andy Moreton <andrewjmoreton@gmail.com>
> Date: Mon, 04 Apr 2016 09:46:38 +0100
> 
> > So you are saying that any new features that present a UI never used
> > before is a bug?
> 
> Of course not. The existing etags based facilities allow search for a
> tag and then subsequent matches without showing any additional windows.
> The new xref based stuff should keep this workflow.

We are miscommunicating.  The new xref stuff is a new feature that
presents a UI never used before.  It cannot keep the tags-based
workflow, because it specifically replaces it with a new one.  And you
said you didn't think such new features are necessarily a bug.  So I
guess we are in violent agreement here.

> >> By all means add new facilites with xref, but without loss of existing
> >> keybindings that many people have ingrained into muscle memory.
> >
> > That's impossible, and you know it.  Not with features that are
> > explicitly meant to replace the old ones.
> 
> Why ever not ?

Because xref wants to replace the old features, not just their
implementation.

> > Anyway, all these opinions should have been brought up many moons ago,
> > when these features were added to the development sources, and perhaps
> > even earlier, when their design and implementation was discussed here.
> > Coming up now, after so much efforts was invested in improving this
> > and documenting it, it's really too late, unless we want to delay the
> > release of Emacs 25.1 by another year or so.  If you don't like some
> > aspects of this feature, the constructive way forward is to submit
> > patches.
> 
> New features are fine, as long as the existing keybindings are retained
> with similar functionality.

Which they are.

> M-, should continue to function as it did for etags

The function M-, invoked for etags is no longer needed with xref.

> Changing long-standing bindings is a disservice to users.

We indeed don't change them too easily, but sometimes we do.  There's
nothing wrong with that, as long as the reasons are good.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

>
> Why is showing the UI a problem?  You can always delete the window if
> you don't want to see it.  "C-x 0" is all it takes.
>

 I use a number of side-by-side windows, each typically contains one thing
I'm working on. If I do a search in one window, I don't want the content of
another to be obscured. C-x 0 is out of the question, as it would break the
side-by-side window layout.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Sun, 3 Apr 2016 22:30:34 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Brief Busters <dgutov@yandex.ru>, 23179@debbugs.gnu.org
> 
>  Why is showing the UI a problem? You can always delete the window if
>  you don't want to see it. "C-x 0" is all it takes.
> 
> I use a number of side-by-side windows, each typically contains one thing I'm working on. If I do a search in
> one window, I don't want the content of another to be obscured.

Sorry, I don't understand: how is this different from *Help* or *Completions*?

> C-x 0 is out of the question, as it would break the side-by-side
> window layout.

OK, then "C-x b" or "C-x 4 C-o" will do, right?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

>
> > I use a number of side-by-side windows, each typically contains one
> thing I'm working on. If I do a search in
> > one window, I don't want the content of another to be obscured.
>
> Sorry, I don't understand: how is this different from *Help* or
> *Completions*?
>

*Completions* go away once it's not needed anymore and *Help* is something
that I explicitly as for, so I don't consider them to be problems.

I only want the xref UI to be displayed when doing commands like
`find-all-occurences' etc. When doing a plain search, there is no need for
it.


> C-x 0 is out of the question, as it would break the side-by-side
> > window layout.
>
> OK, then "C-x b" or "C-x 4 C-o" will do, right?
>

There are ways to restore the window layout, but it would require an extra,
unnecessary step.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Mon, 4 Apr 2016 06:22:28 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Brief Busters <dgutov@yandex.ru>, 23179@debbugs.gnu.org
> 
>  Sorry, I don't understand: how is this different from *Help* or *Completions*?
> 
> *Completions* go away once it's not needed anymore and *Help* is something that I explicitly as for, so I
> don't consider them to be problems.
> 
> I only want the xref UI to be displayed when doing commands like `find-all-occurences' etc. When doing a
> plain search, there is no need for it.

Dmitry, is the patch below okay with you?  Any comments?  (I will add
documentation if this is acceptable.)

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index feed0fb..4ae4c06 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -342,6 +342,14 @@ xref-prompt-for-identifier
 		      (const :tag "Except" not)
 		      (repeat :inline t (symbol :tag "command")))))
 
+(defcustom xref-display-xref-buffer t
+  "When non-nil, xref commands that return multiple hits display XREF buffer.
+
+When nil, the XREF buffer is never shown, and \\[next-error] should
+be used to display the hits."
+  :type '(choice (const :tag "Display XREF buffer with multiple hits" t)
+                 (const :tag "Don't display XREF buffer" nil)))
+
 (defcustom xref-after-jump-hook '(recenter
                                   xref-pulse-momentarily)
   "Functions called after jumping to an xref."
@@ -691,9 +699,14 @@ xref--show-xref-buffer
         (erase-buffer)
         (xref--insert-xrefs xref-alist)
         (xref--xref-buffer-mode)
-        (pop-to-buffer (current-buffer))
+        (and xref-display-xref-buffer
+            (pop-to-buffer (current-buffer)))
         (goto-char (point-min))
         (setq xref--window (assoc-default 'window alist))
+        (unless xref-display-xref-buffer
+          (xref-next-line)
+          (message "%s" (substitute-command-keys
+                         "Use `\\[next-error]' to display further matches")))
         (current-buffer)))))
 
 \f

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/04/2016 06:49 PM, Eli Zaretskii wrote:

>> I only want the xref UI to be displayed when doing commands like `find-all-occurences' etc. When doing a
>> plain search, there is no need for it.
>
> Dmitry, is the patch below okay with you?  Any comments?  (I will add
> documentation if this is acceptable.)

First, it would take effect in both xref-find-definitions and 
xref-find-references, whereas Anders said that he wants that UI only for 
the former, IIUC.

Second, I'd rather you instead create a new separate function to set 
xref-show-xrefs-function to.

For now, you can copy the definition of xref--show-xref-buffer and 
modify it not to show the buffer. That will make it easier to make 
independent changes in that new UI. As well as make it easier for me to 
disclaim the responsibility for it (sorry).

Go ahead if Anders likes it, but personally I don't see a lot of point, 
at least as long as the user still has to wait until all results are 
fetched.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Cc: 23179@debbugs.gnu.org, Anders Lindgren <andlind@gmail.com>
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 4 Apr 2016 19:53:44 +0300
> 
> On 04/04/2016 06:49 PM, Eli Zaretskii wrote:
> 
> >> I only want the xref UI to be displayed when doing commands like `find-all-occurences' etc. When doing a
> >> plain search, there is no need for it.
> >
> > Dmitry, is the patch below okay with you?  Any comments?  (I will add
> > documentation if this is acceptable.)
> 
> First, it would take effect in both xref-find-definitions and 
> xref-find-references, whereas Anders said that he wants that UI only for 
> the former, IIUC.

Anders, is that so?  If so, can you tell with which commands you'd
like to see the UI and with which not to see it, and why?

> Second, I'd rather you instead create a new separate function to set 
> xref-show-xrefs-function to.
> 
> For now, you can copy the definition of xref--show-xref-buffer and 
> modify it not to show the buffer. That will make it easier to make 
> independent changes in that new UI. As well as make it easier for me to 
> disclaim the responsibility for it (sorry).

I hope you are joking.  Since when does anyone need to disclaim
responsibility for some code?  And even if someone does, "git blame"
will blame the guilty parties right away.

Copying a function only to change a line or two sounds extreme to me.

> Go ahead if Anders likes it, but personally I don't see a lot of point, 
> at least as long as the user still has to wait until all results are 
> fetched.

The point is to be able to tell people who, like Anders, don't want to
see the UI to use the option to get what they want, instead of arguing
with them trying to convince them that the UI is for their best.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/05/2016 06:12 PM, Eli Zaretskii wrote:

> I hope you are joking.  Since when does anyone need to disclaim
> responsibility for some code?  And even if someone does, "git blame"
> will blame the guilty parties right away.

I mostly mean feature requests, bugs-which-are-really-missing-features, 
and so on.

> Copying a function only to change a line or two sounds extreme to me.

The idea is that the other UI should really be a new 
xref-show-xrefs-function, that's what that variable is there for (you 
can make it a defcustom). It would be better to avoid coupling the two UIs.

> The point is to be able to tell people who, like Anders, don't want to
> see the UI to use the option to get what they want, instead of arguing
> with them trying to convince them that the UI is for their best.

I'm just wondering if the new looks were a minor part of his complaint, 
and not seeing the first match quickly, a more significant one. Anyway, 
I don't mind.

Here's a more technical concern: if you revisit the discussion 
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489, we've mentioned that 
next-error-find-buffer, like it's currently written, really wants the 
next-error-last-buffer to be visible. Otherwise, it's prone to switch to 
using next-error-function from some other, visible buffer.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Cc: 23179@debbugs.gnu.org, andlind@gmail.com
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 5 Apr 2016 18:27:57 +0300
> 
> On 04/05/2016 06:12 PM, Eli Zaretskii wrote:
> 
> > Copying a function only to change a line or two sounds extreme to me.
> 
> The idea is that the other UI should really be a new 
> xref-show-xrefs-function, that's what that variable is there for (you 
> can make it a defcustom). It would be better to avoid coupling the two UIs.

If the difference will prove to be more significant, I can see the
point.

> > The point is to be able to tell people who, like Anders, don't want to
> > see the UI to use the option to get what they want, instead of arguing
> > with them trying to convince them that the UI is for their best.
> 
> I'm just wondering if the new looks were a minor part of his complaint, 
> and not seeing the first match quickly, a more significant one.

It is, but IMO we need to solve both issues.

> Here's a more technical concern: if you revisit the discussion 
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489, we've mentioned that 
> next-error-find-buffer, like it's currently written, really wants the 
> next-error-last-buffer to be visible. Otherwise, it's prone to switch to 
> using next-error-function from some other, visible buffer.

I thought that if one invokes next-error immediately after xref
collected the matches, this danger is avoided.  Isn't that true?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/05/2016 06:56 PM, Eli Zaretskii wrote:

>> The idea is that the other UI should really be a new
>> xref-show-xrefs-function, that's what that variable is there for (you
>> can make it a defcustom). It would be better to avoid coupling the two UIs.
>
> If the difference will prove to be more significant, I can see the
> point.

The difference will hopefully increase in the future. And then we'll be 
happy that the user doesn't need to change their customization.

> I thought that if one invokes next-error immediately after xref
> collected the matches, this danger is avoided.  Isn't that true?

Not necessarily (e.g. if you have a Grep buffer visible). And a 
next-error-function-capable buffer can come up after one of the 
next-error invocations.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Cc: 23179@debbugs.gnu.org, andlind@gmail.com
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 5 Apr 2016 19:00:53 +0300
> 
> On 04/05/2016 06:56 PM, Eli Zaretskii wrote:
> 
> >> The idea is that the other UI should really be a new
> >> xref-show-xrefs-function, that's what that variable is there for (you
> >> can make it a defcustom). It would be better to avoid coupling the two UIs.
> >
> > If the difference will prove to be more significant, I can see the
> > point.
> 
> The difference will hopefully increase in the future. And then we'll be 
> happy that the user doesn't need to change their customization.

The defcustom can control also which function(s) are called.  Asking
users to customize options whose values are functions is something I
think we should avoid as much as possible.

> > I thought that if one invokes next-error immediately after xref
> > collected the matches, this danger is avoided.  Isn't that true?
> 
> Not necessarily (e.g. if you have a Grep buffer visible). And a 
> next-error-function-capable buffer can come up after one of the 
> next-error invocations.

I guess those who want the UI be hidden will have to consider this
caveat and deal with it, if and when it happens.  TANSTAAFL.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/05/2016 07:18 PM, Eli Zaretskii wrote:

> The defcustom can control also which function(s) are called.  Asking
> users to customize options whose values are functions is something I
> think we should avoid as much as possible.

Why? Just describe each option properly. The user will pick between 
descriptions, not between functions.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[John Wiegley]

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

>> The defcustom can control also which function(s) are called. Asking users
>> to customize options whose values are functions is something I think we
>> should avoid as much as possible.

> Why? Just describe each option properly. The user will pick between
> descriptions, not between functions.

We should avoid it because it's confusing for some people. For example,
`message-send-mail-function' is a good design, because it presents the user
with the most common options, while still allowing a custom function to be
chosen.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/05/2016 09:10 PM, John Wiegley wrote:

> We should avoid it because it's confusing for some people. For example,
> `message-send-mail-function' is a good design, because it presents the user
> with the most common options, while still allowing a custom function to be
> chosen.

What's confusing? And what is the difference between that variable, and 
what I'm proposing here?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[John Wiegley]

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04/05/2016 09:10 PM, John Wiegley wrote:
>> We should avoid it because it's confusing for some people. For example,
>> `message-send-mail-function' is a good design, because it presents the user
>> with the most common options, while still allowing a custom function to be
>> chosen.

> What's confusing? And what is the difference between that variable, and what
> I'm proposing here?

Are they not different? I just scanned back 10 messages but didn't find a
concrete proposal to compare with. Can you show your proposed defcustom again?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/05/2016 10:32 PM, John Wiegley wrote:
> Can you show your proposed defcustom again?

(defcustom xref-show-xrefs-function #'xref--show-xref-buffer
   "Function to display a list of xrefs.

Its signature should be (XREFS ALIST), where XREFS is a list of
xrefs, and ALIST is (...).  Valid values include
`xref--show-xref-buffer' and `xref--party-like-its-1985'."
   :type '(choice (const :tag "So progressive"
                         xref--show-xref-buffer)
                  (const :tag "Much conservative"
                         xref--party-like-its-1985)))

(Yes, I think that joke is hilarious.)

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[John Wiegley]

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04/05/2016 10:32 PM, John Wiegley wrote:
> > Can you show your proposed defcustom again?
> 
> (defcustom xref-show-xrefs-function #'xref--show-xref-buffer
>   "Function to display a list of xrefs.
> 
> Its signature should be (XREFS ALIST), where XREFS is a list of
> xrefs, and ALIST is (...).  Valid values include
> `xref--show-xref-buffer' and `xref--party-like-its-1985'."
>   :type '(choice (const :tag "So progressive"
>                         xref--show-xref-buffer)
>                  (const :tag "Much conservative"
>                         xref--party-like-its-1985)))
>
> (Yes, I think that joke is hilarious.)

Aside from the joke being unacceptable :), there still needs to be a third
choice, for specifying an arbitrary function.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/06/2016 03:55 AM, John Wiegley wrote:
>there still needs to be a third
> choice, for specifying an arbitrary function.

Sure, and we can also use `radio' and `function-item' like in 
message-send-mail-function's definition. My sole point is that a 
defcustom where the value is a function is a reasonable, and in this 
case a desirable, thing to do.

Maybe we shouldn't advertise the "write your own function" possibility 
too much, since the required signature can still change abruptly, but 
that's a minor concern.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Cc: 23179@debbugs.gnu.org, andlind@gmail.com
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 5 Apr 2016 20:40:03 +0300
> 
> On 04/05/2016 07:18 PM, Eli Zaretskii wrote:
> 
> > The defcustom can control also which function(s) are called.  Asking
> > users to customize options whose values are functions is something I
> > think we should avoid as much as possible.
> 
> Why? Just describe each option properly. The user will pick between 
> descriptions, not between functions.

Some users use "M-x set-variable", or some Lisp in their .emacs.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/05/2016 10:23 PM, Eli Zaretskii wrote:

> Some users use "M-x set-variable", or some Lisp in their .emacs.

(setq xref-show-xrefs-function #'xref--party-like-its-1985)

will work just as well in .emacs; the user will need to know the 
possible values, and for that they will consult the defcustom form. Like 
usual with custom variables.

We've had function-valued variables in company-mode, at least, for a few 
years, and nobody has complained about that choice.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Tue, 05 Apr 2016 18:12:25 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 23179@debbugs.gnu.org, andlind@gmail.com
> 
> > Cc: 23179@debbugs.gnu.org, Anders Lindgren <andlind@gmail.com>
> > From: Dmitry Gutov <dgutov@yandex.ru>
> > Date: Mon, 4 Apr 2016 19:53:44 +0300
> > 
> > On 04/04/2016 06:49 PM, Eli Zaretskii wrote:
> > 
> > >> I only want the xref UI to be displayed when doing commands like `find-all-occurences' etc. When doing a
> > >> plain search, there is no need for it.
> > >
> > > Dmitry, is the patch below okay with you?  Any comments?  (I will add
> > > documentation if this is acceptable.)
> > 
> > First, it would take effect in both xref-find-definitions and 
> > xref-find-references, whereas Anders said that he wants that UI only for 
> > the former, IIUC.
> 
> Anders, is that so?  If so, can you tell with which commands you'd
> like to see the UI and with which not to see it, and why?

Ping!  Anders, I'd appreciate an answer to that question, so that I
could figure out what to do with the patch I proposed.  TIA.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

>
> > Anders, is that so?  If so, can you tell with which commands you'd
> > like to see the UI and with which not to see it, and why?
>
> Ping!  Anders, I'd appreciate an answer to that question, so that I
> could figure out what to do with the patch I proposed.  TIA.
>

Oh, I missed that question.

I would like to have one set of commands to do incremental search and
query-replace without a xref UI and one set that do a full search and opens
the UI.

The incremental versions could work more or less like tags-search and
tags-query-replace do today, but they should work with all(ish) xref
backends.

Think of this like C-s vs. M-x occur in a plain buffer.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Fri, 8 Apr 2016 10:56:24 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Brief Busters <dgutov@yandex.ru>, 23179@debbugs.gnu.org
> 
> I would like to have one set of commands to do incremental search and query-replace without a xref UI and
> one set that do a full search and opens the UI.
> 
> 
> The incremental versions could work more or less like tags-search and tags-query-replace do today, but they
> should work with all(ish) xref backends.
> 
> 
> Think of this like C-s vs. M-x occur in a plain buffer.

So does having a customizable option which switches between UI and
no-UI versions sound like a step in the right direction?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

>
> So does having a customizable option which switches between UI and
> no-UI versions sound like a step in the right direction?
>

No. I want to be able to access both versions using different commands.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Fri, 8 Apr 2016 12:28:49 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Brief Busters <dgutov@yandex.ru>, 23179@debbugs.gnu.org
> 
>  So does having a customizable option which switches between UI and
>  no-UI versions sound like a step in the right direction?
> 
> No. I want to be able to access both versions using different commands.

If we have an option, then adding a command that binds that option to
one value or the other is very simple.  So I'm puzzled why you don't
see this as a step in the right direction.  Can you point me to what
I'm missing here?

Thanks.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/08/2016 01:32 PM, Eli Zaretskii wrote:

> If we have an option, then adding a command that binds that option to
> one value or the other is very simple.

Another approach, a bit more complex, would be to implement a sort of 
prefix command that would temporarily change that option for the 
duration of the next command.

(I don't have a code example.)

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

>
> If we have an option, then adding a command that binds that option to
> one value or the other is very simple.  So I'm puzzled why you don't
> see this as a step in the right direction.  Can you point me to what
> I'm missing here?
>

There are two features of tags-search that I would like to see retained in
an xref shape. The first is the "incremental" bit, that matches are
displayed directly when found (but also that it respects the current point
position -- if I move the point the the end of a buffer, the rest of that
buffer is skipped, etc.) The second is the be able to do the search without
the xref UI.

The patch only address the second case, if I understand correctly.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/08/2016 01:53 PM, Anders Lindgren wrote:

> (but also that it respects the current
> point position -- if I move the point the the end of a buffer, the rest
> of that buffer is skipped, etc.)

That seems like an implementation-defined behavior. Are we striving to 
create a UI that works as close to the previous one as possible? That's 
definitely doable, though (it'll need a wrapper for the current 
next-error-function).

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Fri, 8 Apr 2016 12:53:37 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Brief Busters <dgutov@yandex.ru>, 23179@debbugs.gnu.org
> 
>  If we have an option, then adding a command that binds that option to
>  one value or the other is very simple. So I'm puzzled why you don't
>  see this as a step in the right direction. Can you point me to what
>  I'm missing here?
> 
> There are two features of tags-search that I would like to see retained in an xref shape. The first is the
> "incremental" bit, that matches are displayed directly when found (but also that it respects the current point
> position -- if I move the point the the end of a buffer, the rest of that buffer is skipped, etc.) The second is the
> be able to do the search without the xref UI.
> 
> The patch only address the second case, if I understand correctly.

Indeed, the patch addresses only one of the two features you
requested.  But doing that exactly means a "step in the right
direction".  So I'm unsure why you still disagree that it is a step in
the right direction.  I didn't say that it's a complete solution, just
a part of it.  The other part still needs to be implemented.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Sun, 3 Apr 2016 20:32:17 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 23179@debbugs.gnu.org
> 
> * Unlike `tags-search', it search through all source files before presenting the first match. The traditional
> `tags-search' stop of the first match, and continue searching when the used pressed `M-,'. The effect is that it
> becomes much, much slower to find the first match [+++]. I would suggest that xref should provide two kinds
> of searches: one incremental (like `tags-search') and one `find-all' (like the provided function). You could think
> of `isearch' vs. `occur'. It would be fine with me if `next-error' would be used to restart the incremental search
> (even though I would probably bind it to `M-,').

OTOH, seeing all of the hits allows you to find the one(s) you are
looking for much faster, since you don't need to visit them one by
one.  Another nice side effect is that you don't end up visiting all
of the files you needed to look through until you find the hit(s) you
were really looking for.

So this change has upsides as well, not only downsides.  I agree that
IWBNI there was an incremental version, although I'm not sure I'd like
it to bring me one hit at a time, I'd rather see a larger chunk.

> * There is no need for a xref UI window when doing an incremental search or query-replace. It just occupies
> precious screen real estate.

The UI window is the one that allows you to jump to the hit you are
looking for quickly.

> * The xref UI window is not updated to reflect the current location. For example, in a *grep* buffer, the cursor
> move and an arrow in the left fringe reflect the current location.

The cursor does move in the xref buffer if you use 'n' and 'p' in that
buffer.

> * I like the touch that the matches in the *xref* buffer are syntax highlighted. Unfortunately, not all matches are
> highlighted. It appears as though only matches in previously viewed parts of source files retain syntax
> highlighting.

I cannot reproduce this.

> * `next-error' in ChangeLog buffers cause Emacs to go to the corresponding change. This makes it hard to
> step past irrelevant xref matches if they occur a ChangeLog file.

You are supposed to get past them by moving in the xref buffer
instead.

> +++ Using "etags *.h *.m *.c" in the Emacs "src" directory, `(tags-search "nstrace")' find the first occurrence
> in 0.7 seconds, whereas the new `tags-find-regexp' takes over 8 seconds to perform a full search.

But after those 0.7 sec you are blind: you don't know how many hits
are there, and what will be the next hit to be shown to you.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/03/2016 09:32 PM, Anders Lindgren wrote:

> Ideally, xref should define a regexp search command that work with all
> backends. The only thing tags-specific in your code is getting the list
> of files -- if you would expand the xref backend interface with a
> corresponding function it would work with all backends.

I'm still waiting for the specification, including a reply to 
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23179#47.

> I would suggest that xref should provide two kinds of searches:
> one incremental (like `tags-search') and one `find-all' (like the
> provided function).

Patch welcome, I suppose, but the current API is not amenable to such 
usage, so see below (++).

> * The xref UI window is not updated to reflect the current location. For
> example, in a *grep* buffer, the cursor move and an arrow in the left
> fringe reflect the current location.

Not updated when? When you call `next-error'? I imagine that's something 
to implement in that facility, then, so that every other mode 
implementing next-error-function benefits.

> * I like the touch that the matches in the *xref* buffer are syntax
> highlighted. Unfortunately, not all matches are highlighted. It appears
> as though only matches in previously viewed parts of source files retain
> syntax highlighting.

Only those already visited by font-lock, yes.

> * `next-error' issued from an *xref* search don't reuse the source
> windows (whereas a `next-error' issued from a grep buffer does).
> * `next-error' in ChangeLog buffers cause Emacs to go to the
> corresponding change. This makes it hard to step past irrelevant xref
> matches if they occur a ChangeLog file.

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20493

Help welcome.

> +++ Using "etags *.h *.m *.c" in the Emacs "src" directory,
> `(tags-search "nstrace")' find the first occurrence in 0.7 seconds,
> whereas the new `tags-find-regexp' takes over 8 seconds to perform a
> full search.

The previous UI has pathological cases as well. Take e.g. some project 
where "xyz" only occurs in the last file among those listed in TAGS. 
Searching through all of those, by opening each one in Emacs in 
sequence, with re-search-forward, is surely slower that using Grep.

On the other hand, yes, the fact that the matches don't appear as soon 
as they're available, is a problem. Could you open a separate bug for that?

(++)

If your sole goal is to implement an incremental search UI, I fear the 
change to the xref API will become needlessly complex.

I think we should be able to extend the API to return some concurrency 
data structure, like a generator, using generator.el. I haven't 
investigated that yet (+++), and I'd welcome someone else taking the charge.

(+++) Is is feasible to turn Grep output into a generator? Is the result 
easy to render in an xref buffer, while indicating that the search is 
not yet done? Is the generator's overhead small enough in most cases?

>> xref-query-replace-in-results
>
> This is not quite true. `tags-query-replace' do an incremental search
> and replace in the source files referred to by a tags file directly.

The word "equivalent" doesn't necessarily imply the same UI.

> `xref-query-replace-in-results' requires a user to first do a search (to
> get stuff into the xref buffer), then run the command to replace. It
> sounds very awkward to me.

It looks very natural to me. And considering it handles different result 
sets, in the end you have N+1 command, rather than 2N commands, for N 
kinds things to search in.

> In other words, I would prefer if we would add an incremental xref
> query-replace command along the lines I suggested for the search command
> above.

It would need to know where to get the matches from. Or you'll end with 
N new query-replace commands, just like we have now (tags-query-replace, 
dired-do-query-replace-regexp, etc).

> Finally, if all the tags commands should be made obsolete, their
> documentations should be updated with references to the commands that
> are intended to replace them.

We do that with "(declare (obsolete ...".

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[John Wiegley]

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

>> `xref-query-replace-in-results' requires a user to first do a search (to
>> get stuff into the xref buffer), then run the command to replace. It sounds
>> very awkward to me.

> It looks very natural to me.

I realize it all sounds very natural to you, Dmitry, because you designed it.
However, accept that is not as natural to everyone else.  I too want a command
that lets me immediately jump to the first hit, rather than populating a very
large search results list only to visit it.  There is value in
tags-query-replace.

The fact that the xref.el API makes this now hard to do is a deficiency in
that API, not an indication that we shouldn't be doing it. Let's fix the API
-- which is still considered experimental. Your generator suggestion sounds
like a good approach.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/04/2016 01:44 AM, John Wiegley wrote:

> I realize it all sounds very natural to you, Dmitry, because you designed it.

This advantage clearly should be the most convincing, but you've skipped 
the other one: non-proliferation of new query-replace commands.

> However, accept that is not as natural to everyone else.  I too want a command
> that lets me immediately jump to the first hit, rather than populating a very
> large search results list only to visit it.  There is value in
> tags-query-replace.

First hit of what? We already have three different commands which search 
for different things, and return replace-able results.

If we want the xref UI to be reusable, that would bring new search 
commands in the future. Do you want each of them to come with a 
corresponding query-replace command?

> The fact that the xref.el API makes this now hard to do is a deficiency in
> that API, not an indication that we shouldn't be doing it. Let's fix the API
> -- which is still considered experimental. Your generator suggestion sounds
> like a good approach.

Sure, I'm all for that.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

Hi!

I would suggest that xref should provide two kinds of searches:
>> one incremental (like `tags-search') and one `find-all' (like the
>> provided function).
>>
>
> Patch welcome, I suppose, but the current API is not amenable to such
> usage, so see below (++).


Unfortunately, I have very little time to do heavy lifting on Emacs anymore
(which is why recently stepped down maintaining the NS port). However, I
try to help out by providing user experience feedback, whenever I find
something that doesn't feel right.


* The xref UI window is not updated to reflect the current location. For
>> example, in a *grep* buffer, the cursor move and an arrow in the left
>> fringe reflect the current location.
>>
>
> Not updated when? When you call `next-error'? I imagine that's something
> to implement in that facility, then, so that every other mode implementing
> next-error-function benefits.


Yes. In a *grep* buffer, the point and arrow moves to reflect the current
entry.


* I like the touch that the matches in the *xref* buffer are syntax
>> highlighted. Unfortunately, not all matches are highlighted. It appears
>> as though only matches in previously viewed parts of source files retain
>> syntax highlighting.
>>
>
> Only those already visited by font-lock, yes.
>

It could be easily fixed by a call to `font-lock-ensure' (at least for
files already loaded into Emacs).


* `next-error' issued from an *xref* search don't reuse the source
>> windows (whereas a `next-error' issued from a grep buffer does).
>> * `next-error' in ChangeLog buffers cause Emacs to go to the
>> corresponding change. This makes it hard to step past irrelevant xref
>> matches if they occur a ChangeLog file.
>>
>
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20493
>
> Help welcome.


OK, I see that those problems are known. I hope that they get fixed, as
they are annoying.


+++ Using "etags *.h *.m *.c" in the Emacs "src" directory,
>> `(tags-search "nstrace")' find the first occurrence in 0.7 seconds,
>> whereas the new `tags-find-regexp' takes over 8 seconds to perform a
>> full search.
>>
>
> The previous UI has pathological cases as well. Take e.g. some project
> where "xyz" only occurs in the last file among those listed in TAGS.
> Searching through all of those, by opening each one in Emacs in sequence,
> with re-search-forward, is surely slower that using Grep.
>
> On the other hand, yes, the fact that the matches don't appear as soon as
> they're available, is a problem. Could you open a separate bug for that?
>

I'd rather prefer if you would file a bug, I don't know which part I should
refer to, as I've only used your experimental `tags-find-regexp' code.



> In other words, I would prefer if we would add an incremental xref
>> query-replace command along the lines I suggested for the search command
>> above.
>>
>
> It would need to know where to get the matches from. Or you'll end with N
> new query-replace commands, just like we have now (tags-query-replace,
> dired-do-query-replace-regexp, etc).


If an xref backend would supply a list of files, you can easily implement a
generic `xref-query-replace' command.

However, if a backend isn't file based, maybe a better interface would be
to ask it for "next buffer" and it can fill it with whatever content it
want's to. In this case, it would be possible to implement a generic
xref-query-replace.

Anyway, your suggestion with generators sounds interesting.


Finally, if all the tags commands should be made obsolete, their
>> documentations should be updated with references to the commands that
>> are intended to replace them.
>>
>
> We do that with "(declare (obsolete ...".
>

Ah, I see that some commands like `find-tag' is marked as obsolete, however
`tags-search', and a few others, are not.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

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

On 04/04/2016 11:43 AM, Anders Lindgren wrote:

> Unfortunately, I have very little time to do heavy lifting on Emacs
> anymore (which is why recently stepped down maintaining the NS port).
> However, I try to help out by providing user experience feedback,
> whenever I find something that doesn't feel right.

As far as user feedback goes, I need more than "the new key bindings are 
different from the old ones". In-depth discussion of new generic 
commands and their semantics would be welcome.

If nobody steps up to implement your incremental search UI soon, though, 
we'll most likely release Emacs 25.1 with the current xref UI.

>     Not updated when? When you call `next-error'? I imagine that's
>     something to implement in that facility, then, so that every other
>     mode implementing next-error-function benefits.
>
>
> Yes. In a *grep* buffer, the point and arrow moves to reflect the
> current entry.

OK. That's already been brought up in one of the bugs I've referenced.

> It could be easily fixed by a call to `font-lock-ensure' (at least for
> files already loaded into Emacs).

Please test the attached patch. I'd like to know if there are cases when 
the highlighting overhead is noticeable.

However, this approach precludes an optimization I've been considering: 
when a given regexp doesn't use any Emacs-specific syntax, there's no 
need to visit the file. We would simply construct the match based on 
Grep's output. It would speed up the process a lot, in certain cases.

>     http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20489
>     http://debbugs.gnu.org/cgi/bugreport.cgi?bug=20493
>
>     Help welcome.
>
>
> OK, I see that those problems are known. I hope that they get fixed, as
> they are annoying.

Indeed. But so far there's no consensus on how to go about fixing them.

>     On the other hand, yes, the fact that the matches don't appear as
>     soon as they're available, is a problem. Could you open a separate
>     bug for that?
>
>
> I'd rather prefer if you would file a bug, I don't know which part I
> should refer to, as I've only used your experimental `tags-find-regexp'
> code.

You've never used e.g. xref-find-references?

Bug#23212 filed.

[-- Attachment #2: font-lock-ensure-in-xref--collect-matches.diff --]
[-- Type: text/x-patch, Size: 529 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index feed0fb..6cc8dc1 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -992,6 +992,7 @@ xref--collect-matches
               (line-beg (line-beginning-position))
               matches)
           (syntax-propertize line-end)
+          (font-lock-ensure line-beg line-end)
           ;; FIXME: This results in several lines with the same
           ;; summary. Solve with composite pattern?
           (while (re-search-forward regexp line-end t)

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

Hi!


> As far as user feedback goes, I need more than "the new key bindings are
> different from the old ones". In-depth discussion of new generic commands
> and their semantics would be welcome.
>

I really hope that you see my feedback as the latter and not the former.
Apart from that, I'll refrain from commenting.


If nobody steps up to implement your incremental search UI soon, though,
> we'll most likely release Emacs 25.1 with the current xref UI.


If xref doesn't provide something similar to tags-search and tags-query
replace, I would say that it's more likely that Emacs 25.1 will be released
with M-, bound to tags-loop-continue -- as this will make both the old tags
commands and the new xref system work. All we need to do is to find a new
binding for xref-pop-marker-stack.


Please test the attached patch. I'd like to know if there are cases when
> the highlighting overhead is noticeable.
>

I'll test it when I have a time slot available.


You've never used e.g. xref-find-references?
>

No. I went into this with the eyes of an existing tags user, and reported
the problems I saw.


> You might check which versions of 'find' and 'grep' you're using, though:
does M-x rgrep work all right? I remember there were some older versions of
these tools compiled for Windows, with pathological performance.

That's probably it. I have some kind of unix commands installed, apparently
they are not up to scratch. I'll try the tools Eli suggested.

However, most Windows users doesn't even have unix tools installed -- so
it's a really bad idea to assume that tools like `find' and `grep' are
available when running under Windows (at least until Emacs provide all the
tools needed).


> Project commands just need a version-controlled directory to be called
from. Are you not using version control for the project in question?

Yes, of course we use version control. Unfortunately, we use a collection
of repositories, so it's not possible to point to a root directory.


> You've also never answered the questions about the command's semantics,
above. If you want to see xref-find-regexp, I suggest you do that.

I think that I have done that. But I'll try again: I would like an
incremental, UI-free, free text search (like tags-search and
tags-query-replace). It's up to the backend to decide which files should be
included in the search. In the tags case, all files referred to the tags
file should be included. For other environments, public interfaces to used
libraries could be included.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/04/2016 07:58 PM, Anders Lindgren wrote:

> If xref doesn't provide something similar to tags-search and tags-query
> replace, I would say that it's more likely that Emacs 25.1 will be
> released with M-, bound to tags-loop-continue -- as this will make both
> the old tags commands and the new xref system work. All we need to do is
> to find a new binding for xref-pop-marker-stack.

I find that choice unlikely. But if we do end up butchering the new UI 
to use an amalgam of new and old bindings, I'll leave any further work 
on xref to people with more patience.

>     You've never used e.g. xref-find-references?
>
>
> No. I went into this with the eyes of an existing tags user, and
> reported the problems I saw.

etags has no counterpart for this command. Note, though, that the 
default implementation relies on the Project package.

> However, most Windows users doesn't even have unix tools installed -- so
> it's a really bad idea to assume that tools like `find' and `grep' are
> available when running under Windows (at least until Emacs provide all
> the tools needed).

We've already been assuming their presence for a while, in e.g. rgrep 
and find-grep-dired.

Also, maybe you haven't heard yet: the new version of Windows is 
promised to include a Linux subsystem, with GNU tools installed [0]. It 
should make using Grep, etc, much easier in the long run.

> I think that I have done that. But I'll try again: I would like an
> incremental, UI-free, free text search (like tags-search and
> tags-query-replace). It's up to the backend to decide which files should
> be included in the search. In the tags case, all files referred to the
> tags file should be included. For other environments, public interfaces
> to used libraries could be included.

That's better, thanks. But let's clarify this: should the set of files, 
which is decided by the backend, be exactly the same as the files that 
get searched by xref-find-references? I.e. program source code, in most 
cases.

[0] 
http://www.hanselman.com/blog/DevelopersCanRunBashShellAndUsermodeUbuntuLinuxBinariesOnWindows10.aspx

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 4 Apr 2016 20:25:51 +0300
> Cc: 23179@debbugs.gnu.org
> 
> >     You've never used e.g. xref-find-references?
> >
> > No. I went into this with the eyes of an existing tags user, and
> > reported the problems I saw.
> 
> etags has no counterpart for this command. Note, though, that the 
> default implementation relies on the Project package.

It does?  But it works for me (searching C symbols in Emacs sources)
with no extra preparations.  What am I missing?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/04/2016 08:54 PM, Eli Zaretskii wrote:

> It does?  But it works for me (searching C symbols in Emacs sources)
> with no extra preparations.  What am I missing?

The VC project backend is being used automatically (see 
project-find-functions), recognizing the Git checkout of the Emacs repo 
as the current project.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Mon, 4 Apr 2016 18:58:08 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: 23179@debbugs.gnu.org
> 
>  If nobody steps up to implement your incremental search UI soon, though, we'll most likely release
>  Emacs 25.1 with the current xref UI.
> 
> If xref doesn't provide something similar to tags-search and tags-query replace, I would say that it's more
> likely that Emacs 25.1 will be released with M-, bound to tags-loop-continue -- as this will make both the old
> tags commands and the new xref system work. All we need to do is to find a new binding for
> xref-pop-marker-stack.

We don't want to go back.  That would be a terrible waste of energy
already invested in development, improvements, and documentation of
these new features.  We cannot afford doing that.

Many issues with the xref UI and back-ends were already fixed.  You
raised the issue of the UI that you didn't want to see -- now there's
a proposed solution on the table for that.  Something along those
lines will soon be committed, if no one objects.

Beyond that, I see only one issue remaining to make xref a reasonably
good replacement for etags-based commands: the ability to collect
matches in chunks, so as not to delay the display of the first portion
of matches for too long.  I hope a solution for this will be found
soon enough.

With that problem out of our way, we could safely obsolete tags-search
and tags-query-replace, and leave the key bindings for them for those
who will still want to use them.  (I expect their number to gradually
go down with time.)

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

>
> It could be easily fixed by a call to `font-lock-ensure' (at least for
>> files already loaded into Emacs).
>>
>
> Please test the attached patch. I'd like to know if there are cases when
> the highlighting overhead is noticeable.
>

I gave it a test. The result looks good, and all matches are highlighted
(regardless if the files are loaded or not). Unfortunately, it comes with a
hefty overhead. The time increased from 7.5 s to 10.4 s. Also, the time is
not reduced if you run it a second time, even if the files are present in
Emacs buffers, indicating that font-lock is redone unnecessarily.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/05/2016 08:43 AM, Anders Lindgren wrote:

> I gave it a test. The result looks good, and all matches are highlighted
> (regardless if the files are loaded or not). Unfortunately, it comes
> with a hefty overhead. The time increased from 7.5 s to 10.4 s.

That's too bad. Although the difference might be smaller with other 
major modes than CC Mode, where syntax-propertize, currently called 
anyway, is not a no-op.

> Also,
> the time is not reduced if you run it a second time, even if the files
> are present in Emacs buffers, indicating that font-lock
> is redone unnecessarily.

That's because we don't want to open a zillion buffers and keep them 
that way, and nobody has implemented find-file-delayed yet:

http://lists.gnu.org/archive/html/emacs-devel/2015-08/msg00076.html

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 5 Apr 2016 15:54:35 +0300
> Cc: 23179@debbugs.gnu.org
> 
> On 04/05/2016 08:43 AM, Anders Lindgren wrote:
> 
> > I gave it a test. The result looks good, and all matches are highlighted
> > (regardless if the files are loaded or not). Unfortunately, it comes
> > with a hefty overhead. The time increased from 7.5 s to 10.4 s.
> 
> That's too bad. Although the difference might be smaller with other 
> major modes than CC Mode, where syntax-propertize, currently called 
> anyway, is not a no-op.

I personally don't consider a 30% slow-down as significant, but if we
think others might disagree, perhaps we should have this as an
optional behavior.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/05/2016 05:41 PM, Eli Zaretskii wrote:

> I personally don't consider a 30% slow-down as significant, but if we
> think others might disagree, perhaps we should have this as an
> optional behavior.

Is it 30% for you as well?

I don't mind too much either way, but note that fixing bug#23223 would 
make highlighting unavailable at least for the "fast" matches.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Cc: 23179@debbugs.gnu.org, andlind@gmail.com
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 5 Apr 2016 18:30:38 +0300
> 
> On 04/05/2016 05:41 PM, Eli Zaretskii wrote:
> 
> > I personally don't consider a 30% slow-down as significant, but if we
> > think others might disagree, perhaps we should have this as an
> > optional behavior.
> 
> Is it 30% for you as well?

I didn't yet have time to try it, sorry.  Will do later.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

Hi!


> Searching through all of those, by opening each one in Emacs in sequence,
> with re-search-forward, is surely slower that using Grep.
>

I just tried your `tags-find-regexp' on Windows, and it failed miserably.
It appears to be hanging, but after setting `debug-on-quit' to t and
pressing C-g, it always breaks when running an external "grep" command. My
conclusion is that starting "grep" on windows is very, very slow compared
to opening a temporary buffer and doing re-search-forward.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/04/2016 11:54 AM, Anders Lindgren wrote:

> I just tried your `tags-find-regexp' on Windows, and it failed
> miserably. It appears to be hanging, but after setting `debug-on-quit'
> to t and pressing C-g, it always breaks when running an external "grep"
> command. My conclusion is that starting "grep" on windows is very, very
> slow compared to opening a temporary buffer and doing re-search-forward.

AFAIK, Windows has higher process call overhead, and in this solution we 
call both 'find' and 'grep' once per file. It will be easy to write a 
version that doesn't use 'find' at all. If necessary, we can also pass 
multiple files at a time to 'grep'.

You might check which versions of 'find' and 'grep' you're using, 
though: does M-x rgrep work all right? I remember there were some older 
versions of these tools compiled for Windows, with pathological performance.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 4 Apr 2016 13:46:19 +0300
> Cc: 23179@debbugs.gnu.org
> 
> On 04/04/2016 11:54 AM, Anders Lindgren wrote:
> 
> > I just tried your `tags-find-regexp' on Windows, and it failed
> > miserably. It appears to be hanging, but after setting `debug-on-quit'
> > to t and pressing C-g, it always breaks when running an external "grep"
> > command. My conclusion is that starting "grep" on windows is very, very
> > slow compared to opening a temporary buffer and doing re-search-forward.
> 
> AFAIK, Windows has higher process call overhead

Not in my experience, at least not significantly so.

> You might check which versions of 'find' and 'grep' you're using, 
> though: does M-x rgrep work all right? I remember there were some older 
> versions of these tools compiled for Windows, with pathological performance.

Ah, yes, that could also be the reason.  This port of GNU Findutils is
recommended:

  https://sourceforge.net/projects/ezwinports/files/findutils-4.2.30-5-w32-bin.zip/download
  https://sourceforge.net/projects/ezwinports/files/findutils-4.2.30-5-w64-bin.zip/download

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Mon, 4 Apr 2016 10:54:14 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: 23179@debbugs.gnu.org
> 
> I just tried your `tags-find-regexp' on Windows, and it failed miserably. It appears to be hanging, but after setting `debug-on-quit' to t and pressing C-g, it always breaks when running an external "grep" command.

Works fine here, without hanging.  Takes circa 9 sec to collect
matches on the entire Emacs tree.

My guess is that you don't have a Find command installed (or it is not
called "find.exe").  Or maybe your Grep is somehow incompatible.

> My conclusion is that starting "grep" on windows is very, very slow compared to opening a temporary buffer and doing re-search-forward.

Well, the only meaningful conclusion that can be drawn from a failed
command is that it doesn't work, not that it's slow.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/01/2016 01:35 PM, Anders Lindgren wrote:

> Then, how do you search through a set of files using `xref'? Is that
> even possible?

project-find-regexp or dired-do-find-regexp.

> It all boils down to which commands should get access to the premium key
> bindings. I don't think `xref-pop-marker-stack' qualifies, if it means
> that continuing a tags search no longer has a key.

With `M-.' not doing a tags search anymore, the ability to continue a 
tags search has become less important than before.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[pklammer]

I find it incredibly dismissive of current usage, that you choose to break
existing functionality so lightly.

I'll tell you what "I use it all the time." is M-,

So why does your "all the time" overrule mine?



--
Sent from: http://emacs.1067599.n8.nabble.com/Emacs-Bugs-f3.html

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

On April 1, 2019 9:40:19 AM GMT+03:00, pklammer <emacs@pklammer.org> wrote:
> I find it incredibly dismissive of current usage, that you choose to
> break
> existing functionality so lightly.
> 
> I'll tell you what "I use it all the time." is M-,
> 
> So why does your "all the time" overrule mine?
> 
> 
> 
> --
> Sent from: http://emacs.1067599.n8.nabble.com/Emacs-Bugs-f3.html

The bug report to which you responded is quite old, and about an old Emacs version.  As result of that and other similar discussions, we added several features to Emacs.  So please state your Emacs version, and please describe the particular use case where you need to invoke tags-loop-continue.  Specifically, what command starts the tags based loop in your case?

Thanks.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[pklammer]

I use tags-search (bound to F9 in my .emacs) and then tags-loop-continue M-,
"all the time".
When I installed Emacs 26.1 on a new PC, I was  surprised that M-, was
broken, producing only error message "Marker stack is empty". Do you know
what you get when you Google "Marker stack is empty"? About two hours of
blind rabbit holes is what. Do you think I should be billing my client for
those two hours? No, neither do I; thank you very much.

So I mapped M-, to tags-loop-continue and I'm fine now.
I found the attitude (3 years ago today) about this breakage very dismissive
and inconsiderate.
I would have appreciated it if there were some better breadcrumbs to follow
to restore familiar operation.
I'll survive two hours lost billing OK.

Thank you for responding so quickly.




--
Sent from: http://emacs.1067599.n8.nabble.com/Emacs-Bugs-f3.html

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Tue, 2 Apr 2019 07:47:39 -0700 (MST)
> From: pklammer <emacs@pklammer.org>
> 
> I use tags-search (bound to F9 in my .emacs) and then tags-loop-continue M-,
> "all the time".

We should welcome patches to provide implementation of tags-search
based on Xref machinery.  Would you or someone else like to work on
that?

> I found the attitude (3 years ago today) about this breakage very dismissive
> and inconsiderate.

There was a long discussion about that (in addition to the one in this
bug report).

> I would have appreciated it if there were some better breadcrumbs to follow
> to restore familiar operation.

The change was called out in Emacs 25.1's NEWS (as all user-visible
changes are).

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 02.04.2019 18:20, Eli Zaretskii wrote:

> We should welcome patches to provide implementation of tags-search
> based on Xref machinery.  Would you or someone else like to work on
> that?

Y'all can try 'M-x project-find-regexp' or 'M-x project-search' in the 
meantime, depending on the personal preference for the UI.

>> I found the attitude (3 years ago today) about this breakage very dismissive
>> and inconsiderate.
> 
> There was a long discussion about that (in addition to the one in this
> bug report).
> 
>> I would have appreciated it if there were some better breadcrumbs to follow
>> to restore familiar operation.
> 
> The change was called out in Emacs 25.1's NEWS (as all user-visible
> changes are).

Indeed. So I don't think expecting a user who upgrades to a new major 
version (and even skipped over one) to have to adapt to one or two new 
key bindings is all that inconsiderate.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Juri Linkov]

>> We should welcome patches to provide implementation of tags-search
>> based on Xref machinery.  Would you or someone else like to work on
>> that?
>
> Y'all can try 'M-x project-find-regexp' or 'M-x project-search' in the
> meantime, depending on the personal preference for the UI.

I tried 'M-x project-find-regexp' and 'M-x project-search' and found
the following problems:

  M-x project-search

    1. can't use M-n to get the default value such as a word under point
       like project-find-regexp does

  M-x fileloop-continue

    2. its name prefix is inconsistent with the prefix of its
       counterpart command project-search

    3. no keybinding

  M-x project-find-regexp

    4. shouldn't it support an optional more compact grep-like output
       layout as well (where file names are on the same line with matches)?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 07.04.2019 00:12, Juri Linkov wrote:

>    M-x project-search
> 
>      1. can't use M-n to get the default value such as a word under point
>         like project-find-regexp does

fileloop-initialize-search could set up next-error-function, I suppose.

>    M-x fileloop-continue
> 
>      2. its name prefix is inconsistent with the prefix of its
>         counterpart command project-search

It doesn't have to. fileloop-continue is intended to be used with 
different commands (e.g. some with dired- prefix as well).

>      3. no keybinding

There's no keybinding for project-search either. The user is welcome to 
choose some.

>    M-x project-find-regexp
> 
>      4. shouldn't it support an optional more compact grep-like output
>         layout as well (where file names are on the same line with matches)?

You can implement that via a custom xref-show-xrefs-function. Not 
trivial, but not a huge amount of work either.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Juri Linkov]

>>    M-x project-search
>>
>>      1. can't use M-n to get the default value such as a word under point
>>         like project-find-regexp does
>
> fileloop-initialize-search could set up next-error-function, I suppose.

This is fine, but the point was about the minibuffer prompt.

>>    M-x fileloop-continue
>>
>>      2. its name prefix is inconsistent with the prefix of its
>>         counterpart command project-search
>
> It doesn't have to. fileloop-continue is intended to be used with different
> commands (e.g. some with dired- prefix as well).

Maybe an alias with another prefix is in order?

>>      3. no keybinding
>
> There's no keybinding for project-search either. The user is welcome to
> choose some.

Maybe 'M-g M-n'?

>>    M-x project-find-regexp
>>
>>      4. shouldn't it support an optional more compact grep-like output
>>         layout as well (where file names are on the same line with matches)?
>
> You can implement that via a custom xref-show-xrefs-function. Not trivial,
> but not a huge amount of work either.

Good to know that xref is so much customizable.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 07.04.2019 23:27, Juri Linkov wrote:
>>>     M-x project-search
>>>
>>>       1. can't use M-n to get the default value such as a word under point
>>>          like project-find-regexp does
>>
>> fileloop-initialize-search could set up next-error-function, I suppose.
> 
> This is fine, but the point was about the minibuffer prompt.

Ah yes. Sorry (M-n calls next-error in my Emacs).

But the answer would be the same: it's not hard to implement for whoever 
is interested. The easiest fix would be to simply use 
project--read-regexp in the interactive spec, but it shows a different 
prompt, which might or might not be a problem (the prompts follow their 
commands' names).

>>>     M-x fileloop-continue
>>>
>>>       2. its name prefix is inconsistent with the prefix of its
>>>          counterpart command project-search
>>
>> It doesn't have to. fileloop-continue is intended to be used with different
>> commands (e.g. some with dired- prefix as well).
> 
> Maybe an alias with another prefix is in order?

I don't think so. Or else the next step would be to set up multiple 
different key bindings, one for each alias. Which is obviously silly.

>>>       3. no keybinding
>>
>> There's no keybinding for project-search either. The user is welcome to
>> choose some.
> 
> Maybe 'M-g M-n'?

That's 'next-error', isn't it?

Again, the user is welcome to choose whatever key binding that suits them.

project-find-regexp, which is a lot handier IMO, has no default key 
binding either. I'm using 'C-x g'.

>>>     M-x project-find-regexp
>>>
>>>       4. shouldn't it support an optional more compact grep-like output
>>>          layout as well (where file names are on the same line with matches)?
>>
>> You can implement that via a custom xref-show-xrefs-function. Not trivial,
>> but not a huge amount of work either.
> 
> Good to know that xref is so much customizable.

That's the original idea: to be customizable. Not sure if 
xref-show-xrefs-function in its current form facilitates it best, of 
course. We might add some other hooks in the future as well.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Juri Linkov]

>>>>     M-x project-search
>>>>
>>>>       1. can't use M-n to get the default value such as a word under point
>>>>          like project-find-regexp does
>
> But the answer would be the same: it's not hard to implement for whoever is
> interested. The easiest fix would be to simply use project--read-regexp in
> the interactive spec, but it shows a different prompt, which might or might
> not be a problem (the prompts follow their commands' names).

If project-search searches for a regexp, then using a read-regexp
is the right thing.

>>>>     M-x fileloop-continue
>>>>
>>>>       2. its name prefix is inconsistent with the prefix of its
>>>>          counterpart command project-search
>>>
>>> It doesn't have to. fileloop-continue is intended to be used with different
>>> commands (e.g. some with dired- prefix as well).
>>
>> Maybe an alias with another prefix is in order?
>
> I don't think so. Or else the next step would be to set up multiple
> different key bindings, one for each alias. Which is obviously silly.

Then all these commands could share the same key prefix.

>>>>       3. no keybinding
>>>
>>> There's no keybinding for project-search either. The user is welcome to
>>> choose some.
>>
>> Maybe 'M-g M-n'?
>
> That's 'next-error', isn't it?
>
> Again, the user is welcome to choose whatever key binding that suits them.
>
> project-find-regexp, which is a lot handier IMO, has no default key binding
> either. I'm using 'C-x g'.

Maybe something like 'M-g f r' with a mnemonics "go find regexp".

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 08.04.2019 22:55, Juri Linkov wrote:

> If project-search searches for a regexp, then using a read-regexp
> is the right thing.

Sure.

>>>> It doesn't have to. fileloop-continue is intended to be used with different
>>>> commands (e.g. some with dired- prefix as well).
>>>
>>> Maybe an alias with another prefix is in order?
>>
>> I don't think so. Or else the next step would be to set up multiple
>> different key bindings, one for each alias. Which is obviously silly.
> 
> Then all these commands could share the same key prefix.

You might remember this thread: 
http://lists.gnu.org/archive/html/emacs-devel/2019-02/msg00424.html

It ended up with me not doing what you're asking for now.

>>>>>        3. no keybinding
>>>>
>>>> There's no keybinding for project-search either. The user is welcome to
>>>> choose some.
>>>
>>> Maybe 'M-g M-n'?
>>
>> That's 'next-error', isn't it?
>>
>> Again, the user is welcome to choose whatever key binding that suits them.
>>
>> project-find-regexp, which is a lot handier IMO, has no default key binding
>> either. I'm using 'C-x g'.
> 
> Maybe something like 'M-g f r' with a mnemonics "go find regexp".

Do we have other commands to put under 'M-g f'? Otherwise, seems like a 
waste of a keystroke.

Overall, I'm not sure; the M-g bindings, so far, all end up with 
navigation to a single location. Which might be suitable for the 
fileloop based commands, but less so for the xref ones.

Please go ahead and see if there's general support for this or that new 
key binding, but since we never had bindings for rgrep, as well as many 
other frequently-used commands, I don't necessarily feel they're essential.

They could help, though. Aand menu items as well, if we figure out how 
to distinguish project-find-regexp vs. project-search, and showcase both 
kinds of commands there.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Juri Linkov]

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

>>    M-x project-find-regexp
>>
>>      4. shouldn't it support an optional more compact grep-like output
>>         layout as well (where file names are on the same line with matches)?
>
> You can implement that via a custom xref-show-xrefs-function. Not trivial,
> but not a huge amount of work either.

Currently xref uses grep-like color palette, but its output layout is more like
multi-occur.  I tried to customize xref colors to match occur-like palette, but
found that colors are hard-coded, therefore the patch below.

Another problem:

  5. Files in the output of project-find-regexp are not sorted alphabetically,
     so currently the result is a mess in random order.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xref-faces.diff --]
[-- Type: text/x-diff, Size: 2530 bytes --]

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index aed92f8db6..e5e59721eb 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -448,6 +448,18 @@ xref--pop-to-location
 (defconst xref-buffer-name "*xref*"
   "The name of the buffer to show xrefs.")
 
+(defface xref-file-header '((t :inherit compilation-info))
+  "Face used to highlight file header in the xref buffer."
+  :version "27.1")
+
+(defface xref-line-number '((t :inherit compilation-line-number))
+  "Face for displaying line numbers in the xref buffer."
+  :version "27.1")
+
+(defface xref-match '((t :inherit highlight))
+  "Face used to highlight matches in the xref buffer."
+  :version "27.1")
+
 (defmacro xref--with-dedicated-window (&rest body)
   `(let* ((xref-w (get-buffer-window xref-buffer-name))
           (xref-w-dedicated (window-dedicated-p xref-w)))
@@ -737,18 +749,17 @@ xref--insert-xrefs
            for line-format = (and max-line-width
                                   (format "%%%dd: " max-line-width))
            do
-           (xref--insert-propertized '(face compilation-info) group "\n")
+           (xref--insert-propertized '(face xref-file-header) group "\n")
            (cl-loop for (xref . more2) on xrefs do
                     (with-slots (summary location) xref
                       (let* ((line (xref-location-line location))
                              (prefix
                               (if line
                                   (propertize (format line-format line)
-                                              'face 'compilation-line-number)
+                                              'face 'xref-line-number)
                                 "  ")))
                         (xref--insert-propertized
                          (list 'xref-item xref
-                               ;; 'face 'font-lock-keyword-face
                                'mouse-face 'highlight
                                'keymap xref--button-map
                                'help-echo
@@ -1159,7 +1170,7 @@ xref--collect-matches-1
              (end-column (- (match-end 0) line-beg))
              (loc (xref-make-file-location file line beg-column))
              (summary (buffer-substring line-beg line-end)))
-        (add-face-text-property beg-column end-column 'highlight
+        (add-face-text-property beg-column end-column 'xref-match
                                 t summary)
         (push (xref-make-match summary loc (- end-column beg-column))
               matches)))

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 11.04.2019 23:40, Juri Linkov wrote:

> Currently xref uses grep-like color palette, but its output layout is more like
> multi-occur.  I tried to customize xref colors to match occur-like palette, but
> found that colors are hard-coded, therefore the patch below.

LGTM, please go ahead.

> Another problem:
> 
>    5. Files in the output of project-find-regexp are not sorted alphabetically,
>       so currently the result is a mess in random order.

Aren't they in the same order as find-grep outputs them? IOW, rgrep must 
have the same problem. I find it very minor, personally.

We can add sorting here, but that would more or less preclude 
asynchronous generation/rendering of output we'd want to implement someday.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Juri Linkov]

>> Another problem:
>>
>>    5. Files in the output of project-find-regexp are not sorted alphabetically,
>>       so currently the result is a mess in random order.
>
> Aren't they in the same order as find-grep outputs them? IOW, rgrep must
> have the same problem. I find it very minor, personally.
>
> We can add sorting here, but that would more or less preclude asynchronous
> generation/rendering of output we'd want to implement someday.

I tried to run rgrep in `emacs -Q' and see that its output is unsorted
indeed.  I forgot that I customized the sorting of the output long ago:

(setq grep-find-template
"find <D> <X> -type f <F> -print0 | sort -z | xargs -0 -e grep <C> --color -inH -e <R>")
                                    =======

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 14.04.2019 0:57, Juri Linkov wrote:

> I tried to run rgrep in `emacs -Q' and see that its output is unsorted
> indeed.  I forgot that I customized the sorting of the output long ago:
> 
> (setq grep-find-template
> "find <D> <X> -type f <F> -print0 | sort -z | xargs -0 -e grep <C> --color -inH -e <R>")
>                                      =======

I see.

But actually, I looked at the code again, and there's little reason not 
to sort in the current implementation. Please try and see if doing that 
in project--files-in-directory gives any kind of perceptible slowdown.

Doing it in Lisp seems to be the fastest choice (benchmark still shows a 
bit of a difference if I do that via "| sort -z"):

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index dabc4ab6b4..b8a58ed317 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -209,7 +209,8 @@ project--files-in-directory
                                       (shell-quote-argument ")"))"")
                           )))
      (project--remote-file-names
-     (split-string (shell-command-to-string command) "\0" t))))
+     (sort (split-string (shell-command-to-string command) "\0" t)
+           #'string<))))

  (defun project--remote-file-names (local-files)
    "Return LOCAL-FILES as if they were on the system of 
`default-directory'."

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Juri Linkov]

>> I tried to run rgrep in `emacs -Q' and see that its output is unsorted
>> indeed.  I forgot that I customized the sorting of the output long ago:
>>
>> (setq grep-find-template
>> "find <D> <X> -type f <F> -print0 | sort -z | xargs -0 -e grep <C> --color -inH -e <R>")
>>                                      =======
>
> I see.
>
> But actually, I looked at the code again, and there's little reason not to
> sort in the current implementation. Please try and see if doing that in
> project--files-in-directory gives any kind of perceptible slowdown.

No perceptible slowdown, thanks.

> Doing it in Lisp seems to be the fastest choice (benchmark still shows
> a bit of a difference if I do that via "| sort -z"):

And also avoids any possible incompatibilities in options of the external command.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 14.04.2019 22:55, Juri Linkov wrote:

>> But actually, I looked at the code again, and there's little reason not to
>> sort in the current implementation. Please try and see if doing that in
>> project--files-in-directory gives any kind of perceptible slowdown.
> 
> No perceptible slowdown, thanks.

Pushed. Thanks for checking.

Each backend's implementation would need to sort on its own, though.

>> Doing it in Lisp seems to be the fastest choice (benchmark still shows
>> a bit of a difference if I do that via "| sort -z"):
> 
> And also avoids any possible incompatibilities in options of the external command.

Yup.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Juri Linkov]

>>    M-x project-find-regexp
>>
>>      4. shouldn't it support an optional more compact grep-like output
>>         layout as well (where file names are on the same line with matches)?
>
> You can implement that via a custom xref-show-xrefs-function. Not trivial,
> but not a huge amount of work either.

A new problem:

6. M-x project-find-regexp RET -- RET

   finds nothing whereas I can see there are many places with
   such string “--” in project files.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 24.04.2019 23:33, Juri Linkov wrote:

> A new problem:
> 
> 6. M-x project-find-regexp RET -- RET
> 
>     finds nothing whereas I can see there are many places with
>     such string “--” in project files.

Thank you for the report, should be fixed in f0e026a849.

Please file new bug reports for unrelated problems, though. Even if you 
don't like interacting with Debbugs too much (I don't), it's better to 
have proper structured discussions (and meaningful bug references in 
commit messages).

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Juri Linkov]

>> 6. M-x project-find-regexp RET -- RET
>>
>>     finds nothing whereas I can see there are many places with
>>     such string “--” in project files.
>
> Thank you for the report, should be fixed in f0e026a849.

Thanks, I verified this is fixed.

> Please file new bug reports for unrelated problems, though. Even if you
> don't like interacting with Debbugs too much (I don't), it's better to have
> proper structured discussions (and meaningful bug references in commit
> messages).

Actually I don't dislike Debbugs, I just hesitated to create a new report
for such a small problem.  Even if we used another bug tracker like GitLab,
I would try to add this as a comment to an existing issue instead of
creating a separate one.  So the problem is not in Debbugs, and GitLab
is not a panacea.  Moreover, when working on a project in GitLab, I strive
to do as much work as possible in Emacs, e.g. read MR diffs using vc-git,
but still miss an ability to follow GitLab discussions in Emacs.
Until someone writes Emacs frontend to GitLab discussion lists,
I prefer the current Gnus frontend to Debbugs discussion threads.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Fri, 1 Apr 2016 10:55:46 +0200
> From: Anders Lindgren <andlind@gmail.com>
> 
> I often use "etags" to search in files. The key binding `M-,' used to be bound to `tags-loop-continue', a generic
> command used to continue the last tags operation, like `tags-search' and `tags-query-replace'.
> 
> In Emacs 25.0.92, `M-,' is bound to `xref-pop-marker-stack', whereas `tags-loop-continue' is unbound.
> 
> Clearly, this breaks things for existing etags users and brings very little in return.

Please describe the use case where you needed to invoke
tags-loop-continue.  I think we made an effort to eliminate these use
cases (by providing alternatives that are built on top of xref), but
maybe some slipped through the cracks.

> (I expect `xref-pop-marker-stack' to be used relatively seldom.)

Like Dmitry, I use it all the time.  You really cannot avoid using it,
since, unlike find-tag, xref-find-definitions doesn't push a mark
storing the place where you invoked "M-.", so the only reasonable way
of getting back is with "M-,".

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

Hi!

Please describe the use case where you needed to invoke
> tags-loop-continue.  I think we made an effort to eliminate these use
> cases (by providing alternatives that are built on top of xref), but
> maybe some slipped through the cracks.
>

The basic use case is `M-x tags-search RET whatever RET'.

This places the cursor at the first occurrence of "whatever". In earlier
Emacs versions, I used `M-,' to take me to the next occurrence.

Maybe there is an "xref" command to continue the search, unfortunately, I
have no clue which. The doc string to `tags-search' only mentions
`tags-loop-continue'.


> (I expect `xref-pop-marker-stack' to be used relatively seldom.)
>
> Like Dmitry, I use it all the time.  You really cannot avoid using it,
> since, unlike find-tag, xref-find-definitions doesn't push a mark
> storing the place where you invoked "M-.", so the only reasonable way
> of getting back is with "M-,".
>

Ok, I can see the merits of the command, but I don't think it should take
up a prominent key binding like `M-,', especially since that has an
established use.

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Fri, 1 Apr 2016 12:13:39 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: 23179@debbugs.gnu.org
> 
>  Please describe the use case where you needed to invoke
>  tags-loop-continue. I think we made an effort to eliminate these use
>  cases (by providing alternatives that are built on top of xref), but
>  maybe some slipped through the cracks.
> 
> The basic use case is `M-x tags-search RET whatever RET'.

I guess we need an xref-based alternative to that.

> Ok, I can see the merits of the command, but I don't think it should take up a prominent key binding like `M-,',
> especially since that has an established use.

I'm afraid that ship has sailed.  We need to find a solution to this
issue without going back.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

Hi!

Eli:

> I'm afraid that ship has sailed.  We need to find a solution to this
> issue without going back.
>

I can agree with this.

The "xref" package is a big step forward, since it supports multiple
backends etc. Unfortunately, vital functionality is missing -- searching
and query-replace in all included files. Personally, I use `tags-search' at
least 20 times per day (often more), and `tags-query-replace' several times
each week. I don't think that my use pattern is extreme by any means.

I think that we would be making a big mistake if we would release Emacs 25
with an "xref" without searching and query-replace, but with key bindings
that, for most tags users, break existing use patterns.


Dmitry:
> If you want to have an xref-find-regexp as well, we should first
understand clearly how it would differ from those two commands. And its
specification cannot refer to tags files directly.

Today, many people use "tags" as a simple project file. They don't want to
redo this process with another tool ("project") and a dired approach might
not match a project layout at all.

Maybe I'm being naive, but a tags file (and presumably all other xref
backends) must represent a list of files. A free text search and
query-replace across those files would be very straight forward to specify,
wouldn't it?


Eli:
> But we could have a tags-only command that presented an xref UI, I think.
(Its name could be "tags-search" ;-)

It would have been neat... Unfortunately, the problem is not launching the
command, but rather continue searching past the first match -- since a
source buffer, and not the xref UI buffer, will be current.


I have given this some thought -- if we decide to really do make a change,
maybe we should try to make the xref search command more isearch-like, so
that a user could be able to continue an xref search using `C-s' rather
than `M-,'. Unfortunately, there is no natural key binding to continue a
normal query replace, but we could create something like
`xref-query-replace-from-here', to continue query-replacing from the point
in the current buffer and then continue with the next file in the file list.

   -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> Date: Sat, 2 Apr 2016 21:46:48 +0200
> From: Anders Lindgren <andlind@gmail.com>
> Cc: 23179@debbugs.gnu.org
> 
> The "xref" package is a big step forward, since it supports multiple backends etc. Unfortunately, vital
> functionality is missing -- searching and query-replace in all included files. Personally, I use `tags-search' at
> least 20 times per day (often more), and `tags-query-replace' several times each week. I don't think that my
> use pattern is extreme by any means.

Did you try any of the alternatives suggested in this discussion?  If
none of them satisfies your needs, can you elaborate why?

> I think that we would be making a big mistake if we would release Emacs 25 with an "xref" without searching
> and query-replace, but with key bindings that, for most tags users, break existing use patterns.

We are still discussing this issue, don't we? ;-)  And Emacs 25.1
release is still a couple of months away, so we still have time.

> > But we could have a tags-only command that presented an xref UI, I think. (Its name could be "tags-search"
> ;-)
> 
> 
> It would have been neat... Unfortunately, the problem is not launching the command, but rather continue
> searching past the first match -- since a source buffer, and not the xref UI buffer, will be current.

The xref UI solve this problem, as was already mentioned in this
discussion.

> I have given this some thought -- if we decide to really do make a change, maybe we should try to make the
> xref search command more isearch-like, so that a user could be able to continue an xref search using `C-s'
> rather than `M-,'. Unfortunately, there is no natural key binding to continue a normal query replace, but we
> could create something like `xref-query-replace-from-here', to continue query-replacing from the point in the
> current buffer and then continue with the next file in the file list.

xref-query-replace-in-results already provides a way for continuing
the replacement, so I'm not sure what you are had in mind here.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[John Wiegley]

>>>>> Anders Lindgren <andlind@gmail.com> writes:

> I think that we would be making a big mistake if we would release Emacs 25
> with an "xref" without searching and query-replace, but with key bindings
> that, for most tags users, break existing use patterns.

I very much agree with this, Andres. "xref" as a new feature should not cause
users to think that we've regressed in our capabilities. I've been concerned
about this for a few months now, and even think this issue should become a
release blocker for emacs-25. This needs to be addressed.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/03/2016 12:42 AM, John Wiegley wrote:
> This needs to be addressed.

What is?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[John Wiegley]

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

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04/03/2016 12:42 AM, John Wiegley wrote:
>> This needs to be addressed.

> What is?

The subject of the bug, and M-, having a potentially surprising new
non-behavior. If it's just a matter of right configuration, then we should
have that configuration be the default.

Tags is a really old service that many people have grown to depend on, so we
shouldn't be taking anything away from people by default who switch to
Emacs 25. Otherwise, there may be many irate bugs coming.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 629 bytes --]

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/03/2016 08:31 PM, John Wiegley wrote:

> The subject of the bug, and M-, having a potentially surprising new
> non-behavior.

Sorry, what? It has new behavior, yes, but it's not a "non-behavior".

> If it's just a matter of right configuration, then we should
> have that configuration be the default.

tags-loop-continue, which it was bound to before, doesn't do anything in 
xref.

> Tags is a really old service that many people have grown to depend on, so we
> shouldn't be taking anything away from people by default who switch to
> Emacs 25. Otherwise, there may be many irate bugs coming.

Tags are still available, both as an xref backend, and as a set of old 
commands a user can switch the bindings to.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[John Wiegley]

>>>>> Dmitry Gutov <dgutov@yandex.ru> writes:

>> If it's just a matter of right configuration, then we should
>> have that configuration be the default.

> tags-loop-continue, which it was bound to before, doesn't do anything in
> xref.

What does M-, do now, and what is the equivalent of tags-query-replace? Are
they easily discoverable by someone who knows nothing about xref.el or
project.el?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/03/2016 09:04 PM, John Wiegley wrote:

> What does M-, do now,

Jump back.

> and what is the equivalent of tags-query-replace?

xref-query-replace-in-results

> Are
> they easily discoverable by someone who knows nothing about xref.el or
> project.el?

We have them mentioned in the manual. A user familiar with our help 
facilities can also type `C-h f xref-replace TAB'.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> From: John Wiegley <jwiegley@gmail.com>
> Date: Sun, 03 Apr 2016 11:04:14 -0700
> Cc: 23179@debbugs.gnu.org, Anders Lindgren <andlind@gmail.com>
> 
> >>>>> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
> >> If it's just a matter of right configuration, then we should
> >> have that configuration be the default.
> 
> > tags-loop-continue, which it was bound to before, doesn't do anything in
> > xref.
> 
> What does M-, do now, and what is the equivalent of tags-query-replace? Are
> they easily discoverable by someone who knows nothing about xref.el or
> project.el?

xref is described in the manual.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Eli Zaretskii]

> From: John Wiegley <jwiegley@gmail.com>
> Date: Sun, 03 Apr 2016 10:31:54 -0700
> Cc: 23179@debbugs.gnu.org, Anders Lindgren <andlind@gmail.com>
> 
> The subject of the bug, and M-, having a potentially surprising new
> non-behavior. If it's just a matter of right configuration, then we should
> have that configuration be the default.
> 
> Tags is a really old service that many people have grown to depend on, so we
> shouldn't be taking anything away from people by default who switch to
> Emacs 25. Otherwise, there may be many irate bugs coming.

We've made a consistent and conscious effort to switch from the
tags-loop-continue UI to the xref UI.  The only command that still
needs that and doesn't have an xref-based equivalent is tags-search.
If we add such an equivalent command, the issue with
tags-loop-continue will only arise if the user uses the obsolete
commands (in which case they will probably rebind the keys anyway).

When all of these commands have xref UI, the M-, binding to
tags-loop-continue is no longer necessary because the command is
unnecessary.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/02/2016 10:46 PM, Anders Lindgren wrote:

> I think that we would be making a big mistake if we would release Emacs
> 25 with an "xref" without searching and query-replace, but with key
> bindings that, for most tags users, break existing use patterns.

As already mentioned, we have multiple solutions for searching, and one 
unified command for query-replace, invoked from the xref buffer.

> Today, many people use "tags" as a simple project file. They don't want
> to redo this process with another tool ("project") and a dired approach
> might not match a project layout at all.

project-or-external-find-regexp will take the tags files into account, 
as long as the current major mode hasn't overridden 
project-vc-external-roots-function, or the current project 
implementation hasn't overridden project-vc-external-roots-function.

"redo this process"? Which one?

> Maybe I'm being naive, but a tags file (and presumably all other xref
> backends) must represent a list of files. A free text search and
> query-replace across those files would be very straight forward to
> specify, wouldn't it?

An xref backend aims to represent the current coding environment; it 
could combine the source files in the current project, the library files 
external to the project (which could be compiled, zipped, etc), the 
information available in the currently running REPL.

Yes, there probably is a list of files in there a backend could search, 
but it should be specified better than that. Search only inside source 
code, but not documentation, resources, etc? Including any external 
files that do not belong to this project (try imagining a different xref 
backend for C code; it would probably include the installed libraries)?

And again, what do you see as the main advantage of the new command over 
project-or-external-find-regexp?

> I have given this some thought -- if we decide to really do make a
> change, maybe we should try to make the xref search command more
> isearch-like, so that a user could be able to continue an xref search
> using `C-s' rather than `M-,'.

That doesn't sound clear to me at all. You mean pressing C-s in an xref 
buffer? The place where you can currently use `n', `p', or `,' and `.'?

If you mean in a source buffer, what about next-error and previous-error?

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Anders Lindgren]

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

Hi!

Dmitry:

I think that we would be making a big mistake if we would release Emacs
>> 25 with an "xref" without searching and query-replace, but with key
>> bindings that, for most tags users, break existing use patterns.
>>
>
> As already mentioned, we have multiple solutions for searching, and one
> unified command for query-replace, invoked from the xref buffer.


I have tried all the the functions you have suggested, but I didn't get any
of them to perform like I wanted to. Of course, I didn't have time to dig
into each one of them, so that, for example,
`project-or-external-find-regexp' asked for a new project root directory
and then failed to turn up any matches I put it aside. Likewise, the
"dired" command is not what I was looking for, as it would require me to
mark a number of directories manually before starting.



> Today, many people use "tags" as a simple project file. They don't want
>> to redo this process with another tool ("project") and a dired approach
>> might not match a project layout at all.
>>
>
> project-or-external-find-regexp will take the tags files into account, as
> long as the current major mode hasn't overridden
> project-vc-external-roots-function, or the current project implementation
> hasn't overridden project-vc-external-roots-function.
>
> "redo this process"? Which one?


The process of deciding which files and directories should be included in a
"project". If you use TAGS, you typically do that from an external script
cherry-picking directories and files. You don't want to do that a second
time using some other kind of project manager.



> Maybe I'm being naive, but a tags file (and presumably all other xref
>> backends) must represent a list of files. A free text search and
>> query-replace across those files would be very straight forward to
>> specify, wouldn't it?
>>
>
> An xref backend aims to represent the current coding environment; it could
> combine the source files in the current project, the library files external
> to the project (which could be compiled, zipped, etc), the information
> available in the currently running REPL.
>
> Yes, there probably is a list of files in there a backend could search,
> but it should be specified better than that. Search only inside source
> code, but not documentation, resources, etc? Including any external files
> that do not belong to this project (try imagining a different xref backend
> for C code; it would probably include the installed libraries)?
>
> And again, what do you see as the main advantage of the new command over
> project-or-external-find-regexp?


The advantage over `tags-search' (which I use today) is that I would be
able to use another, more powerful, underlying database. E.g. TAGS does not
manage references to objects whereas systems like "kythe" does.

Apart from that, I rather like the way etags handles search and query
replace -- incrementally and without opening a UI window.


I have given this some thought -- if we decide to really do make a
>> change, maybe we should try to make the xref search command more
>> isearch-like, so that a user could be able to continue an xref search
>> using `C-s' rather than `M-,'.
>>
>
> That doesn't sound clear to me at all. You mean pressing C-s in an xref
> buffer? The place where you can currently use `n', `p', or `,' and `.'?
>
> If you mean in a source buffer, what about next-error and previous-error?
>

I mean the source buffer. Yes, `next-error' is a good candidate. (It's key
binding is somewhat clumsy though, if you need to skip past several
matches.)

    -- Anders

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

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Dmitry Gutov]

On 04/04/2016 11:21 AM, Anders Lindgren wrote:

> I have tried all the the functions you have suggested, but I didn't get
> any of them to perform like I wanted to. Of course, I didn't have time
> to dig into each one of them, so that, for example,
> `project-or-external-find-regexp' asked for a new project root directory
> and then failed to turn up any matches I put it aside.

Project commands just need a version-controlled directory to be called 
from. Are you not using version control for the project in question?

>     "redo this process"? Which one?
>
>
> The process of deciding which files and directories should be included
> in a "project". If you use TAGS, you typically do that from an external
> script cherry-picking directories and files. You don't want to do that a
> second time using some other kind of project manager.

Fair enough. But you'll also miss out on e.g. project-find-file.

We've been considering to approach this duplication of effort from the 
other direction: augmenting the project API with information necessary 
to generate a TAGS file.

>     Yes, there probably is a list of files in there a backend could
>     search, but it should be specified better than that. Search only
>     inside source code, but not documentation, resources, etc? Including
>     any external files that do not belong to this project (try imagining
>     a different xref backend for C code; it would probably include the
>     installed libraries)?
>
>     And again, what do you see as the main advantage of the new command
>     over project-or-external-find-regexp?
>
>
> The advantage over `tags-search' (which I use today) is that I would be
> able to use another, more powerful, underlying database.

That's not the question I asked. What's the advantage over 
project-or-external-find-regexp, at least when it works?

You've also never answered the questions about the command's semantics, 
above. If you want to see xref-find-regexp, I suggest you do that.

> E.g. TAGS does
> not manage references to objects whereas systems like "kythe" does.

That's one advantage to using a generic API like xref. I don't think 
it'll help much with xref-find-regexp, though: you're not just looking 
for references, it's a full-text regexp search.

> I mean the source buffer. Yes, `next-error' is a good candidate. (It's
> key binding is somewhat clumsy though, if you need to skip past several
> matches.)

I bind them to `M-n' and `M-p'. Luckily, they're not occupied by default.

bug#23179: 25.0.92; Restore `M-,' to continue etags search

[Lars Ingebrigtsen]

This was a very long thread, and several issues were touched upon.  If
I'm skimming this correctly, everything discussed was either fixed or it
was decided that should not be fixed, so I'm closing this bug report.

If there's anything more to be worked on here, please send a mail to the
debbugs address and we'll reopen the bug report, or perhaps file a new
report with any outstanding issues.

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