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

Add breadcrumb on headerline #1799

Merged
merged 8 commits into from
Jun 14, 2020
Merged

Add breadcrumb on headerline #1799

merged 8 commits into from
Jun 14, 2020

Conversation

ericdallo
Copy link
Member

  • Add breadcrumb on headerline via lsp-headerline-breadcrumb-mode:
    Screenshot from 2020-06-13 19-07-53
  • Support for deprecated symbols:
    image
  • Add missing documentation to modeline code actions and breadcrumb.

Full screenshot:
image

In a next PR, the idea is to add mouse support to breadcrumb.

@ericdallo ericdallo self-assigned this Jun 13, 2020
lsp-mode.el Outdated Show resolved Hide resolved
lsp-mode.el Outdated Show resolved Hide resolved
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.

lsp-mode.el Outdated Show resolved Hide resolved
lsp-mode.el Outdated Show resolved Hide resolved
lsp-mode.el Outdated
"Convert SYMBOL-INFORMATIONS to symbols hierarchy."
(seq-some (-lambda ((symbol &as &SymbolInformation :location (&Location :range)))
(-let (((beg . end) (lsp--range-to-region range)))
(and (<= beg (point) end)
Copy link
Member

Choose a reason for hiding this comment

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

when tested manually, when I move outside of an element with one char it still stays the same, so probably something here is off by one.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't get this problem with lsp-dart, could you provide more details? Maybe the dart or other server is sending different ranges?

Copy link
Member

Choose a reason for hiding this comment

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

I tested it, seems like the issue is lack of refresh mentioned bellow.

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, so I guess it's fixed now after adding (force-modeline-update)

lsp-mode.el Show resolved Hide resolved
lsp-mode.el Show resolved Hide resolved
@ericdallo ericdallo requested a review from yyoncho June 14, 2020 15:41
@yyoncho yyoncho merged commit b1621c5 into master Jun 14, 2020
@yyoncho
Copy link
Member

yyoncho commented Jun 14, 2020

Can you merge it in plists branch as well?

Few things that can be addressed in the future:

  1. Breadcrumb functionality - I like the way it works in vscode, clicking symbol shows the stuff on the same level.
  2. It contains also the path to the file in the project.
  3. Allowing clicking with avy, similar to what we do with lsp-avy-lens. I think this will be very powerfull way to navigate, with no equivalent in emacs world. That would allow jumping in anything containing the current file. As an alternative to avy we may have a shortcut open Nth symbol from the breadcrumb.
  4. Review the whole document symbol story. Those patches we did for imenu are really ugly.

@ericdallo ericdallo deleted the headerline-breadcrumb branch June 14, 2020 16:24
@ericdallo ericdallo added this to the 7.0 milestone Jun 14, 2020
@ericdallo
Copy link
Member Author

Can you merge it in plists branch as well?

Few things that can be addressed in the future:

  1. Breadcrumb functionality - I like the way it works in vscode, clicking symbol shows the stuff on the same level.
  2. It contains also the path to the file in the project.
  3. Allowing clicking with avy, similar to what we do with lsp-avy-lens. I think this will be very powerfull way to navigate, with no equivalent in emacs world. That would allow jumping in anything containing the current file. As an alternative to avy we may have a shortcut open Nth symbol from the breadcrumb.
  4. Review the whole document symbol story. Those patches we did for imenu are really ugly.
  1. we can start using completing-read when user clicks on it.
  2. I thought about that, but don't you think that can be too much stuff on headerline? what about just the file name?
  3. Agree, we can think in implement that.

@yyoncho
Copy link
Member

yyoncho commented Jun 14, 2020

2. I thought about that, but don't you think that can be too much stuff on headerline? what about just the file name?

I think that it looks fine in vscode. We could limit the length and use some kind of strategy to shorten the path(e. g. replace with the first letter of the directory). Anyways, it won't be a feature for everyone.

Just to make sure we are on the same page, the proposal is to you the file name relative to project root.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants