Skip to content

Commit

Permalink
editors - shorten diff labels (#110694)
Browse files Browse the repository at this point in the history
  • Loading branch information
bpasero committed Sep 19, 2021
1 parent 54cdd9c commit adee8fa
Show file tree
Hide file tree
Showing 12 changed files with 218 additions and 139 deletions.
7 changes: 0 additions & 7 deletions extensions/git/src/fileSystemProvider.ts
Expand Up @@ -48,13 +48,6 @@ export class GitFileSystemProvider implements FileSystemProvider {
model.onDidChangeRepository(this.onDidChangeRepository, this),
model.onDidChangeOriginalResource(this.onDidChangeOriginalResource, this),
workspace.registerFileSystemProvider('git', this, { isReadonly: true, isCaseSensitive: true }),
workspace.registerResourceLabelFormatter({

This comment has been minimized.

Copy link
@bpasero

bpasero Sep 20, 2021

Author Member

@joaomoreno @eamodio fyi I decided to remove this formatter for Git because it was constantly fighting with our built in one which imho is a lot better when it comes to paths. With this change, git files nicely appear as other files with respect to their path presentation, e.g.:

image

vs before:

image

This comment has been minimized.

Copy link
@joaomoreno

joaomoreno Sep 29, 2021

Member

Awesome!

scheme: 'git',
formatting: {
label: '${path} (git)',
separator: '/'
}
})
);

setInterval(() => this.cleanup(), FIVE_MINUTES);
Expand Down
2 changes: 1 addition & 1 deletion src/vs/base/browser/ui/iconLabel/iconlabel.css
Expand Up @@ -87,7 +87,7 @@
opacity: 0.75;
font-size: 90%;
font-weight: 600;
margin: 0 16px 0 5px;
margin: auto 16px 0 5px; /* https://github.com/microsoft/vscode/issues/113223 */
text-align: center;
}

Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/browser/parts/editor/editorQuickAccess.ts
Expand Up @@ -155,7 +155,7 @@ export abstract class BaseEditorQuickAccessProvider extends PickerQuickAccessPro

return isDirty ? localize('entryAriaLabelDirty', "{0}, dirty", nameAndDescription) : nameAndDescription;
})(),
description: editor.getDescription(),
description,
iconClasses: getIconClasses(this.modelService, this.modeService, resource).concat(editor.getLabelExtraClasses()),
italic: !this.editorGroupService.getGroup(groupId)?.isPinned(editor),
buttons: (() => {
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/browser/parts/editor/noTabsTitleControl.ts
Expand Up @@ -78,13 +78,13 @@ export class NoTabsTitleControl extends TitleControl {
this._register(addDisposableListener(titleContainer, TouchEventType.Tap, (e: GestureEvent) => this.onTitleTap(e)));

// Context Menu
[EventType.CONTEXT_MENU, TouchEventType.Contextmenu].forEach(event => {
for (const event of [EventType.CONTEXT_MENU, TouchEventType.Contextmenu]) {
this._register(addDisposableListener(titleContainer, event, e => {
if (this.group.activeEditor) {
this.onContextMenu(this.group.activeEditor, e, titleContainer);
}
}));
});
}
}

private onTitleLabelClick(e: MouseEvent): void {
Expand Down
85 changes: 46 additions & 39 deletions src/vs/workbench/browser/parts/editor/tabsTitleControl.ts
Expand Up @@ -6,7 +6,7 @@
import 'vs/css!./media/tabstitlecontrol';
import { isMacintosh, isWindows } from 'vs/base/common/platform';
import { shorten } from 'vs/base/common/labels';
import { EditorResourceAccessor, GroupIdentifier, Verbosity, IEditorPartOptions, SideBySideEditor, DEFAULT_EDITOR_ASSOCIATION } from 'vs/workbench/common/editor';
import { EditorResourceAccessor, GroupIdentifier, Verbosity, IEditorPartOptions, SideBySideEditor, DEFAULT_EDITOR_ASSOCIATION, EditorInputCapabilities } from 'vs/workbench/common/editor';
import { EditorInput } from 'vs/workbench/common/editor/editorInput';
import { computeEditorAriaLabel } from 'vs/workbench/browser/editor';
import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
Expand Down Expand Up @@ -40,7 +40,7 @@ import { CloseOneEditorAction, UnpinEditorAction } from 'vs/workbench/browser/pa
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { BreadcrumbsControl } from 'vs/workbench/browser/parts/editor/breadcrumbsControl';
import { IFileService } from 'vs/platform/files/common/files';
import { withNullAsUndefined, assertAllDefined, assertIsDefined } from 'vs/base/common/types';
import { assertAllDefined, assertIsDefined } from 'vs/base/common/types';
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
import { basenameOrAuthority } from 'vs/base/common/resources';
import { RunOnceScheduler } from 'vs/base/common/async';
Expand All @@ -56,11 +56,12 @@ import { UNLOCK_GROUP_COMMAND_ID } from 'vs/workbench/browser/parts/editor/edito
interface IEditorInputLabel {
name?: string;
description?: string;
forceDescription?: boolean;
title?: string;
ariaLabel?: string;
}

type AugmentedLabel = IEditorInputLabel & { editor: EditorInput };
type IEditorInputLabelAndEditor = IEditorInputLabel & { editor: EditorInput };

export class TabsTitleControl extends TitleControl {

Expand Down Expand Up @@ -218,7 +219,7 @@ export class TabsTitleControl extends TitleControl {
}));

// New file when double clicking on tabs container (but not tabs)
[TouchEventType.Tap, EventType.DBLCLICK].forEach(eventType => {
for (const eventType of [TouchEventType.Tap, EventType.DBLCLICK]) {
this._register(addDisposableListener(tabsContainer, eventType, (e: MouseEvent | GestureEvent) => {
if (eventType === EventType.DBLCLICK) {
if (e.target !== tabsContainer) {
Expand All @@ -245,7 +246,7 @@ export class TabsTitleControl extends TitleControl {
}
}, this.group.id);
}));
});
}

// Prevent auto-scrolling (https://github.com/microsoft/vscode/issues/16690)
this._register(addDisposableListener(tabsContainer, EventType.MOUSE_DOWN, e => {
Expand Down Expand Up @@ -784,7 +785,7 @@ export class TabsTitleControl extends TitleControl {
}));

// Double click: either pin or toggle maximized
[TouchEventType.Tap, EventType.DBLCLICK].forEach(eventType => {
for (const eventType of [TouchEventType.Tap, EventType.DBLCLICK]) {
disposables.add(addDisposableListener(tab, eventType, (e: MouseEvent | GestureEvent) => {
if (eventType === EventType.DBLCLICK) {
EventHelper.stop(e);
Expand All @@ -799,7 +800,7 @@ export class TabsTitleControl extends TitleControl {
this.group.pinEditor(editor);
}
}));
});
}

// Context menu
disposables.add(addDisposableListener(tab, EventType.CONTEXT_MENU, e => {
Expand Down Expand Up @@ -953,11 +954,12 @@ export class TabsTitleControl extends TitleControl {
const { verbosity, shortenDuplicates } = this.getLabelConfigFlags(labelFormat);

// Build labels and descriptions for each editor
const labels = this.group.editors.map((editor, index) => ({
const labels: IEditorInputLabelAndEditor[] = this.group.editors.map((editor, index) => ({
editor,
name: editor.getName(),
description: editor.getDescription(verbosity),
title: withNullAsUndefined(editor.getTitle(Verbosity.LONG)),
forceDescription: editor.hasCapability(EditorInputCapabilities.ForceDescription),
title: editor.getTitle(Verbosity.LONG),
ariaLabel: computeEditorAriaLabel(editor, index, this.group, this.editorGroupService.count)
}));

Expand All @@ -969,63 +971,68 @@ export class TabsTitleControl extends TitleControl {
this.tabLabels = labels;
}

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

// Gather duplicate titles, while filtering out invalid descriptions
const mapTitleToDuplicates = new Map<string, AugmentedLabel[]>();
const mapNameToDuplicates = new Map<string, IEditorInputLabelAndEditor[]>();
for (const label of labels) {
if (typeof label.description === 'string') {
getOrSet(mapTitleToDuplicates, label.name, []).push(label);
getOrSet(mapNameToDuplicates, label.name, []).push(label);
} else {
label.description = '';
}
}

// Identify duplicate titles and shorten descriptions
mapTitleToDuplicates.forEach(duplicateTitles => {
// Identify duplicate names and shorten descriptions
for (const [, duplicateLabels] of mapNameToDuplicates) {

// Remove description if the title isn't duplicated
if (duplicateTitles.length === 1) {
duplicateTitles[0].description = '';
// and we have no indication to enforce description
if (duplicateLabels.length === 1 && !duplicateLabels[0].forceDescription) {
duplicateLabels[0].description = '';

return;
continue;
}

// Identify duplicate descriptions
const mapDescriptionToDuplicates = new Map<string, AugmentedLabel[]>();
for (const label of duplicateTitles) {
getOrSet(mapDescriptionToDuplicates, label.description, []).push(label);
const mapDescriptionToDuplicates = new Map<string, IEditorInputLabelAndEditor[]>();
for (const duplicateLabel of duplicateLabels) {
getOrSet(mapDescriptionToDuplicates, duplicateLabel.description, []).push(duplicateLabel);
}

// For editors with duplicate descriptions, check whether any long descriptions differ
let useLongDescriptions = false;
mapDescriptionToDuplicates.forEach((duplicateDescriptions, name) => {
if (!useLongDescriptions && duplicateDescriptions.length > 1) {
const [first, ...rest] = duplicateDescriptions.map(({ editor }) => editor.getDescription(Verbosity.LONG));
for (const [, duplicateLabels] of mapDescriptionToDuplicates) {
if (!useLongDescriptions && duplicateLabels.length > 1) {
const [first, ...rest] = duplicateLabels.map(({ editor }) => editor.getDescription(Verbosity.LONG));
useLongDescriptions = rest.some(description => description !== first);
}
});
}

// If so, replace all descriptions with long descriptions
if (useLongDescriptions) {
mapDescriptionToDuplicates.clear();
duplicateTitles.forEach(label => {
label.description = label.editor.getDescription(Verbosity.LONG);
getOrSet(mapDescriptionToDuplicates, label.description, []).push(label);
});
for (const duplicateLabel of duplicateLabels) {
duplicateLabel.description = duplicateLabel.editor.getDescription(Verbosity.LONG);
getOrSet(mapDescriptionToDuplicates, duplicateLabel.description, []).push(duplicateLabel);
}
}

// Obtain final set of descriptions
const descriptions: string[] = [];
mapDescriptionToDuplicates.forEach((_, description) => descriptions.push(description));
for (const [description] of mapDescriptionToDuplicates) {
descriptions.push(description);
}

// Remove description if all descriptions are identical
// Remove description if all descriptions are identical unless forced
if (descriptions.length === 1) {
for (const label of mapDescriptionToDuplicates.get(descriptions[0]) || []) {
label.description = '';
if (!label.forceDescription) {
label.description = '';
}
}

return;
continue;
}

// Shorten descriptions
Expand All @@ -1035,7 +1042,7 @@ export class TabsTitleControl extends TitleControl {
label.description = shortenedDescriptions[index];
}
});
});
}
}

private getLabelConfigFlags(value: string | undefined) {
Expand Down Expand Up @@ -1096,21 +1103,21 @@ export class TabsTitleControl extends TitleControl {

// Settings
const tabActionsVisibility = isTabSticky && options.pinnedTabSizing === 'compact' ? 'off' /* treat sticky compact tabs as tabCloseButton: 'off' */ : options.tabCloseButton;
['off', 'left', 'right'].forEach(option => {
for (const option of ['off', 'left', 'right']) {
tabContainer.classList.toggle(`tab-actions-${option}`, tabActionsVisibility === option);
});
}

const tabSizing = isTabSticky && options.pinnedTabSizing === 'shrink' ? 'shrink' /* treat sticky shrink tabs as tabSizing: 'shrink' */ : options.tabSizing;
['fit', 'shrink'].forEach(option => {
for (const option of ['fit', 'shrink']) {
tabContainer.classList.toggle(`sizing-${option}`, tabSizing === option);
});
}

tabContainer.classList.toggle('has-icon', options.showIcons && options.hasIcons);

tabContainer.classList.toggle('sticky', isTabSticky);
['normal', 'compact', 'shrink'].forEach(option => {
for (const option of ['normal', 'compact', 'shrink']) {
tabContainer.classList.toggle(`sticky-${option}`, isTabSticky && options.pinnedTabSizing === option);
});
}

// Sticky compact/shrink tabs need a position to remain at their location
// when scrolling to stay in view (requirement for position: sticky)
Expand Down
10 changes: 9 additions & 1 deletion src/vs/workbench/common/editor.ts
Expand Up @@ -522,7 +522,15 @@ export const enum EditorInputCapabilities {
* Signals that the editor can split into 2 in the same
* editor group.
*/
CanSplitInGroup = 1 << 5
CanSplitInGroup = 1 << 5,

/**
* Signals that the editor wants it's description to be
* visible when presented to the user. By default, a UI
* component may decide to hide the description portion
* for brevity.
*/
ForceDescription = 1 << 6

This comment has been minimized.

Copy link
@bpasero

bpasero Sep 20, 2021

Author Member

@meganrogge @Tyriar fyi this maybe useful for terminals. I recall a Slack chat where you expressed interest in always showing the current working directory of a terminal tab. With this capability you can enforce that the description of the editor input appears in the tab label, even when we would otherwise decide to not show it.

This comment has been minimized.

Copy link
@Tyriar

Tyriar Sep 20, 2021

Member
}

export type IUntypedEditorInput = IResourceEditorInput | ITextResourceEditorInput | IUntitledTextResourceEditorInput | IResourceDiffEditorInput | IResourceSideBySideEditorInput;
Expand Down

0 comments on commit adee8fa

Please sign in to comment.