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

markers: improve 'description' #7209

Merged
merged 1 commit into from
Feb 27, 2020
Merged

markers: improve 'description' #7209

merged 1 commit into from
Feb 27, 2020

Conversation

vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Feb 25, 2020

What it does

The following pull-request improves the description for the MarkerInfoNodes:

marker-tree-label-provider.ts:

The marker-tree-label-provider.ts has been updated to handle
extra cases where we want custom behavior when displaying the description.
These cases include omitting the description for when a file resource
located at the workspace root is opened with markers in both single
and multiple root workspaces. This behavior aligns the problems-view
further with what VS Code provides.

marker-tree-label-provider.spec.ts:

The marker-tree-label-provider.spec.ts has been updated to include a new
label contribution provider (WorkspaceUriLabelProviderContribution), in
order to get a better label representation of a real-world application. The test
cases have also been updated and split into a more readable and verbose format.
The test cases for getLongName() have been updated to include both single and
multiple root test cases.

How to test

  1. start the application using both a single and multiple root workspace
  2. in general, the following should hold true:
Root Test Case Result (description)
single open a file not at the workspace root the parent directory should be displayed
single open a file at the workspace root no description should be displayed
single open a file outside of the workspace the full path should be displayed
multi open a file at not at one of the workspace roots the root name and parent folder should be displayed
multi open a file at one of the workspace roots only the root name should be displayed
multi open a file outside of any workspace root only the full path of the resource should be displayed

image

Review checklist

Reminder for reviewers

Signed-off-by: Vincent Fugnitto [email protected]

@vince-fugnitto vince-fugnitto added markers issues related to problem markers ui/ux issues related to user interface / user experience labels Feb 25, 2020
@vince-fugnitto vince-fugnitto self-assigned this Feb 25, 2020
The following pull-request improves the `description` for the `MarkerInfoNodes`:

`marker-tree-label-provider.ts`:

The `marker-tree-label-provider.ts` has been updated to handle
extra cases where we want custom behavior when displaying the description.
These cases include omitting the description for when a file resource
located at the workspace root is opened with markers in both single
and multiple root workspaces. This behavior aligns the `problems-view`
further with what VS Code provides.

`marker-tree-label-provider.spec.ts`

The `marker-tree-label-provider.spec.ts` has been updated to include a new
label contribution provider (`WorkspaceUriLabelProviderContribution`), in
order to get a better label representation of a real-world application. The test
cases have also been updated and split into a more readable and verbose format.
The test cases for `getLongName()` have been updated to include both single and
multiple root test cases.

Signed-off-by: Vincent Fugnitto <[email protected]>
@vince-fugnitto
Copy link
Member Author

@lmcbout the pull-request has been rebased to include #7207 and new handling (with corresponding tests) has been added to handle resources outside of the current workspace root(s).

Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

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

Tested on Ubuntu 18.08, Chrome and Electron.
Works fine as expected and tests are also running fine.

Copy link
Contributor

@elaihau elaihau left a comment

Choose a reason for hiding this comment

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

Worked really well.

I tested on gitpod. The following edge cases worked:

  1. Opens one folder as a workspace, and one ts file outside the workspace.
  2. Opens one folder, and its parent folder as the second root, to make a multi root workspace.

Thank you for the change !

@vince-fugnitto
Copy link
Member Author

Thank you for the review @lmcbout @Anasshahidd21 @elaihau !

@vince-fugnitto vince-fugnitto merged commit 34949ab into master Feb 27, 2020
@vince-fugnitto vince-fugnitto deleted the vf/markers branch February 27, 2020 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
markers issues related to problem markers ui/ux issues related to user interface / user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants