bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[sbaugh]

An existing *xref* buffer doesn't have its default-directory changed
when running project-find-regexp.  Since project-find-regexp switches to
*xref*, that means running project-find-regexp twice in a row may search
two different projects, which is unexpected.

Steps to reproduce:

With buffers in two different projects as detected by project.el, do the
following sequence:

1. Switch to a buffer in project A

2. project-find-regexp, which will search project A, and create and
switch to an *xref* buffer with a default-directory pointing at the
project root of A

3. project-find-regexp again, which will search project A again. (This
is the desired behavior)

4. Switch to a buffer in project B

5. project-find-regexp, which will search project B and switch to the
existing *xref* buffer (which is still pointing at project A)

6. project-find-regexp again, which will search project A instead of B.

Suggested fix:

Change project-find-regexp to reset the default-directory of the *xref*
buffer used to the most recently used project root.




In GNU Emacs 28.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.16.0)
Repository revision: 525dc6e5c428185b62c72d7958cd4fe17937f126
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: NixOS 21.05 (Okapi)

Configured using:
 'configure
 --prefix=/nix/store/023rdncicx7hz02dq986rnnpl12l1kas-emacs-git-20220115.0
 --disable-build-details --with-modules --with-x-toolkit=gtk3 --with-xft
 --with-cairo'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE
XIM XPM GTK3 ZLIB

Important settings:
  value of $EMACSLOADPATH: 
  value of $EMACSNATIVELOADPATH: /nix/store/fs7slsl0rz28h6dq8rnhgk4ddkk8dh0w-emacs-packages-deps/share/emacs/native-lisp::
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: notmuch-hello

Minor modes in effect:
  windmove-mode: t
  envrc-global-mode: t
  envrc-mode: t
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  savehist-mode: t
  save-place-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tab-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/sbaugh/.nix-profile/share/emacs/site-lisp/site-start hides /nix/store/fs7slsl0rz28h6dq8rnhgk4ddkk8dh0w-emacs-packages-deps/share/emacs/site-lisp/site-start
/home/sbaugh/.nix-profile/share/emacs/site-lisp/site-start hides /nix/store/023rdncicx7hz02dq986rnnpl12l1kas-emacs-git-20220115.0/share/emacs/site-lisp/site-start
/nix/store/fs7slsl0rz28h6dq8rnhgk4ddkk8dh0w-emacs-packages-deps/share/emacs/site-lisp/elpa/transient-20220112.1305/transient hides /nix/store/023rdncicx7hz02dq986rnnpl12l1kas-emacs-git-20220115.0/share/emacs/28.0.91/lisp/transient
/nix/store/fs7slsl0rz28h6dq8rnhgk4ddkk8dh0w-emacs-packages-deps/share/emacs/site-lisp/elpa/let-alist-1.0.6/let-alist hides /nix/store/023rdncicx7hz02dq986rnnpl12l1kas-emacs-git-20220115.0/share/emacs/28.0.91/lisp/emacs-lisp/let-alist

Features:
(shadow emacsbug ibuf-ext sh-script executable pulse qp sort tabify man
git-rebase pcmpl-unix image-file image-converter korea-util novice
timezone network-stream url-http url-gw nsm url-auth ggtags etags
fileloop ewoc two-column skeleton dumb-jump popup s xref quail log-view
magit-extras ibuffer ibuffer-loaddefs em-unix em-term term disp-table
ehelp em-script em-prompt em-ls em-hist em-pred em-glob em-cmpl em-dirs
esh-var em-basic em-banner em-alias esh-mode eshell esh-cmd esh-ext
esh-opt esh-proc esh-io esh-arg esh-module esh-groups esh-util make-mode
find-dired grep mhtml-mode css-mode js cc-mode cc-fonts cc-guess
cc-menus cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs sgml-mode
facemenu org-attach org-id warnings mail-extr nix-mode ffap smie
nix-repl nix-shell nix-store nix-instantiate nix-shebang nix-format nix
project reposition rect misc vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn
vc-cvs vc-rcs vc bug-reference finder-inf deuglify gnus-async gnus-cite
gnus-cus gnus-demon gnus-diary nndiary gnus-draft gnus-agent nnvirtual
nntp gnus-cache nndraft nnmh gnus-dup gnus-fun gnus-html url-cache
gnus-kill gnus-logic gnus-mh mh-comp mh-scan mh-gnus mh-e mh-compat
mh-buffers mh-loaddefs gnus-registry registry eieio-base gnus-salt
gnus-score score-mode gnus-srvr gnus-topic gnus-uu yenc gnus-vm gnus-msg
sendmail cus-dep autoload lisp-mnt cus-theme cl-print shortdoc
hippie-exp windmove help-fns radix-tree compile shr-color color
mule-util cus-edit pp notmuch notmuch-tree notmuch-jump notmuch-hello
notmuch-show notmuch-print notmuch-crypto notmuch-mua notmuch-message
notmuch-draft notmuch-maildir-fcc notmuch-address notmuch-company
notmuch-parser notmuch-wash coolj notmuch-query goto-addr icalendar
diary-lib diary-loaddefs notmuch-tag notmuch-lib notmuch-compat pcase
hl-line ol-eww eww xdg url-queue mm-url ol-rmail ol-mhe ol-irc ol-info
ol-gnus nnselect gnus-search eieio-opt speedbar ezimage dframe gnus-art
mm-uu mml2015 mm-view mml-smime smime dig gnus-sum shr kinsoku svg dom
gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud nnimap
nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range
gnus-win gnus nnheader wid-edit ol-docview doc-view jka-compr image-mode
exif ol-bibtex ol-bbdb ol-w3m ol-doi org-link-doi vc-git vc-dispatcher
python tramp-sh tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat parse-time ls-lisp face-remap misearch multi-isearch
cus-start cus-load dired-aux envrc inheritenv page-ext dired-x
magit-submodule magit-obsolete magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log which-func imenu
magit-diff smerge-mode diff git-commit log-edit message rmc dired
dired-loaddefs rfc822 mml mml-sec epa derived epg rfc6068 epg-config
gnus-util rmail rmail-loaddefs text-property-search mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums
mail-prsvr mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log
magit-core magit-autorevert autorevert filenotify magit-margin
magit-transient magit-process with-editor shell server magit-mode
transient cl-extra magit-git magit-section magit-utils crm dash
lui-autopaste circe diff-mode lui-irc-colors irc gnutls puny lcs
lui-logging lui-format lui tracking shorten thingatpt help-mode flyspell
ispell circe-compat ox-odt rng-loc rng-uri rng-parse rng-match rng-dt
rng-util rng-pttrn nxml-parse nxml-ns nxml-enc xmltok nxml-util ox-latex
ox-icalendar org-agenda org-refile ox-html table ox-ascii ox-publish ox
org-element avl-tree generator org ob ob-tangle ob-ref ob-lob ob-table
ob-exp org-macro org-footnote org-src ob-comint org-pcomplete pcomplete
org-list org-faces org-entities noutline outline org-version
ob-emacs-lisp ob-core ob-eval org-table oc-basic bibtex iso8601
time-date ol rx org-keys oc org-compat advice org-macs org-loaddefs
format-spec find-func cal-menu calendar cal-loaddefs gdb-mi gud
easy-mmode comint ansi-color ring cyberpunk-theme better-defaults
savehist saveplace ido tex-site edmacro kmacro info package browse-url
url url-proxy url-privacy url-expand url-methods url-history url-cookie
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
iso-transl tooltip eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer 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 composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 1076574 118416)
 (symbols 48 46869 2)
 (strings 32 222312 21193)
 (string-bytes 1 8226000)
 (vectors 16 108694)
 (vector-slots 8 1962697 150648)
 (floats 8 695 502)
 (intervals 56 63651 321)
 (buffers 992 195))

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Sean Whitton]

Hello,

On Sat 29 Jan 2022 at 07:11pm GMT, sbaugh@catern.com wrote:

> An existing *xref* buffer doesn't have its default-directory changed
> when running project-find-regexp.  Since project-find-regexp switches to
> *xref*, that means running project-find-regexp twice in a row may search
> two different projects, which is unexpected.

I think the problem is the let bindings for default-directory
established by project-find-regexp and also project-switch-project,
which latter I was using for testing.  These bindings hide the
buffer-local value for default-directory in *xref*, such that
xref--show-xref-buffer is only able to set the binding's value, not the
real buffer-local value, and so when the let forms unwind *xref*'s old
default-directory is restored.

Neither project-find-regexp nor project-switch-project should
special-case *xref*, because it looks like xref-show-xrefs-function
could be such as not to use that buffer.

One possible fix is the following patch, plus something similar in
project-find-regexp, but it feels whack-a-mole -- there are other
similar bindings of default-directory in project.el.  And it is not
really correct because perhaps the command would like to set
default-directory in whatever buffer the command was called in, without
knowing its name as xref--show-xref-buffer does.

> @@ -1605,9 +1605,12 @@
>    (let ((command (if (symbolp project-switch-commands)
>                       project-switch-commands
>                     (project--switch-project-command))))
> -    (let ((default-directory dir)
> -          (project-current-inhibit-prompt t))
> -      (call-interactively command))))
> +    ;; Switch to a temporary buffer to avoid shadowing a buffer-local value
> +    ;; for `default-directory' that command might want to set.
> +    (with-temp-buffer
> +      (let ((default-directory dir)
> +           (project-current-inhibit-prompt t))
> +       (call-interactively command)))))
>
>  (provide 'project)
>  ;;; project.el ends here

-- 
Sean Whitton

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

Hi!

On 29.01.2022 21:11, sbaugh@catern.com wrote:
> An existing*xref*  buffer doesn't have its default-directory changed
> when running project-find-regexp.  Since project-find-regexp switches to
> *xref*, that means running project-find-regexp twice in a row may search
> two different projects, which is unexpected.
> 
> Steps to reproduce:
> 
> With buffers in two different projects as detected by project.el, do the
> following sequence:
> 
> 1. Switch to a buffer in project A
> 
> 2. project-find-regexp, which will search project A, and create and
> switch to an*xref*  buffer with a default-directory pointing at the
> project root of A
> 
> 3. project-find-regexp again, which will search project A again. (This
> is the desired behavior)
> 
> 4. Switch to a buffer in project B
> 
> 5. project-find-regexp, which will search project B and switch to the
> existing*xref*  buffer (which is still pointing at project A)
> 
> 6. project-find-regexp again, which will search project A instead of B.
> 
> Suggested fix:
> 
> Change project-find-regexp to reset the default-directory of the*xref*
> buffer used to the most recently used project root.

This sounds like something that was fixed in commit b99848c72cb2570c 
(meaning, about a year ago).

I have tried to quickly reproduce this in my build from master, and 
hadn't managed to.

How recent is Emacs that you're trying this in? Do you perhaps have an 
older version of project.el or xref installed through ELPA?

Or did you maybe use project-switch-project, doing a search from its 
menu, like Sean seems to be hinting?

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Spencer Baugh]

Dmitry Gutov <dgutov@yandex.ru> writes:
> This sounds like something that was fixed in commit b99848c72cb2570c 
> (meaning, about a year ago).
>
> I have tried to quickly reproduce this in my build from master, and 
> hadn't managed to.
>
> How recent is Emacs that you're trying this in? Do you perhaps have an 
> older version of project.el or xref installed through ELPA?
>
> Or did you maybe use project-switch-project, doing a search from its 
> menu, like Sean seems to be hinting?

You're exactly right, the bug is only on an old Emacs.  I can't
reproduce it on a recent version.  My apologies, I thought I had repro'd
it on recent Emacs before submitting this.

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 03.02.2022 15:28, Spencer Baugh wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>> This sounds like something that was fixed in commit b99848c72cb2570c
>> (meaning, about a year ago).
>>
>> I have tried to quickly reproduce this in my build from master, and
>> hadn't managed to.
>>
>> How recent is Emacs that you're trying this in? Do you perhaps have an
>> older version of project.el or xref installed through ELPA?
>>
>> Or did you maybe use project-switch-project, doing a search from its
>> menu, like Sean seems to be hinting?
> 
> You're exactly right, the bug is only on an old Emacs.  I can't
> reproduce it on a recent version.  My apologies, I thought I had repro'd
> it on recent Emacs before submitting this.

Thanks for checking.

If you're running an older Emacs (27 or older), you should be able to 
install the latest versions of project.el and xref from GNU ELPA to 
avoid this problem.

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

Hi Sean,

On 30.01.2022 08:28, Sean Whitton wrote:
> I think the problem is the let bindings for default-directory
> established by project-find-regexp and also project-switch-project,
> which latter I was using for testing.

Could you also try your scenario with the master version of Emacs?

Or with the latest project.el and xref, at least.

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Sean Whitton]

Hello Dmitry,

On Thu 03 Feb 2022 at 04:00pm +02, Dmitry Gutov wrote:

> Hi Sean,
>
> On 30.01.2022 08:28, Sean Whitton wrote:
>> I think the problem is the let bindings for default-directory
>> established by project-find-regexp and also project-switch-project,
>> which latter I was using for testing.
>
> Could you also try your scenario with the master version of Emacs?
>
> Or with the latest project.el and xref, at least.

I was using master as of one week ago to test and looking at the code as
of the day I was writing.  And it doesn't look like there were any
relevant changes to project.el or xref.el in that time.

-- 
Sean Whitton

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 03.02.2022 17:19, Sean Whitton wrote:
> Hello Dmitry,
> 
> On Thu 03 Feb 2022 at 04:00pm +02, Dmitry Gutov wrote:
> 
>> Hi Sean,
>>
>> On 30.01.2022 08:28, Sean Whitton wrote:
>>> I think the problem is the let bindings for default-directory
>>> established by project-find-regexp and also project-switch-project,
>>> which latter I was using for testing.
>> Could you also try your scenario with the master version of Emacs?
>>
>> Or with the latest project.el and xref, at least.
> I was using master as of one week ago to test and looking at the code as
> of the day I was writing.  And it doesn't look like there were any
> relevant changes to project.el or xref.el in that time.

OK.

Then could you write down your problem scenario step-by-step?

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Sean Whitton]

Hello,

On Thu 03 Feb 2022 at 05:41PM +02, Dmitry Gutov wrote:

> Then could you write down your problem scenario step-by-step?

1. C-x p g foo RET
2. C-x p p ~/src/emacs RET bar RET
3. Observe that default-directory != ~/src/emacs, but instead the root
   of whatever project you searched for 'foo' in.
4. Thus, C-x p g baz RET will search in the first project, not
   ~/src/emacs.

Can test more if needed.

-- 
Sean Whitton

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 04.02.2022 01:16, Sean Whitton wrote:
> 2. C-x p p ~/src/emacs RET bar RET

You probably mean RET g bar RET.

And it seems the key is that the search is performed while the *xref* 
buffer is current. With that condition, I can repro, thanks.

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 30.01.2022 08:28, Sean Whitton wrote:
> These bindings hide the
> buffer-local value for default-directory in*xref*, such that
> xref--show-xref-buffer is only able to set the binding's value, not the
> real buffer-local value, and so when the let forms unwind*xref*'s old
> default-directory is restored.

I have to say I'm surprised by this mechanic: not even setq-local helps.

Guess we could kill the xref buffer every time, instead of erasing and 
re-filling it, though. Like

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 4efa652084..bb08db726b 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1111,6 +1111,7 @@ xref--show-xref-buffer
           (xref-alist (xref--analyze xrefs))
           (dd default-directory)
           buf)
+    (ignore-errors (kill-buffer xref-buffer-name))
      (with-current-buffer (get-buffer-create xref-buffer-name)
        (setq default-directory dd)
        (xref--xref-buffer-mode)

It has the unfortunate side-effect of having that buffer displayed in a 
different window, though. Or at least the possibility (though it happens 
every time here).

Maybe some kill-but-recreate-and-show-in-the-same-window-first kind of 
hack would do the trick.

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Sean Whitton]

Hello,

On Fri 04 Feb 2022 at 04:32am +02, Dmitry Gutov wrote:

> On 30.01.2022 08:28, Sean Whitton wrote:
>> These bindings hide the
>> buffer-local value for default-directory in*xref*, such that
>> xref--show-xref-buffer is only able to set the binding's value, not the
>> real buffer-local value, and so when the let forms unwind*xref*'s old
>> default-directory is restored.
>
> I have to say I'm surprised by this mechanic: not even setq-local helps.

I guess that's dynamic binding for you :)

> Guess we could kill the xref buffer every time, instead of erasing and
> re-filling it, though.
> [...]
> It has the unfortunate side-effect of having that buffer displayed in a
> different window, though. Or at least the possibility (though it happens
> every time here).
>
> Maybe some kill-but-recreate-and-show-in-the-same-window-first kind of
> hack would do the trick.

Maybe we could rename the old buffer to a temp name, then create and
fill the new one, then set the old buffer's window's buffer to the new
one, and finally kill the old one.  But this is not nice at all.

How necessary is it that project.el set up this binding?  Is there
something else it could do to achieve the same effect?

At worst, it seems better to special-case *xref* over in project.el than
in xref.el, but you have a better overall perspective of the two
libraries than me.

-- 
Sean Whitton

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 04.02.2022 07:32, Sean Whitton wrote:
> Hello,
> 
> On Fri 04 Feb 2022 at 04:32am +02, Dmitry Gutov wrote:
> 
>> On 30.01.2022 08:28, Sean Whitton wrote:
>>> These bindings hide the
>>> buffer-local value for default-directory in*xref*, such that
>>> xref--show-xref-buffer is only able to set the binding's value, not the
>>> real buffer-local value, and so when the let forms unwind*xref*'s old
>>> default-directory is restored.
>>
>> I have to say I'm surprised by this mechanic: not even setq-local helps.
> 
> I guess that's dynamic binding for you :)
> 
>> Guess we could kill the xref buffer every time, instead of erasing and
>> re-filling it, though.
>> [...]
>> It has the unfortunate side-effect of having that buffer displayed in a
>> different window, though. Or at least the possibility (though it happens
>> every time here).
>>
>> Maybe some kill-but-recreate-and-show-in-the-same-window-first kind of
>> hack would do the trick.
> 
> Maybe we could rename the old buffer to a temp name, then create and
> fill the new one, then set the old buffer's window's buffer to the new
> one, and finally kill the old one.  But this is not nice at all.

Right.

> How necessary is it that project.el set up this binding?  Is there
> something else it could do to achieve the same effect?
> 
> At worst, it seems better to special-case *xref* over in project.el than
> in xref.el, but you have a better overall perspective of the two
> libraries than me.

I suppose which is the better fix depends on whether we consider it to 
be important for third-party or simply new/future code to be able add a 
let-binding for default-directory to affect xref's behavior without 
having to worry about using a temp buffer. Until now, I've considered it 
to be a reasonable approach that shouldn't cause problems requiring the 
code author to travel down the call chain for investigation.

But maybe that was a bad assumption, and the added complexity of the 
approach described above it too much. I like your first suggested fix 
well enough, FWIW.

I wonder if we hit this problem in Emacs in some packages in the past, 
and how we chose to solve it. After all, default-directory is always 
buffer-local, and modifying its value is a common thing to do.

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 07.02.2022 05:12, Dmitry Gutov wrote:
>> Maybe we could rename the old buffer to a temp name, then create and
>> fill the new one, then set the old buffer's window's buffer to the new
>> one, and finally kill the old one.  But this is not nice at all.
> 
> Right.

We can also try a lighter-weight hack like below. WDYT?

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 784c745477..082d64aaeb 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -1107,6 +1107,13 @@ xref--analyze
               (cdr pair)))
       alist)))

+(defun xref--ensure-default-directory (dd buffer)
+  ;; We might be in a let-binding which will restore the current value
+  ;; to a previous one (bug#53626).  So do this later.
+  (run-with-timer
+   0 nil
+   (lambda () (with-current-buffer buffer (setq default-directory dd)))))
+
  (defun xref--show-xref-buffer (fetcher alist)
    (cl-assert (functionp fetcher))
    (let* ((xrefs
@@ -1117,7 +1124,7 @@ xref--show-xref-buffer
           (dd default-directory)
           buf)
      (with-current-buffer (get-buffer-create xref-buffer-name)
-      (setq default-directory dd)
+      (xref--ensure-default-directory dd (current-buffer))
        (xref--xref-buffer-mode)
        (xref--show-common-initialize xref-alist fetcher alist)
        (pop-to-buffer (current-buffer))
@@ -1216,7 +1223,7 @@ xref-show-definitions-buffer-at-bottom
                              (assoc-default 'display-action alist)))
       (t
        (with-current-buffer (get-buffer-create xref-buffer-name)
-        (setq default-directory dd)
+        (xref--ensure-default-directory dd (current-buffer))
          (xref--transient-buffer-mode)
          (xref--show-common-initialize (xref--analyze xrefs) fetcher alist)
          (pop-to-buffer (current-buffer)

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Sean Whitton]

Hello,

On Mon 21 Feb 2022 at 03:55am +02, Dmitry Gutov wrote:

> On 07.02.2022 05:12, Dmitry Gutov wrote:
>>> Maybe we could rename the old buffer to a temp name, then create and
>>> fill the new one, then set the old buffer's window's buffer to the new
>>> one, and finally kill the old one.  But this is not nice at all.
>>
>> Right.
>
> We can also try a lighter-weight hack like below. WDYT?

I think I prefer my with-temp-buffer thing to this, to be honest --
adding a timer into the mix seems like it might make debugging harder.

Assuming you're still okay with my initial idea, I'll come up with a
patch to do it with a macro, and apply it across project.el.

-- 
Sean Whitton

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 22.02.2022 01:00, Sean Whitton wrote:
> I think I prefer my with-temp-buffer thing to this, to be honest --
> adding a timer into the mix seems like it might make debugging harder.
> 
> Assuming you're still okay with my initial idea, I'll come up with a
> patch to do it with a macro, and apply it across project.el.

I liked the simplicity of the with-temp-buffer solution, but it doesn't 
solve the conceptual problem: that any code trying to let-bind 
default-directory around the call for xref-show-xrefs will have 
rediscover and solve this problem (or, more likely, live with it for a 
few years until somebody notices).

The timer-based solution is hacky, but it affects only one place (the 
xref buffer and the variable in it), so it's easier to verify that it 
has the intended effect, and no external callers will need to bother 
with additional knowledge (hopefully, of course).

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Sean Whitton]

Hello,

On Tue 22 Feb 2022 at 01:14AM +02, Dmitry Gutov wrote:

> On 22.02.2022 01:00, Sean Whitton wrote:
>> I think I prefer my with-temp-buffer thing to this, to be honest --
>> adding a timer into the mix seems like it might make debugging harder.
>>
>> Assuming you're still okay with my initial idea, I'll come up with a
>> patch to do it with a macro, and apply it across project.el.
>
> I liked the simplicity of the with-temp-buffer solution, but it doesn't
> solve the conceptual problem: that any code trying to let-bind
> default-directory around the call for xref-show-xrefs will have
> rediscover and solve this problem (or, more likely, live with it for a
> few years until somebody notices).

Good point.

> The timer-based solution is hacky, but it affects only one place (the
> xref buffer and the variable in it), so it's easier to verify that it
> has the intended effect, and no external callers will need to bother
> with additional knowledge (hopefully, of course).

Is there no chance of any concurrency issues with this, btw?

-- 
Sean Whitton

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 23.02.2022 01:09, Sean Whitton wrote:
>> The timer-based solution is hacky, but it affects only one place (the
>> xref buffer and the variable in it), so it's easier to verify that it
>> has the intended effect, and no external callers will need to bother
>> with additional knowledge (hopefully, of course).
> Is there no chance of any concurrency issues with this, btw?

Probably not, or not currently: the fetching of the list of xrefs is 
synchronous, and in all cases (so far) (I think) does not depend on the 
value of default-directory. It only affects the subsequent interaction 
with the buffer (jumping to locations), so deferring the assignment of 
the value of dd seems fine.

FETCHER is also called in the current buffer (before the 
with-current-buffer form), so even depending on the dynamic value of 
default-directory should be fine. At least until we get to 
asynchronous/chunked fetchers someday, but that's a bigger challenge 
with its own major changes.

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 21.02.2022 03:55, Dmitry Gutov wrote:
> On 07.02.2022 05:12, Dmitry Gutov wrote:
>>> Maybe we could rename the old buffer to a temp name, then create and
>>> fill the new one, then set the old buffer's window's buffer to the new
>>> one, and finally kill the old one.  But this is not nice at all.
>>
>> Right.
> 
> We can also try a lighter-weight hack like below. WDYT?

Long story short, I've pushed that change to master (in 0f67a3df0e).

Let me know if you find any problems with it.

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Sean Whitton]

Hello,

On Wed 23 Feb 2022 at 03:55am +02, Dmitry Gutov wrote:

> On 21.02.2022 03:55, Dmitry Gutov wrote:
>> On 07.02.2022 05:12, Dmitry Gutov wrote:
>>>> Maybe we could rename the old buffer to a temp name, then create and
>>>> fill the new one, then set the old buffer's window's buffer to the new
>>>> one, and finally kill the old one.  But this is not nice at all.
>>>
>>> Right.
>>
>> We can also try a lighter-weight hack like below. WDYT?
>
> Long story short, I've pushed that change to master (in 0f67a3df0e).
>
> Let me know if you find any problems with it.

Had a look and ran some tests.  LGTM.  Thanks!

-- 
Sean Whitton

bug#53626: 28.0.91; project-find-regexp (C-x p g) twice results in searching different projects

[Dmitry Gutov]

On 23.02.2022 07:36, Sean Whitton wrote:
> Had a look and ran some tests.  LGTM.  Thanks!

Thanks for checking! Closing.