Skip to content

editor: use full path for title captions#10238

Merged
vince-fugnitto merged 1 commit intomasterfrom
vf/multi-root-path
Oct 8, 2021
Merged

editor: use full path for title captions#10238
vince-fugnitto merged 1 commit intomasterfrom
vf/multi-root-path

Conversation

@vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Oct 6, 2021

What it does

Fixes: #10237

The pull-request updates the editor title caption to use the full path rather than the relative path from the label-provider which aligns with the behavior present in vscode. The full-path is especially useful in a multi-root workspace where two files with the same name could not be easily differentiated previously.

How to test

  1. confirm that CI passes
  2. start the application
  3. open multiple editors - confirm that their full path is displayed when hovering over the tab-bar
  4. open a multi-root workspace - open multiple editors from different roots - confirm that their full path is displayed when hovering over the tab-bar

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto vince-fugnitto added workspace issues related to the workspace multi-root issues related to multi-root support labels Oct 6, 2021
@vince-fugnitto vince-fugnitto self-assigned this Oct 6, 2021
@vince-fugnitto
Copy link
Member Author

I'll need to take a look at the failures in the markers tests (which likely relies on the workspace label-provider).

Copy link
Contributor

@alvsan09 alvsan09 left a comment

Choose a reason for hiding this comment

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

I have tested as per the instructions provided and works as expected 👍

It would be great of you could also add steps to verify the changes to marker-tree-label-provider.ts .

@vince-fugnitto
Copy link
Member Author

It would be great of you could also add steps to verify the changes to marker-tree-label-provider.ts .

The marker-tree-label-provider was updated in order to make it's tests (which cover multiple use-cases for single and multi-root) pass again since it used to rely on workspace-uri-label-provider to compute the label in a multi-root setting. The changes now handle the getLongName properly rather than depend on another label-provider.

Copy link
Contributor

@alvsan09 alvsan09 left a comment

Choose a reason for hiding this comment

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

It looks good to me !
Thanks Vince!

The commit updates the editor title caption (used on hover) to display
the full-path of the editor (similarly to vscode). The change will
especially help in a multi-root workspace where two files with the same
name could not be easily differentiated.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto vince-fugnitto changed the title workspace: update label-provider for multi-root editor: use full path for title captions Oct 7, 2021
@colin-grant-work
Copy link
Contributor

colin-grant-work commented Oct 7, 2021

This is having negative effects on the 'disambiguator' when files with the same name are opened.

  1. Create two folders that are grandchildren of the same folder

e.g. /a/b/c/d and /a/b/e/f

  1. Add a file with the same name to both: /a/b/c/d/hello.txt and /a/b/e/f/hello.txt
  2. Add folders d and f to a workspace and open both hello.txt files.
  3. The disambiguator will use c and e as disambiguators, even though those are parents of the workspace folders.

@vince-fugnitto vince-fugnitto added editor issues related to the editor and removed workspace issues related to the workspace labels Oct 7, 2021
@vince-fugnitto
Copy link
Member Author

@colin-grant-work @alvsan09 I simplified the pull-request to not modify the workspace-uri-label-provider since it might have cascading consequences and instead use the full-path for the editor's title caption.

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Hard to argue with simplicity.

@vince-fugnitto vince-fugnitto merged commit 4e8b563 into master Oct 8, 2021
@vince-fugnitto vince-fugnitto deleted the vf/multi-root-path branch October 8, 2021 12:56
@github-actions github-actions bot added this to the 1.19.0 milestone Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

editor issues related to the editor multi-root issues related to multi-root support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

multi-root: use full path when hovering over open editors

3 participants