From 457ab389745e83913d973c2e587ad19a40fd7fd6 Mon Sep 17 00:00:00 2001 From: Eric Amodio Date: Fri, 17 Aug 2018 02:35:32 -0400 Subject: [PATCH] Fixes #468 - better handles detached head state --- CHANGELOG.md | 1 + src/commands/diffWithRevision.ts | 2 +- src/commands/openFileRevision.ts | 2 +- src/commands/showQuickFileHistory.ts | 2 +- src/git/git.ts | 26 ++++++++++++++++++++------ src/git/models/branch.ts | 30 ++++++++++++++++++++++-------- src/git/models/status.ts | 7 ++++++- src/git/models/tag.ts | 4 ++++ src/gitService.ts | 4 ++-- src/views/explorerCommands.ts | 2 +- src/views/nodes/branchNode.ts | 6 +++--- src/views/nodes/branchesNode.ts | 2 +- src/views/nodes/remoteNode.ts | 2 +- src/views/nodes/statusNode.ts | 10 ++++++++-- 14 files changed, 72 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 86add57..07bd3fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p ### Fixed - Fixes [#471](https://github.com/eamodio/vscode-gitlens/issues/471) - Don't use Ctrl+Alt+[character] as a shortcut - Fixes [#478](https://github.com/eamodio/vscode-gitlens/issues/478) - `suppressShowKeyBindingsNotice` gets saved even when it is not required +- Fixes [#468](https://github.com/eamodio/vscode-gitlens/issues/468) - Commit history for detached HEAD doesn't work properly ## [8.5.4] - 2018-07-31 ### Added diff --git a/src/commands/diffWithRevision.ts b/src/commands/diffWithRevision.ts index 504c95b..b8fe18c 100644 --- a/src/commands/diffWithRevision.ts +++ b/src/commands/diffWithRevision.ts @@ -45,7 +45,7 @@ export class DiffWithRevisionCommand extends ActiveEditorCommand { try { const log = await Container.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, { maxCount: args.maxCount, - ref: (args.branchOrTag && args.branchOrTag.name) || gitUri.sha + ref: (args.branchOrTag && args.branchOrTag.ref) || gitUri.sha }); if (log === undefined) { if (args.branchOrTag) { diff --git a/src/commands/openFileRevision.ts b/src/commands/openFileRevision.ts index 89495d8..f7b6347 100644 --- a/src/commands/openFileRevision.ts +++ b/src/commands/openFileRevision.ts @@ -75,7 +75,7 @@ export class OpenFileRevisionCommand extends ActiveEditorCommand { const log = await Container.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, { maxCount: args.maxCount, - ref: (args.branchOrTag && args.branchOrTag.name) || gitUri.sha + ref: (args.branchOrTag && args.branchOrTag.ref) || gitUri.sha }); if (log === undefined) { if (args.branchOrTag) { diff --git a/src/commands/showQuickFileHistory.ts b/src/commands/showQuickFileHistory.ts index f91c84b..ef2eb23 100644 --- a/src/commands/showQuickFileHistory.ts +++ b/src/commands/showQuickFileHistory.ts @@ -49,7 +49,7 @@ export class ShowQuickFileHistoryCommand extends ActiveEditorCachedCommand { args.log = await Container.git.getLogForFile(gitUri.repoPath, gitUri.fsPath, { maxCount: args.maxCount, range: args.range, - ref: (args.branchOrTag && args.branchOrTag.name) || gitUri.sha + ref: (args.branchOrTag && args.branchOrTag.ref) || gitUri.sha }); if (args.log === undefined) { if (args.branchOrTag) { diff --git a/src/git/git.ts b/src/git/git.ts index 7531d21..80d6e15 100644 --- a/src/git/git.ts +++ b/src/git/git.ts @@ -603,33 +603,47 @@ export class Git { } } - static async revparse_currentBranch(repoPath: string): Promise { + static async revparse_currentBranch(repoPath: string): Promise<[string, string?] | undefined> { const params = ['rev-parse', '--abbrev-ref', '--symbolic-full-name', '@', '@{u}']; const opts = { cwd: repoPath } as CommandOptions; try { const data = await gitCommandCore(opts, ...params); - return data; + return [data, undefined]; } catch (ex) { const msg = ex && ex.toString(); - if (GitWarnings.headNotABranch.test(msg)) return undefined; + if (GitWarnings.headNotABranch.test(msg)) { + try { + const params = ['log', '-n1', '--format=%H', '--']; + const data = await gitCommandCore(opts, ...params); + if (data === undefined) return undefined; + + // Matches output of `git branch -vv` + const sha = data.trim(); + return [`(HEAD detached at ${this.shortenSha(sha)})`, sha]; + } + catch { + return undefined; + } + } const result = GitWarnings.noUpstream.exec(msg); - if (result !== null) return result[1]; + if (result !== null) return [result[1], undefined]; if (GitWarnings.unknownRevision.test(msg)) { try { const params = ['symbolic-ref', '-q', '--short', 'HEAD']; const data = await gitCommandCore(opts, ...params); - return data; + return [data, undefined]; } catch { return undefined; } } - return gitCommandDefaultErrorHandler(ex, opts, ...params); + gitCommandDefaultErrorHandler(ex, opts, ...params); + return undefined; } } diff --git a/src/git/models/branch.ts b/src/git/models/branch.ts index 6d19a66..955c51c 100644 --- a/src/git/models/branch.ts +++ b/src/git/models/branch.ts @@ -1,8 +1,10 @@ +import { Git } from '../git'; import { GitStatus } from './status'; 'use strict'; export class GitBranch { + readonly detached: boolean; readonly name: string; readonly remote: boolean; readonly tracking?: string; @@ -18,7 +20,8 @@ export class GitBranch { public readonly sha?: string, tracking?: string, ahead: number = 0, - behind: number = 0 + behind: number = 0, + detached: boolean = false ) { if (branch.startsWith('remotes/')) { branch = branch.substring(8); @@ -28,7 +31,14 @@ export class GitBranch { this.remote = false; } - this.name = branch; + this.detached = detached || (this.current ? GitBranch.isDetached(branch) : false); + if (this.detached) { + this.name = GitBranch.formatDetached(this.sha!); + } + else { + this.name = branch; + } + this.tracking = tracking === '' || tracking == null ? undefined : tracking; this.state = { ahead: ahead, @@ -36,6 +46,10 @@ export class GitBranch { }; } + get ref() { + return this.detached ? this.sha! : this.name; + } + private _basename: string | undefined; getBasename(): string { if (this._basename === undefined) { @@ -73,17 +87,17 @@ export class GitBranch { return GitStatus.getUpstreamStatus(this.tracking, this.state, options); } - isValid(): boolean { - return GitBranch.isValid(this.name); - } - static getRemote(branch: string): string { return branch.substring(0, branch.indexOf('/')); } - static isValid(name: string): boolean { + static formatDetached(sha: string): string { + return `(${Git.shortenSha(sha)}...)`; + } + + static isDetached(name: string): boolean { // If there is whitespace in the name assume this is not a valid branch name // Deals with detached HEAD states - return name.match(/\s/) === null; + return name.match(/\s/) !== null || name.match(/\(detached\)/) !== null; } } diff --git a/src/git/models/status.ts b/src/git/models/status.ts index e403023..b5dfe49 100644 --- a/src/git/models/status.ts +++ b/src/git/models/status.ts @@ -4,6 +4,7 @@ import { Uri } from 'vscode'; import { GlyphChars } from '../../constants'; import { Strings } from '../../system'; import { GitUri } from '../gitUri'; +import { GitBranch } from './branch'; import { GitLogCommit } from './logCommit'; export interface GitStatusUpstreamState { @@ -19,7 +20,11 @@ export class GitStatus { public readonly files: GitStatusFile[], public readonly state: GitStatusUpstreamState, public readonly upstream?: string - ) {} + ) { + if (GitBranch.isDetached(branch)) { + this.branch = GitBranch.formatDetached(this.sha); + } + } private _diff?: { added: number; diff --git a/src/git/models/tag.ts b/src/git/models/tag.ts index f7aafcd..26e7d9a 100644 --- a/src/git/models/tag.ts +++ b/src/git/models/tag.ts @@ -7,6 +7,10 @@ export class GitTag { public readonly annotation?: string ) {} + get ref() { + return this.name; + } + private _basename: string | undefined; getBasename(): string { if (this._basename === undefined) { diff --git a/src/gitService.ts b/src/gitService.ts index b344313..f525182 100644 --- a/src/gitService.ts +++ b/src/gitService.ts @@ -891,8 +891,8 @@ export class GitService implements Disposable { const data = await Git.revparse_currentBranch(repoPath); if (data === undefined) return undefined; - const branch = data.split('\n'); - return new GitBranch(repoPath, branch[0], true, undefined, branch[1]); + const branch = data[0].split('\n'); + return new GitBranch(repoPath, branch[0], true, data[1], branch[1]); } async getBranches(repoPath: string | undefined): Promise { diff --git a/src/views/explorerCommands.ts b/src/views/explorerCommands.ts index 966add0..f097f34 100644 --- a/src/views/explorerCommands.ts +++ b/src/views/explorerCommands.ts @@ -134,7 +134,7 @@ export class ExplorerCommands implements Disposable { const branch = await Container.git.getBranch(node.repoPath); if (branch === undefined) return; - const commonAncestor = await Container.git.getMergeBase(node.repoPath, branch.name, node.ref); + const commonAncestor = await Container.git.getMergeBase(node.repoPath, branch.ref, node.ref); if (commonAncestor === undefined) return; Container.resultsExplorer.showComparisonInResults( diff --git a/src/views/nodes/branchNode.ts b/src/views/nodes/branchNode.ts index 7fef0cd..134f54e 100644 --- a/src/views/nodes/branchNode.ts +++ b/src/views/nodes/branchNode.ts @@ -28,7 +28,7 @@ export class BranchNode extends ExplorerRefNode { const branchName = this.branch.getName(); if (this.explorer.config.branches.layout === ExplorerBranchesLayout.List) return branchName; - return GitBranch.isValid(branchName) && !this.current ? this.branch.getBasename() : branchName; + return this.current || GitBranch.isDetached(branchName) ? branchName : this.branch.getBasename(); } get markCurrent(): boolean { @@ -36,11 +36,11 @@ export class BranchNode extends ExplorerRefNode { } get ref(): string { - return this.branch.name; + return this.branch.ref; } async getChildren(): Promise { - const log = await Container.git.getLog(this.uri.repoPath!, { maxCount: this.maxCount, ref: this.branch.name }); + const log = await Container.git.getLog(this.uri.repoPath!, { maxCount: this.maxCount, ref: this.ref }); if (log === undefined) return [new MessageNode('No commits yet')]; const branches = await Container.git.getBranches(this.uri.repoPath); diff --git a/src/views/nodes/branchesNode.ts b/src/views/nodes/branchesNode.ts index 1dc04a4..caa8f31 100644 --- a/src/views/nodes/branchesNode.ts +++ b/src/views/nodes/branchesNode.ts @@ -40,7 +40,7 @@ export class BranchesNode extends ExplorerNode { const hierarchy = Arrays.makeHierarchical( branchNodes, - n => (n.branch.isValid() ? n.branch.getName().split('/') : [n.branch.name]), + n => (n.branch.detached ? [n.branch.name] : n.branch.getName().split('/')), (...paths: string[]) => paths.join('/'), this.explorer.config.files.compact ); diff --git a/src/views/nodes/remoteNode.ts b/src/views/nodes/remoteNode.ts index aea4eee..fc37232 100644 --- a/src/views/nodes/remoteNode.ts +++ b/src/views/nodes/remoteNode.ts @@ -40,7 +40,7 @@ export class RemoteNode extends ExplorerNode { const hierarchy = Arrays.makeHierarchical( branchNodes, - n => (n.branch.isValid() ? n.branch.getName().split('/') : [n.branch.name]), + n => (n.branch.detached ? [n.branch.name] : n.branch.getName().split('/')), (...paths: string[]) => paths.join('/'), this.explorer.config.files.compact ); diff --git a/src/views/nodes/statusNode.ts b/src/views/nodes/statusNode.ts index 3f578d4..1cdfe97 100644 --- a/src/views/nodes/statusNode.ts +++ b/src/views/nodes/statusNode.ts @@ -53,7 +53,8 @@ export class StatusNode extends ExplorerNode { branch.sha, branch.tracking, status.state.ahead, - status.state.behind + status.state.behind, + branch.detached ); } @@ -168,7 +169,12 @@ export class StatusBranchNode extends BranchNode { async getTreeItem(): Promise { const item = await super.getTreeItem(); - item.label = `History (${item.label})`; + if (item.label!.startsWith('(') && item.label!.endsWith(')')) { + item.label = `History ${item.label}`; + } + else { + item.label = `History (${item.label})`; + } return item; } }