Make lsp-headerline update breadcrumb when previewing with consult-xref

[dhashe]

lsp-mode doesn't depend on 'consult being loaded, but it is safe in emacs to call add-hook before loading the package that provides the hook, and that is the simplest approach here. If 'consult is not installed or is never loaded, then the hook will simply never be used.

Fixes #4802

[kiennq]

Shouldn't this be an issue of consult-xref instead? I.e., consult-xref should subscribe/invoke to xref-after-jump-hook. That seems to be more approriate

[dhashe]

Shouldn't this be an issue of consult-xref instead?

Maybe, I can certainly file an issue there and see what they say.

Skimming over their issue list, I see a relevant comment that it is intentional not to invoke xref-after-jump-hook during preview, because the set of hooks that you would want to invoke for previewing a buffer could be a subset of the set of hooks that you would want when jumping for real: minad/consult#59 (comment)

But that is an old comment and perhaps it is worth discussing the topic again.

[dhashe]

I discussed this with the consult author here: minad/consult#1229

I don't think that it makes sense for consult-xref to invoke xref-after-jump-hook during previews, because the preview action even on consult-xref doesn't use xref. xref is only called when doing the final jump after selecting a candidate.

It also wouldn't solve this problem in general if it did, because there are other consult commands besides consult-xref that can end up previewing buffers that have lsp-headerline-breadcrumb-mode enabled. For example, consult-mark, which selects between and previews the various marker locations in the marker list.

consult-mark does not use xref in any way, and xref is not necessarily even loaded when consult-mark is called. But it uses the same preview logic, and if we call it on a buffer with lsp-headerline-breadcrumb-mode enabled then the breadcrumb similarly does not update as you scroll through the candidates.

I think that it's better to frame the issue like this:

• lsp-headerline-breadcrumb-mode provides the "breadcrumb" feature, and wants the breadcrumb to be correct in common cases
• xref provides the "jump" feature, and lsp-headerline-breadcrumb-mode hooks into it using the provided hook to update the breadcrumb on xref jumps
• consult (if present) provides a related but distinct "preview-jump" feature, and lsp-headerline-breadcrumb-mode should hook into it using the provided hook to update the breadcrumb on consult previews

And with that framing I think that this is the best place to make the change, although I concede that it's a little weird to add to a hook for a project (consult) that is not a dependency of lsp-mode. I welcome other ideas for where it would make sense to put this.

[dhashe]

Alternatively, this is solvable in user configuration (with the wrinkle of needing to call an internal function) with the following:

(add-hook 'lsp-headerline-breadcrumb-mode-hook
          (lambda ()
            (if lsp-headerline-breadcrumb-mode
                (add-hook 'consult-after-jump-hook #'lsp-headerline--check-breadcrumb nil t)
                (remove-hook 'consult-after-jump-hook #'lsp-headerline--check-breadcrumb t))))

Maybe that's better and we can close this and leave to downstream? Maybe it's worth exposing a public version of lsp-headerline--check-breadcrumb? What do you think?

[kiennq]

Maybe it's worth exposing a public version of lsp-headerline--check-breadcrumb? What do you think?

I agree, expose lsp-headerline--check-breadcrumb and update the wiki for this usage seems to be the most probable approach. Feel free to update your PR with that.