Skip to content

Commit

Permalink
Improves comparison file filtering enabled state
Browse files Browse the repository at this point in the history 
Remembers comparison file filtering on filter/refresh
Disallows comparisons with the working tree (e.g. right side)
Disables swap when comparing from the working tree

Adds new in-memory node state storage to views
  • Loading branch information
@eamodio
eamodio committed Jul 20, 2022
1 parent 2dc29eb commit 17457b5
Show file tree
Hide file tree
Showing 6 changed files with 197 additions and 114 deletions.
34 changes: 31 additions & 3 deletions package.json
View file
Expand Up @@ -5880,7 +5880,8 @@
"command": "gitlens.views.searchAndCompare.swapComparison",
"title": "Swap Comparison",
"category": "GitLens",
"icon": "$(arrow-swap)"
"icon": "$(arrow-swap)",
"enablement": "viewItem =~ /gitlens:compare:results(?!:)\\b(?!.*?\\b\\+working\\b)/"
},
{
"command": "gitlens.views.searchAndCompare.setFilesFilterOnLeft",
Expand Down Expand Up @@ -9738,7 +9739,12 @@
},
{
"submenu": "gitlens/view/searchAndCompare/comparison/filter",
"when": "viewItem =~ /gitlens:results:files\\b(?=.*?\\b\\+filterable\\b)/",
"when": "viewItem =~ /gitlens:results:files\\b(?=.*?\\b\\+filterable\\b)(?!.*?\\b\\+filtered\\b)/",
"group": "inline@1"
},
{
"submenu": "gitlens/view/searchAndCompare/comparison/filtered",
"when": "viewItem =~ /gitlens:results:files\\b(?=.*?\\b\\+filterable\\b)(?=.*?\\b\\+filtered\\b)/",
"group": "inline@1"
},
{
Expand All @@ -9763,7 +9769,7 @@
},
{
"command": "gitlens.views.searchAndCompare.swapComparison",
"when": "viewItem =~ /gitlens:compare:results(?!:)\\b/",
"when": "viewItem =~ /gitlens:compare:results(?!:)\\b(?!.*?\\b\\+working\\b)/",
"group": "1_gitlens_actions@2"
},
{
Expand Down Expand Up @@ -10416,6 +10422,23 @@
"when1": "viewItem =~ /gitlens:results:files\\b(?!.*?\\b\\+filtered~right\\b)/",
"group": "navigation_1@2"
}
],
"gitlens/view/searchAndCompare/comparison/filtered": [
{
"command": "gitlens.views.searchAndCompare.setFilesFilterOff",
"when": "viewItem =~ /gitlens:results:files\\b(?=.*?\\b\\+filtered\\b)/",
"group": "navigation@1"
},
{
"command": "gitlens.views.searchAndCompare.setFilesFilterOnLeft",
"when1": "viewItem =~ /gitlens:results:files\\b(?!.*?\\b\\+filtered~left\\b)/",
"group": "navigation_1@1"
},
{
"command": "gitlens.views.searchAndCompare.setFilesFilterOnRight",
"when1": "viewItem =~ /gitlens:results:files\\b(?!.*?\\b\\+filtered~right\\b)/",
"group": "navigation_1@2"
}
]
},
"submenus": [
Expand Down Expand Up @@ -10484,6 +10507,11 @@
"id": "gitlens/view/searchAndCompare/comparison/filter",
"label": "Filter",
"icon": "$(filter)"
},
{
"id": "gitlens/view/searchAndCompare/comparison/filtered",
"label": "Filter",
"icon": "$(filter-filled)"
}
],
"keybindings": [
Expand Down
44 changes: 24 additions & 20 deletions src/views/nodes/compareBranchNode.ts
View file
Expand Up @@ -8,6 +8,7 @@ import { ReferencePicker } from '../../quickpicks/referencePicker';
import { BranchComparison, BranchComparisons, WorkspaceStorageKeys } from '../../storage';
import { gate } from '../../system/decorators/gate';
import { debug, log } from '../../system/decorators/log';
import { getSettledValue } from '../../system/promise';
import { pluralize } from '../../system/string';
import { BranchesView } from '../branchesView';
import { CommitsView } from '../commitsView';
Expand Down Expand Up @@ -82,7 +83,7 @@ export class CompareBranchNode extends ViewNode<BranchesView | CommitsView | Rep
new ResultsCommitsNode(
this.view,
this,
this.uri.repoPath!,
this.repoPath,
'Behind',
{
query: this.getCommitsQuery(GitRevision.createRange(behind.ref1, behind.ref2, '..')),
Expand All @@ -103,7 +104,7 @@ export class CompareBranchNode extends ViewNode<BranchesView | CommitsView | Rep
new ResultsCommitsNode(
this.view,
this,
this.uri.repoPath!,
this.repoPath,
'Ahead',
{
query: this.getCommitsQuery(
Expand All @@ -126,7 +127,7 @@ export class CompareBranchNode extends ViewNode<BranchesView | CommitsView | Rep
new ResultsFilesNode(
this.view,
this,
this.uri.repoPath!,
this.repoPath,
this._compareWith.ref || 'HEAD',
this.compareWithWorkingTree ? '' : this.branch.ref,
this.getFilesQuery.bind(this),
Expand Down Expand Up @@ -249,15 +250,21 @@ export class CompareBranchNode extends ViewNode<BranchesView | CommitsView | Rep
}

private async getAheadFilesQuery(): Promise<FilesQueryResults> {
let files = await this.view.container.git.getDiffStatus(
this.repoPath,
GitRevision.createRange(this._compareWith?.ref || 'HEAD', this.branch.ref || 'HEAD', '...'),
);
const comparison = GitRevision.createRange(this._compareWith?.ref || 'HEAD', this.branch.ref || 'HEAD', '...');

const [filesResult, workingFilesResult] = await Promise.allSettled([
this.view.container.git.getDiffStatus(this.repoPath, comparison),
this.compareWithWorkingTree ? this.view.container.git.getDiffStatus(this.repoPath, 'HEAD') : undefined,
]);

let files = getSettledValue(filesResult) ?? [];

if (this.compareWithWorkingTree) {
const workingFiles = await this.view.container.git.getDiffStatus(this.repoPath, 'HEAD');
const workingFiles = getSettledValue(workingFilesResult);
if (workingFiles != null) {
if (files != null) {
if (files.length === 0) {
files = workingFiles;
} else {
for (const wf of workingFiles) {
const index = files.findIndex(f => f.path === wf.path);
if (index !== -1) {
Expand All @@ -266,32 +273,29 @@ export class CompareBranchNode extends ViewNode<BranchesView | CommitsView | Rep
files.push(wf);
}
}
} else {
files = workingFiles;
}
}
}

return {
label: `${pluralize('file', files?.length ?? 0, { zero: 'No' })} changed`,
label: `${pluralize('file', files.length, { zero: 'No' })} changed`,
files: files,
};
}

private async getBehindFilesQuery(): Promise<FilesQueryResults> {
const files = await this.view.container.git.getDiffStatus(
this.uri.repoPath!,
GitRevision.createRange(this.branch.ref, this._compareWith?.ref || 'HEAD', '...'),
);
const comparison = GitRevision.createRange(this.branch.ref, this._compareWith?.ref || 'HEAD', '...');

const files = (await this.view.container.git.getDiffStatus(this.repoPath, comparison)) ?? [];

return {
label: `${pluralize('file', files?.length ?? 0, { zero: 'No' })} changed`,
label: `${pluralize('file', files.length, { zero: 'No' })} changed`,
files: files,
};
}

private getCommitsQuery(range: string): (limit: number | undefined) => Promise<CommitsQueryResults> {
const repoPath = this.uri.repoPath!;
const repoPath = this.repoPath;
return async (limit: number | undefined) => {
const log = await this.view.container.git.getLog(repoPath, {
limit: limit,
Expand Down Expand Up @@ -323,10 +327,10 @@ export class CompareBranchNode extends ViewNode<BranchesView | CommitsView | Rep
comparison = `${this._compareWith.ref}..${this.branch.ref}`;
}

const files = await this.view.container.git.getDiffStatus(this.uri.repoPath!, comparison);
const files = (await this.view.container.git.getDiffStatus(this.repoPath, comparison)) ?? [];

return {
label: `${pluralize('file', files?.length ?? 0, { zero: 'No' })} changed`,
label: `${pluralize('file', files.length, { zero: 'No' })} changed`,
files: files,
};
}
Expand Down
84 changes: 41 additions & 43 deletions src/views/nodes/compareResultsNode.ts
View file
@@ -1,9 +1,10 @@
import { ThemeIcon, TreeItem, TreeItemCollapsibleState } from 'vscode';
import { ThemeIcon, TreeItem, TreeItemCollapsibleState, window } from 'vscode';
import { GitUri } from '../../git/gitUri';
import { GitRevision } from '../../git/models';
import { NamedRef } from '../../storage';
import { gate } from '../../system/decorators/gate';
import { debug, log } from '../../system/decorators/log';
import { getSettledValue } from '../../system/promise';
import { md5, pluralize } from '../../system/string';
import { SearchAndCompareView } from '../searchAndCompareView';
import { RepositoryNode } from './repositoryNode';
Expand Down Expand Up @@ -86,7 +87,7 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {
new ResultsCommitsNode(
this.view,
this,
this.uri.repoPath!,
this.repoPath,
'Behind',
{
query: this.getCommitsQuery(GitRevision.createRange(behind.ref1, behind.ref2, '..')),
Expand All @@ -107,7 +108,7 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {
new ResultsCommitsNode(
this.view,
this,
this.uri.repoPath!,
this.repoPath,
'Ahead',
{
query: this.getCommitsQuery(GitRevision.createRange(ahead.ref1, ahead.ref2, '..')),
Expand All @@ -128,7 +129,7 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {
new ResultsFilesNode(
this.view,
this,
this.uri.repoPath!,
this.repoPath,
this._compareWith.ref,
this._ref.ref,
this.getFilesQuery.bind(this),
Expand All @@ -145,8 +146,8 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {
getTreeItem(): TreeItem {
let description;
if (this.view.container.git.repositoryCount > 1) {
const repo = this.uri.repoPath ? this.view.container.git.getRepository(this.uri.repoPath) : undefined;
description = repo?.formattedName ?? this.uri.repoPath;
const repo = this.repoPath ? this.view.container.git.getRepository(this.repoPath) : undefined;
description = repo?.formattedName ?? this.repoPath;
}

const item = new TreeItem(
Expand All @@ -159,7 +160,9 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {
TreeItemCollapsibleState.Collapsed,
);
item.id = this.id;
item.contextValue = `${ContextValues.CompareResults}${this._pinned ? '+pinned' : ''}`;
item.contextValue = `${ContextValues.CompareResults}${this._pinned ? '+pinned' : ''}${
this._ref.ref === '' ? '+working' : ''
}`;
item.description = description;
if (this._pinned) {
item.iconPath = new ThemeIcon('pinned');
Expand Down Expand Up @@ -194,6 +197,11 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {

@log()
async swap() {
if (this._ref.ref === '') {
void window.showErrorMessage('Cannot swap comparisons with the working tree');
return;
}

// Save the current id so we can update it later
const currentId = this.getPinnableId();

Expand Down Expand Up @@ -227,45 +235,36 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {
}

private async getAheadFilesQuery(): Promise<FilesQueryResults> {
let files = await this.view.container.git.getDiffStatus(
this.repoPath,
return this.getAheadBehindFilesQuery(
GitRevision.createRange(this._compareWith?.ref || 'HEAD', this._ref.ref || 'HEAD', '...'),
this._ref.ref === '',
);

if (this._ref.ref === '') {
const workingFiles = await this.view.container.git.getDiffStatus(this.repoPath, 'HEAD');
if (workingFiles != null) {
if (files != null) {
for (const wf of workingFiles) {
const index = files.findIndex(f => f.path === wf.path);
if (index !== -1) {
files.splice(index, 1, wf);
} else {
files.push(wf);
}
}
} else {
files = workingFiles;
}
}
}

return {
label: `${pluralize('file', files?.length ?? 0, { zero: 'No' })} changed`,
files: files,
};
}

private async getBehindFilesQuery(): Promise<FilesQueryResults> {
let files = await this.view.container.git.getDiffStatus(
this.repoPath,
return this.getAheadBehindFilesQuery(
GitRevision.createRange(this._ref.ref || 'HEAD', this._compareWith.ref || 'HEAD', '...'),
false,
);
}

if (this._compareWith.ref === '') {
const workingFiles = await this.view.container.git.getDiffStatus(this.repoPath, 'HEAD');
private async getAheadBehindFilesQuery(
comparison: string,
compareWithWorkingTree: boolean,
): Promise<FilesQueryResults> {
const [filesResult, workingFilesResult] = await Promise.allSettled([
this.view.container.git.getDiffStatus(this.repoPath, comparison),
compareWithWorkingTree ? this.view.container.git.getDiffStatus(this.repoPath, 'HEAD') : undefined,
]);

let files = getSettledValue(filesResult) ?? [];

if (compareWithWorkingTree) {
const workingFiles = getSettledValue(workingFilesResult);
if (workingFiles != null) {
if (files != null) {
if (files.length === 0) {
files = workingFiles ?? [];
} else {
for (const wf of workingFiles) {
const index = files.findIndex(f => f.path === wf.path);
if (index !== -1) {
Expand All @@ -274,14 +273,12 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {
files.push(wf);
}
}
} else {
files = workingFiles;
}
}
}

return {
label: `${pluralize('file', files?.length ?? 0, { zero: 'No' })} changed`,
label: `${pluralize('file', files.length, { zero: 'No' })} changed`,
files: files,
};
}
Expand Down Expand Up @@ -312,17 +309,18 @@ export class CompareResultsNode extends ViewNode<SearchAndCompareView> {
private async getFilesQuery(): Promise<FilesQueryResults> {
let comparison;
if (this._compareWith.ref === '') {
comparison = this._ref.ref;
debugger;
throw new Error('Cannot get files for comparisons of a ref with working tree');
} else if (this._ref.ref === '') {
comparison = this._compareWith.ref;
} else {
comparison = `${this._compareWith.ref}..${this._ref.ref}`;
}

const files = await this.view.container.git.getDiffStatus(this.uri.repoPath!, comparison);
const files = (await this.view.container.git.getDiffStatus(this.repoPath, comparison)) ?? [];

return {
label: `${pluralize('file', files?.length ?? 0, { zero: 'No' })} changed`,
label: `${pluralize('file', files.length, { zero: 'No' })} changed`,
files: files,
};
}
Expand Down

0 comments on commit 17457b5

Please sign in to comment.