Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto select-window on find references/implementations #33

Merged
merged 4 commits into from
Feb 20, 2020

Conversation

ericdallo
Copy link
Member

Currently when you use lsp-treemacs-errors-list, automatically jumps to the new buffer showing the errors, but lsp-treemacs-references don't do that.

This PR fixes lsp-treemacs-references and lsp-treemacs-implementations to jump like lsp-treemacs-errors-list

@ericdallo ericdallo changed the title select-window on references/implementations auto select-window on find references/implementations Feb 18, 2020
@ericdallo ericdallo changed the title auto select-window on find references/implementations Auto select-window on find references/implementations Feb 18, 2020
Copy link
Contributor

@danielmartin danielmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good, but I want to check with @yyoncho before merging because we were discussing about this behaviour before: If we want to keep the current behaviour when the command is invoked with a prefix argument, or if we can implement that in a later PR.

IIRC, the key UX aspect to consider is that the operation that loads references in lsp-mode is asynchronous and you'll jump to a "Loading..." window for a moment. I've never found any server slow enough for this to be noticeable.

@ericdallo
Copy link
Member Author

Thanks @danielmartin, I understand and I really hope we find a solution because I want to replace my lsp-find-references with lsp-treemacs-refereneces because treemacs has a better UX.

@yyoncho
Copy link
Member

yyoncho commented Feb 19, 2020

@ericdallo willing to implement the following behavior?

M-x lsp-treemacs-references - switches to the window.
C-u M-x lsp-treemacs-references - show the window but it does not switch to it.

@danielmartin

I've never found any server slow enough for this to be noticeable.

JDT LS could take 20+ seconds. I think that ccls/clangd have everything precalculated and they just return the result.

@ericdallo
Copy link
Member Author

ericdallo commented Feb 19, 2020

@yyoncho, Can I use the prefix-argument that currently is used for expand the references?
https://github.com/emacs-lsp/lsp-treemacs/pull/33/files#diff-f7b652b3dbdcf99d9b44702b250a77e7R1199-R1201

If not, can you give some suggestions on how I can achieve that? Checking if the expand? prefix-argument is (4)? PrefixArgument

@yyoncho
Copy link
Member

yyoncho commented Feb 19, 2020

@yyoncho, Can I use the prefix-argument that currently is used for expand the references?

Ah, I totally forgot about that, sorry:

Can we then make it like that:

C-0 M-x lsp-treemacs-references - async call(this is equivalent to prefix = 0).
C-u M-x lsp-treemacs-references - select buffer and expand
M-x lsp-treemacs-references - select buffer and no expand

@ericdallo
Copy link
Member Author

Done @yyoncho

  • M-x lsp-treemacs-references - nil - select, no expands.
  • C-u M-x lsp-treemacs-references - '(4) - select and expands.
  • C-0 M-x lsp-treemacs-references - 0 - no select, no expands.

Copy link
Member

@yyoncho yyoncho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one comment:

lsp-treemacs.el Outdated Show resolved Hide resolved
lsp-treemacs.el Outdated Show resolved Hide resolved
@ericdallo
Copy link
Member Author

Done @yyoncho, thanks!

@yyoncho yyoncho merged commit 3436153 into emacs-lsp:master Feb 20, 2020
yyoncho added a commit that referenced this pull request Feb 20, 2020
@yyoncho
Copy link
Member

yyoncho commented Feb 20, 2020

Thank you for contributing to lsp-treemacs! I merged the PR. In addition, I did some small adjustments (one of them was to revert my advice to use zerop because (zerop nil) throws).

@ericdallo ericdallo deleted the improve-references branch February 20, 2020 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants