minad / consult

:mag: consult.el - Consulting completing-read
GNU General Public License v3.0
1.26k stars 106 forks source link

consult-xref not exporting to a xref major-mode buffer on embark-export #557

Closed slayer152 closed 2 years ago

slayer152 commented 2 years ago

Hi Daniel,

I'm not sure if this is a consult issue or an embark issue or something else; please let me know if it should be moved or I need to customize something.

Repro with emacs -Q:

(progn
  (package-initialize)
  (require 'consult)
  (require 'consult-xref)
  (require 'embark)
  (require 'vertico)
  (require 'embark-consult) ; not needed here?
  (global-set-key (kbd "C-c a") 'embark-act)
  (setq xref-show-xrefs-function          #'consult-xref
           xref-show-definitions-function #'consult-xref)
  (vertico-mode))

With the above config, if I:

  1. M-x dired and select a directory
  2. mark some files in the dired buffer
  3. M-x dired-do-find-regexp and provide a regexp, I get a minibuffer prompt Go to xref: with a list of the results.
  4. However, if I run embark-export on the list of results, I get a normal Embark Collect buffer with major-mode as embark-collect-mode (and not xref--xref-buffer-mode which would allow me to use M-g M-n and M-g M-p compilation-mode-like bindings to navigate the results).

However, If I don't use the above 2 setq xref-show... bindings and I run M-x dired-do-find-regexp and provide a regexp in step-3, I directly get a buffer with major-mode as xref--xref-buffer-mode which allows me to use M-g M-n and M-g M-p bindings to navigate the results. Would it be possible to use consult-xref and also embark-export to a xref--xref-buffer-mode buffer if needed.

Thanks.

oantolin commented 2 years ago

Oh, boy, I think I must have only tested with project-find-regexp! There indeed seem to be problems with dired-do-find-regexp.

oantolin commented 2 years ago

I figured out that some small part of the problem is not my fault 😛. Refresh doesn't seem to work correctly for the xref buffer produced by dired-do-find-regexp! Try this experiment:

Go back to the default settings:

 (setq xref-show-xrefs-function #'xref--show-xref-buffer
           xref-show-definitions-function #'xref-show-definitions-buffer)

Run dired, mark more than one file, and use dired-do-find-regexp. Then refresh the resulting xref buffer. What happens on my machine is that, after refreshing, only the results from the first file remain, the other files aren't taken into account it seems.

minad commented 2 years ago

Great! Time for report-emacs-bug! Fortunately xref is available via ELPA so people can profit soon from potential fixes.

oantolin commented 2 years ago

I reported it.

oantolin commented 2 years ago

And I've figured out the part that was my fault!

@slayer152 said that reverting the xref buffer would open the first marked file and erase it's contents. It took me a few tries to reproduce that and it only happened by a combination of circumstances:

  1. The xref-revert-buffer bug mentioned above means that when reverting we only see matches from the first file.
  2. The above makes it much more likely that you have a single match, and when you have a single match consult-xref doesn't even open the minibuffer (which I was counting on), but instead jumps right to it.
  3. Then xref-revert-buffer takes over, not suspecting that consult-xref changed the current buffer from the xref buffer to the first marked file, deletes the entire buffer's contents!

I've implemented a fix: in case rerunning the fetcher produces 0 or 1 items, I don't call consult-xref but return the items directly. This way we won't delete any unsuspecting buffer's contents, but the revert won't feel fully correct since it will only use the regexp and not the minibuffer input you gave to consult-xref prior to exporting. But that's definitely better than deleting a buffer's contents!

Fixed in oantolin/embark@70f720f914b78bd11f84b0790df5f125945dd50f

slayer152 commented 2 years ago

...What happens on my machine is that after refreshing only the results from the first file remain, the other files aren't taken into account it seems...I reported it.

Indeed, I see the same behavior with default settings. Thanks for reporting it and it looks like Lars Ingebrigtsen has already addressed it.

I've implemented a fix....But that's definitely better than deleting a buffer's contents!

Definitely. Thank you very much for the quick fix.