Skip to content

Commit

Permalink
Fixes issues w/ range notation & branches w/ special characters
Browse files Browse the repository at this point in the history
  • Loading branch information
eamodio committed May 25, 2020
1 parent ff9d13f commit 33905e9
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/commands/git/cherry-pick.ts
@@ -1,7 +1,7 @@
'use strict';
/* eslint-disable no-loop-func */
import { Container } from '../../container';
import { GitReference, Repository } from '../../git/gitService';
import { GitReference, GitRevision, Repository } from '../../git/gitService';
import { GlyphChars } from '../../constants';
import { Iterables, Strings } from '../../system';
import {
Expand Down Expand Up @@ -157,7 +157,7 @@ export class CherryPickGitCommand extends QuickCommandBase<State> {

if (selectedBranchOrTag !== undefined && state.counter < 3) {
const log = await Container.git.getLog(state.repo.path, {
ref: `${destination.ref}..${selectedBranchOrTag.ref}`,
ref: GitRevision.createRange(destination.ref, selectedBranchOrTag.ref),
merges: false
});

Expand Down
4 changes: 2 additions & 2 deletions src/commands/git/merge.ts
@@ -1,6 +1,6 @@
'use strict';
import { Container } from '../../container';
import { GitReference, Repository } from '../../git/gitService';
import { GitReference, GitRevision, Repository } from '../../git/gitService';
import { GlyphChars } from '../../constants';
import {
getBranchesAndOrTags,
Expand Down Expand Up @@ -145,7 +145,7 @@ export class MergeGitCommand extends QuickCommandBase<State> {

const count =
(await Container.git.getCommitCount(state.repo.path, [
`${destination.name}..${state.reference.name}`
GitRevision.createRange(destination.name, state.reference.name)
])) || 0;
if (count === 0) {
const step = this.createConfirmStep(
Expand Down
4 changes: 2 additions & 2 deletions src/commands/git/rebase.ts
@@ -1,6 +1,6 @@
'use strict';
import { Container } from '../../container';
import { GitReference, Repository } from '../../git/gitService';
import { GitReference, GitRevision, Repository } from '../../git/gitService';
import { GlyphChars } from '../../constants';
import {
getBranchesAndOrTags,
Expand Down Expand Up @@ -216,7 +216,7 @@ export class RebaseGitCommand extends QuickCommandBase<State> {

const count =
(await Container.git.getCommitCount(state.repo.path, [
`${state.reference.ref}..${destination.ref}`
GitRevision.createRange(state.reference.ref, destination.ref)
])) || 0;
if (count === 0) {
const step = this.createConfirmStep(
Expand Down
19 changes: 10 additions & 9 deletions src/git/git.ts
Expand Up @@ -6,11 +6,12 @@ import * as iconv from 'iconv-lite';
import { GlyphChars } from '../constants';
import { Container } from '../container';
import { Logger } from '../logger';
import { Objects, Strings } from '../system';
import { Iterables, Objects, Strings } from '../system';
import { findGitPath, GitLocation } from './locator';
import { run, RunOptions } from './shell';
import { GitBranchParser, GitLogParser, GitReflogParser, GitStashParser, GitTagParser } from './parsers/parsers';
import { GitFileStatus } from './models/file';
import { GitRevision } from './models/models';

export * from './models/models';
export * from './parsers/parsers';
Expand Down Expand Up @@ -617,22 +618,22 @@ export namespace Git {
params.push(`--diff-filter=${filter}`);
}
if (ref1) {
params.push(ref1);
params.push(...GitRevision.toParams(ref1));
}
if (ref2) {
params.push(ref2);
params.push(...GitRevision.toParams(ref2));
}

return git<string>({ cwd: repoPath, configs: ['-c', 'color.diff=false'] }, ...params);
return git<string>({ cwd: repoPath, configs: ['-c', 'color.diff=false'] }, ...params, '--');
}

export function diff__shortstat(repoPath: string, ref?: string) {
const params = ['diff', '--shortstat', '--no-ext-diff'];
if (ref) {
params.push(ref);
params.push(...GitRevision.toParams(ref));
}

return git<string>({ cwd: repoPath, configs: ['-c', 'color.diff=false'] }, ...params);
return git<string>({ cwd: repoPath, configs: ['-c', 'color.diff=false'] }, ...params, '--');
}

export function difftool(
Expand Down Expand Up @@ -729,7 +730,7 @@ export namespace Git {
if (reverse) {
params.push('--reverse', '--ancestry-path', `${ref}..HEAD`);
} else {
params.push(ref);
params.push(...GitRevision.toParams(ref));
}
}

Expand Down Expand Up @@ -973,9 +974,9 @@ export namespace Git {
if (options.count) {
params.push('--count');
}
params.push(...refs);
params.push(...Iterables.flatMap(refs, r => GitRevision.toParams(r)));

const data = await git<string>({ cwd: repoPath, errors: GitErrorHandling.Ignore }, 'rev-list', ...params);
const data = await git<string>({ cwd: repoPath, errors: GitErrorHandling.Ignore }, 'rev-list', ...params, '--');
return data.length === 0 ? undefined : Number(data.trim()) || undefined;
}

Expand Down
32 changes: 31 additions & 1 deletion src/git/models/models.ts
@@ -1,7 +1,37 @@
'use strict';

import { Git } from '../git';

const revisionRangeRegex = /([^.]*)(\.\.\.?)([^.]*)/;

export namespace GitRevision {
export function createRange(
ref1: string | undefined,
ref2: string | undefined,
notation: '..' | '...' = '..'
): string {
return `${ref1 || ''}${notation}${ref2 || ''}`;
}

export function toParams(ref: string | undefined) {
if (ref == null || ref.length === 0) return [];

const match = revisionRangeRegex.exec(ref);
if (match == null) return [ref];

const [, ref1, notation, ref2] = match;

const range = [];
if (ref1) {
range.push(ref1);
}
range.push(notation);
if (ref2) {
range.push(ref2);
}
return range;
}
}

export interface GitReference {
readonly refType: 'branch' | 'tag' | 'revision';
name: string;
Expand Down
5 changes: 3 additions & 2 deletions src/quickpicks/repoStatusQuickPick.ts
Expand Up @@ -16,6 +16,7 @@ import {
GitCommitType,
GitFileStatus,
GitLogCommit,
GitRevision,
GitService,
GitStatus,
GitStatusFile,
Expand Down Expand Up @@ -363,7 +364,7 @@ export class RepoStatusQuickPick {
},
Commands.ShowQuickBranchHistory,
[
GitUri.fromRepoPath(status.repoPath, `${status.upstream}..${status.ref}`),
GitUri.fromRepoPath(status.repoPath, GitRevision.createRange(status.upstream, status.ref)),
branchHistoryCommandArgs
]
)
Expand Down Expand Up @@ -394,7 +395,7 @@ export class RepoStatusQuickPick {
},
Commands.ShowQuickBranchHistory,
[
GitUri.fromRepoPath(status.repoPath, `${status.ref}..${status.upstream}`),
GitUri.fromRepoPath(status.repoPath, GitRevision.createRange(status.ref, status.upstream)),
branchHistoryCommandArgs
]
)
Expand Down
6 changes: 3 additions & 3 deletions src/views/nodes/branchTrackingStatusNode.ts
@@ -1,7 +1,7 @@
'use strict';
import { TreeItem, TreeItemCollapsibleState } from 'vscode';
import { Container } from '../../container';
import { GitBranch, GitLog, GitTrackingState, GitUri } from '../../git/gitService';
import { GitBranch, GitLog, GitRevision, GitTrackingState, GitUri } from '../../git/gitService';
import { debug, gate, Iterables, Strings } from '../../system';
import { ViewWithFiles } from '../viewBase';
import { CommitNode } from './commitNode';
Expand Down Expand Up @@ -134,8 +134,8 @@ export class BranchTrackingStatusNode extends ViewNode<ViewWithFiles> implements
private async getLog() {
if (this._log === undefined) {
const range = this.ahead
? `${this.status.upstream}..${this.status.ref}`
: `${this.status.ref}..${this.status.upstream}`;
? GitRevision.createRange(this.status.upstream, this.status.ref)
: GitRevision.createRange(this.status.ref, this.status.upstream);

this._log = await Container.git.getLog(this.uri.repoPath!, {
limit: this.limit ?? this.view.config.defaultItemLimit,
Expand Down
43 changes: 23 additions & 20 deletions src/views/nodes/compareBranchNode.ts
Expand Up @@ -3,11 +3,11 @@ import { TreeItem, TreeItemCollapsibleState } from 'vscode';
import { BranchComparison, BranchComparisons, GlyphChars, WorkspaceState } from '../../constants';
import { ResourceType, ViewNode } from './viewNode';
import { RepositoriesView } from '../repositoriesView';
import { GitBranch, GitService, GitUri } from '../../git/gitService';
import { GitBranch, GitRevision, GitService, GitUri } from '../../git/gitService';
import { CommandQuickPickItem, ReferencesQuickPick } from '../../quickpicks';
import { CommitsQueryResults, ResultsCommitsNode } from './resultsCommitsNode';
import { Container } from '../../container';
import { log, Mutable, Strings } from '../../system';
import { debug, gate, log, Mutable, Strings } from '../../system';
import { FilesQueryResults, ResultsFilesNode } from './resultsFilesNode';
import { ViewShowBranchComparison } from '../../config';
import { RepositoryNode } from './repositoryNode';
Expand All @@ -25,7 +25,7 @@ export class CompareBranchNode extends ViewNode<RepositoriesView> {
super(uri, view, parent);

const comparisons = Container.context.workspaceState.get<BranchComparisons>(WorkspaceState.BranchComparisons);
const compareWith = comparisons && comparisons[branch.id];
const compareWith = comparisons?.[branch.id];
if (compareWith !== undefined && typeof compareWith === 'string') {
this._compareWith = {
ref: compareWith,
Expand Down Expand Up @@ -145,26 +145,19 @@ export class CompareBranchNode extends ViewNode<RepositoriesView> {
this.view.triggerNodeChange(this);
}

private get comparisonNotation() {
return (
(this._compareWith && this._compareWith.notation) ||
(Container.config.advanced.useSymmetricDifferenceNotation ? '...' : '..')
);
private get comparisonNotation(): '..' | '...' {
return this._compareWith?.notation ?? (Container.config.advanced.useSymmetricDifferenceNotation ? '...' : '..');
}

private get diffComparisonNotation() {
private get diffComparisonNotation(): '..' | '...' {
// In git diff the range syntax doesn't mean the same thing as with git log -- since git diff is about comparing endpoints not ranges
// see https://git-scm.com/docs/git-diff#Documentation/git-diff.txt-emgitdiffemltoptionsgtltcommitgtltcommitgt--ltpathgt82308203
// So inverting the range syntax should be about equivalent for the behavior we want
return this.comparisonNotation === '...' ? '..' : '...';
}

private get comparisonType() {
return (
(this._compareWith && this._compareWith.type) ||
this.view.config.showBranchComparison ||
ViewShowBranchComparison.Working
);
return this._compareWith?.type ?? (this.view.config.showBranchComparison || ViewShowBranchComparison.Working);
}

private get compareWithWorkingTree() {
Expand All @@ -191,9 +184,11 @@ export class CompareBranchNode extends ViewNode<RepositoriesView> {
private async getCommitsQuery(limit: number | undefined): Promise<CommitsQueryResults> {
const log = await Container.git.getLog(this.uri.repoPath!, {
limit: limit,
ref: `${(this._compareWith && this._compareWith.ref) || 'HEAD'}${this.comparisonNotation}${
this.compareWithWorkingTree ? '' : this.branch.ref
}`
ref: GitRevision.createRange(
this._compareWith?.ref || 'HEAD',
this.compareWithWorkingTree ? '' : this.branch.ref,
this.comparisonNotation
)
});

const count = log?.count ?? 0;
Expand Down Expand Up @@ -221,12 +216,20 @@ export class CompareBranchNode extends ViewNode<RepositoriesView> {
return results as CommitsQueryResults;
}

@gate()
@debug()
refresh() {
this._children = undefined;
}

private async getFilesQuery(): Promise<FilesQueryResults> {
const diff = await Container.git.getDiffStatus(
this.uri.repoPath!,
`${(this._compareWith && this._compareWith.ref) || 'HEAD'}${this.diffComparisonNotation}${
this.compareWithWorkingTree ? '' : this.branch.ref
}`
GitRevision.createRange(
this._compareWith?.ref || 'HEAD',
this.compareWithWorkingTree ? '' : this.branch.ref,
this.diffComparisonNotation
)
);

return {
Expand Down
3 changes: 2 additions & 1 deletion src/views/nodes/repositoryNode.ts
Expand Up @@ -4,6 +4,7 @@ import { GlyphChars } from '../../constants';
import { Container } from '../../container';
import {
GitBranch,
GitRevision,
GitStatus,
GitUri,
Repository,
Expand Down Expand Up @@ -81,7 +82,7 @@ export class RepositoryNode extends SubscribeableViewNode<RepositoriesView> {
}

if (status.state.ahead || (status.files.length !== 0 && this.includeWorkingTree)) {
const range = status.upstream ? `${status.upstream}..${branch.ref}` : undefined;
const range = status.upstream ? GitRevision.createRange(status.upstream, branch.ref) : undefined;
children.push(new StatusFilesNode(this.view, this, status, range));
}

Expand Down

0 comments on commit 33905e9

Please sign in to comment.