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

Allow to shorten the diff editor title #110694

Closed
shasiru opened this issue Nov 16, 2020 · 20 comments
Closed

Allow to shorten the diff editor title #110694

shasiru opened this issue Nov 16, 2020 · 20 comments
Assignees
Labels
diff-editor Diff editor mode issues feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities on-testplan workbench-editors Managing of editor widgets in workbench window

Comments

@shasiru
Copy link

shasiru commented Nov 16, 2020

Issue Type: Feature Request

When comparing two codes side by side, the order is confusing.
It takes a considerable amount of time just to realize which file is on the left and which is on the right.

And the tab title showing comparing files is not helpful at all since it is too long
For example, in tab title;
different file names in same location
.....very/very/very/very/long/path/file1.js <-> .....very/very/very/very/long/path/file2.js

or

same file name in different locations
.....very/very/very/very/long/path1/file1.js <-> .....very/very/very/very/long/path2/file1.js

either situations, I find it really frustrating to identify which file is on which side by reading the long URL (title).

What i propose is to implement the vscode interface to allow the user to add labels with custom titles to each side (when in code comparison).

So instead of a single long URL like this:
screenshot

implement a UI feature like this:
screenshot2

to show clearly which file is open on which side and also allow the user to add custom title names to each of comparing file (temporarily - for reference)

VS Code version: Code 1.51.1 (e5a624b, 2020-11-10T23:34:32.027Z)
OS version: Windows_NT x64 10.0.19042

@ArturoDent
Copy link

Related: #44916

@bpasero
Copy link
Member

bpasero commented Nov 16, 2020

The title reflects the editors, the first name is for the left and the second for the right editor?

@bpasero bpasero added the info-needed Issue requires more information from poster label Nov 16, 2020
@shasiru
Copy link
Author

shasiru commented Nov 16, 2020

The title reflects the editors, the first name is for the left and the second for the right editor?

Yes, you are right.

But the title is taking too much space thus wasting time realizing which code is on which side.

@bpasero bpasero changed the title A convenient code comparison Allow to shorten the diff editor title Nov 16, 2020
@bpasero bpasero added diff-editor Diff editor mode issues feature-request Request for new features or functionality workbench-editors Managing of editor widgets in workbench window and removed info-needed Issue requires more information from poster labels Nov 16, 2020
@bpasero bpasero removed their assignment Nov 16, 2020
@bpasero bpasero added this to the Backlog Candidates milestone Nov 16, 2020
@bpasero
Copy link
Member

bpasero commented Nov 30, 2020

With #109224 at least for files in the same folder, the label will be much shorter now:

Before
image

After
image

@bpasero
Copy link
Member

bpasero commented Nov 30, 2020

The UI challenge here is really that we have many settings that enable a description to appear besides the main editor label (e.g. when you disable tabs) and we typically put the relative path of the file to the root in there, if we did something like that for diff editors, it would end up like this:

image

If someone has better ideas how to resolve this, I am all up for making a change.

@shasiru
Copy link
Author

shasiru commented Dec 11, 2020

The UI challenge here is really that we have many settings that enable a description to appear besides the main editor label (e.g. when you disable tabs) and we typically put the relative path of the file to the root in there, if we did something like that for diff editors, it would end up like this:

image

If someone has better ideas how to resolve this, I am all up for making a change.

How about if we separate the causes for this issue into main two factors:

  • when files have different file names
  • when files have the same name but stored in different locations

when comparing two files with different files names, you can completely ignore the path. For example:

tab2

If the files names are exactly the same, then I suggest to check the path from the lowest level of the url and go up from there looking for the first mismatching directory and when found one, show that directory in actual characters while showing rest of the directories as ellipsis (/.../). For example:

lets assume the file paths are as such:

tab_fileurls2

Since the first mismatching directories from bottom up url search are the "GIT" and "Backups", the tab title can be something like this:

tab1

In worst case scenario, let's assume the files are stored in completely different locations.
For example:

tab_fileurls1

Since there are no part of the urls are the same, the tab title can be something like this:
tab3

Besides, the vs code already has the feature to show the full path when the cursor is hovering above the tab title, I believe this method would be practical in any case.

Any thoughts??

@bpasero
Copy link
Member

bpasero commented Dec 12, 2020

@shasiru yeah I somewhat like that idea, if you want to give it a try through a PR, the change would need to go somewhere here:

getName(): string {
if (!this.name) {
// Craft a name from original and modified input that includes the
// relative path in case both sides have different parents and we
// compare file resources.
const fileResources = this.asFileResources();
if (fileResources && dirname(fileResources.original).path !== dirname(fileResources.modified).path) {
return `${this.labelService.getUriLabel(fileResources.original, { relative: true })}${this.labelService.getUriLabel(fileResources.modified, { relative: true })}`;
}
return localize('sideBySideLabels', "{0} ↔ {1}", this.originalInput.getName(), this.modifiedInput.getName());
}
return this.name;
}

Interestingly we have a similar challenge already when you open 2 editors with the same name we try to put a description next to the file name that can be used to distinguish the one file from the other:

image

That code lives here and maybe can be reused for this task?

private shortenTabLabels(labels: AugmentedLabel[]): void {

@bpasero bpasero added the help wanted Issues identified as good community contribution opportunities label Dec 12, 2020
@bpasero bpasero modified the milestones: Backlog Candidates, Backlog Dec 12, 2020
@goldst
Copy link
Contributor

goldst commented Dec 13, 2020

I'd like to suggest that it should use the same format to differentiate between the paths then, like this:
workBenchTestService.ts .../common <-> workBenchTestService.ts .../browser

What do you think about this format? @shasiru if you don't want to implement it and @bpasero if that's ok for you, I'd like to give it a try.

@shasiru
Copy link
Author

shasiru commented Dec 13, 2020

Although I've already started working on it, I realized this won't be implemented by me anytime soon, because I'm very busy with other several projects at this time.

@goldst, Thanks, be my guest.. the sooner someone comes up with a reliable implementation is better... 😊

@jeanp413
Copy link
Contributor

jeanp413 commented Jan 8, 2021

@goldst are you working on this? if not I can take a look

@goldst
Copy link
Contributor

goldst commented Jan 8, 2021

@jeanp413 yes I am, was occupied with other things during the holidays but will likely finish this weekend.

@goldst
Copy link
Contributor

goldst commented Jan 10, 2021

Okay, I have now both versions working, I'd just need to fix a few bugs on version B.
@bpasero Are you ok with me finishing version B or should I PR version A?

Version A: just shorten diff path
Version A

Version B: implements multiple labels for tabs, the path is in description labels
Version B

Reasons for version A:

  • fewer changes
  • less potential for bugs
  • is finished
  • slightly shorter tabs for short paths

Reasons for version B:

  • UI is more consistent
  • slightly shorter tabs for long paths

@bpasero
Copy link
Member

bpasero commented Jan 11, 2021

@goldst I am happy to look at the changes for both to understand the consequence. as for "less potential for bugs", I would say that some unit tests would help to improve on that.

goldst added a commit to goldst/vscode that referenced this issue Jan 18, 2021
goldst added a commit to goldst/vscode that referenced this issue Jan 18, 2021
@goldst
Copy link
Contributor

goldst commented Jan 18, 2021

Thanks, @bpasero for the comment! I have redone solution B more cleanly now.

I can PR as soon as you tell me which one. I prefer B.
A: master...goldst:shorten-diff-title-A
B: master...goldst:multiple-tab-labels-B

@goldst
Copy link
Contributor

goldst commented Feb 7, 2021

@bpasero short reminder 😄 If something is unclear, feel free to ask.

@bpasero
Copy link
Member

bpasero commented Feb 9, 2021

I can currently not allocate time for this but suggest to still do a PR with your proposals (maybe link to the alternate solution from the PR).

@bpasero
Copy link
Member

bpasero commented Sep 19, 2021

Apologies for the delay in working on this but as we get closer to our issue grooming iteration in October, I decided to look into this again with a fresh mind. @goldst I looked at the PR in #116178 but had a hard time following along given the amount of changes. Instead I tried to implement this with concepts we already had, let me try to explain:

My solution aims to leverage existing concepts we already had for normal editors:

  • with default settings, a normal file editor tab always just shows the file name
  • if there are more than 1 tab open with the same name, we show a description that indicates the difference of the 2 tabs in the folders they are contained in

image

Taking this concept over to diff editors:

  • we should always show filenameA ↔ filenameB as the main label for a tab to have a brief and readable label that puts emphasise on the file names and not their folders
  • if both filenameA and filenameB are the same, we should indicate the difference similarly to what we do for normal file editor tabs when the same name appears twice and show a description that is shortened to reflect the difference in the folders they are contained in

Examples:

image

image

image

image

I didn't find the repeated usage of the character in the description so compelling so I went with just -. The ideal solution here would probably be to allow to have something like labelA | descriptionA ↔ labelB | descriptionB but that is a lot more work involved.

Thoughts?

@bpasero bpasero self-assigned this Sep 19, 2021
@bpasero bpasero modified the milestones: Backlog, September 2021 Sep 19, 2021
@goldst
Copy link
Contributor

goldst commented Sep 20, 2021

@bpasero thanks for having a look at my code! I agree that it contains a lot of changes and that this might be a reason to choose a simpler solution. It would have been nice to know that in advance though, but no worries.

Your solution looks good! There is one edge case in which I think it should behave differently (not sure if this should be part of this issue or a separate one):

grafik

Here, there are two different workbenchTestServices.ts files opened, one in .../common and one in .../browser, but only one of them is in a diff. I think both tabs should have a label here, because they would if both of them were a normal tab.

@bpasero
Copy link
Member

bpasero commented Sep 20, 2021

@goldst yeah you are right, the computation of labels for tabs is strictly identifying duplicates and not cases where a label is made up of 2 sides where one is a duplicate of others. I suggest to treat that as a separate issue and allow this one to close for verification.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
diff-editor Diff editor mode issues feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities on-testplan workbench-editors Managing of editor widgets in workbench window
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@bpasero @shasiru @jeanp413 @ArturoDent @goldst and others