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.

Sign up for GitHub

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

Jump to bottom

Update editor tabbar captions for better multi-root support #5924

Merged
merged 1 commit into from
Aug 13, 2019
Merged

Update editor tabbar captions for better multi-root support #5924

merged 1 commit into from
Aug 13, 2019

Conversation

vince-fugnitto
Copy link
Member

What it does

Fixes #5921

  • updated the editor's caption (used for tabbars) in order
    to better support multi-root workspaces, and align with VSCode.
    Currently, when in a multi-root workspace there is no way to
    easily distinguish which root the file resides in. With this
    change the entire path is displayed, similarly to the explorer.

How to test

  1. open a single-root workspace, open an editor, and highlight over the tabbar (the entire path should be displayed as the html title)
  2. open a multi-root workspace, open an editor, and highlight over the tabbar (the entire path should be displayed as the html title)

Review checklist

Reminder for reviewers

Signed-off-by: Vincent Fugnitto vincent.fugnitto@ericsson.com

@vince-fugnitto
Update editor tabbar captions for better multi-root support
ccd9bce 
Fixes #5921

- updated the editor's caption (used for tabbars) in order
to better support multi-root workspaces, and align with VSCode.
Currently, when in a multi-root workspace there is no way to
easily distinguish which root the file resides in. With this
change the entire path is displayed, similarly to the explorer.

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves multi-root issues related to multi-root support labels Aug 13, 2019
@vince-fugnitto
Copy link
Member Author

vince-fugnitto commented Aug 13, 2019 •
edited
Loading

@akosyakov I know the coding guidelines state to use the labelProvider.getLongName() in order to get a uniform representation of the path systemwide, but it does not work correctly for the following case.

Debugging the code I can see that WorkspaceUriLabelProviderContribution is used while I require the default implementation present in the DefaultUriLabelProviderContribution.

https://github.com/theia-ide/theia/blob/11badc80598254e8c78a204b7b93c2368a300fe4/packages/core/src/browser/label-provider.ts#L83-L86

Is there a way to be able to programmatically decide which contribution I want to handle the implementation of the method (ex: Default over Workspace)?

In order to get the correct results, I followed the logic present in the file-tree-widget.tsx:

https://github.com/theia-ide/theia/blob/11badc80598254e8c78a204b7b93c2368a300fe4/packages/filesystem/src/browser/file-tree/file-tree-widget.tsx#L96

akosyakov
akosyakov reviewed Aug 13, 2019
View reviewed changes
packages/editor/src/browser/editor-widget-factory.ts
@@ -50,7 +50,7 @@ export class EditorWidgetFactory implements WidgetFactory {
newEditor.title.closable = true;
newEditor.title.label = this.labelProvider.getName(uri);
newEditor.title.iconClass = icon + ' file-icon';
newEditor.title.caption = this.labelProvider.getLongName(uri);
Copy link
Member

Choose a reason for hiding this comment

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

but it breaks single root scenarios? Should not we work on improving getLongName for multi-roots?

Have you checked what VS Code show? I would imagine we can show something like README.md (cpp)?

akosyakov
akosyakov approved these changes Aug 13, 2019
View reviewed changes
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

ok, checked with VS Code, for tooltips always the full path is used

@vince-fugnitto
Copy link
Member Author

ok, checked with VS Code, for tooltips always the full path is used

Sorry I just got to work, yes exactly, they display the long path irregardless of if its a single or multi-root workspace.

@vince-fugnitto vince-fugnitto merged commit a6720b5 into master Aug 13, 2019
@vince-fugnitto vince-fugnitto deleted the vf/GH-5921 branch August 13, 2019 10:48
@vince-fugnitto vince-fugnitto mentioned this pull request Aug 13, 2019
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akosyakov akosyakov akosyakov approved these changes

Assignees
No one assigned
Labels
enhancement issues that are enhancements to current functionality - nice to haves multi-root issues related to multi-root support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[core] better tabbar support for multi-root workspaces
2 participants
@vince-fugnitto @akosyakov