Fixes #468 - better handles detached head state

eamodio · eamodio · commit 457ab389745e · 2018-08-18T01:01:58.000-04:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -603,33 +603,47 @@ export class Git {
         }
     }
 
-    static async revparse_currentBranch(repoPath: string): Promise<string | undefined> {
+    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
@@ -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,14 +31,25 @@ 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,
             behind: behind
         };
     }
 
+    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
@@ -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
@@ -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
@@ -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<GitBranch[]> {
diff --git 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
@@ -28,19 +28,19 @@ 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 {
         return true;
     }
 
     get ref(): string {
-        return this.branch.name;
+        return this.branch.ref;
     }
 
     async getChildren(): Promise<ExplorerNode[]> {
-        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
@@ -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
@@ -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
@@ -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<TreeItem> {
         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;
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -891,8 +891,8 @@ export class GitService implements Disposable {`
`891`	`891`	`const data = await Git.revparse_currentBranch(repoPath);`
`892`	`892`	`if (data === undefined) return undefined;`
`893`	`893`
`894`		`- const branch = data.split('\n');`
`895`		`- return new GitBranch(repoPath, branch[0], true, undefined, branch[1]);`
	`894`	`+ const branch = data[0].split('\n');`
	`895`	`+ return new GitBranch(repoPath, branch[0], true, data[1], branch[1]);`
`896`	`896`	`}`
`897`	`897`
`898`	`898`	`async getBranches(repoPath: string \| undefined): Promise<GitBranch[]> {`