Skip to content

Commit 643e226

Browse files
eamodioeamodio
committed
Overhauls previous line diffing - closes #719
Changes & Details hovers now know about staged vs unstaged changes Significantly increased accuracy with the following: - Open Line Changes with Previous Revision - Open Changes with Previous Revision - Changes & Details hovers
1 parent 26cbee6 commit 643e226

File tree

11 files changed

+356
-200
lines changed

11 files changed

+356
-200
lines changed

‎src/annotations/annotations.ts‎

Lines changed: 114 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import {
1717
GitBlameCommit,
1818
GitCommit,
1919
GitDiffHunkLine,
20+
GitLogCommit,
21+
GitService,
2022
GitUri
2123
} from '../git/gitService';
2224
import { Objects, Strings } from '../system';
@@ -62,74 +64,121 @@ export class Annotations {
6264
commit: GitBlameCommit,
6365
uri: GitUri,
6466
editorLine: number
67+
): Promise<MarkdownString | undefined>;
68+
static async changesHoverMessage(
69+
commit: GitLogCommit,
70+
uri: GitUri,
71+
editorLine: number,
72+
hunkLine: GitDiffHunkLine
73+
): Promise<MarkdownString | undefined>;
74+
static async changesHoverMessage(
75+
commit: GitBlameCommit | GitLogCommit,
76+
uri: GitUri,
77+
editorLine: number,
78+
hunkLine?: GitDiffHunkLine
6579
): Promise<MarkdownString | undefined> {
66-
let ref;
67-
if (commit.isUncommitted) {
68-
if (uri.isUncommittedStaged) {
69-
ref = uri.sha;
80+
const documentRef = uri.sha;
81+
if (commit instanceof GitBlameCommit) {
82+
// TODO: Figure out how to optimize this
83+
let ref;
84+
if (commit.isUncommitted) {
85+
if (GitService.isUncommittedStaged(documentRef)) {
86+
ref = documentRef;
87+
}
88+
}
89+
else {
90+
ref = commit.sha;
7091
}
71-
}
72-
else {
73-
ref = commit.sha;
74-
}
7592

76-
const line = editorLine + 1;
77-
const commitLine = commit.lines.find(l => l.line === line) || commit.lines[0];
93+
const line = editorLine + 1;
94+
const commitLine = commit.lines.find(l => l.line === line) || commit.lines[0];
7895

79-
let originalFileName = commit.originalFileName;
80-
if (originalFileName === undefined) {
81-
if (uri.fsPath !== commit.uri.fsPath) {
82-
originalFileName = commit.fileName;
96+
let originalFileName = commit.originalFileName;
97+
if (originalFileName === undefined) {
98+
if (uri.fsPath !== commit.uri.fsPath) {
99+
originalFileName = commit.fileName;
100+
}
83101
}
84-
}
85102

86-
const commitEditorLine = commitLine.originalLine - 1;
87-
const hunkLine = await Container.git.getDiffForLine(uri, commitEditorLine, ref, undefined, originalFileName);
88-
return this.changesHoverDiffMessage(commit, uri, hunkLine, commitEditorLine);
89-
}
103+
editorLine = commitLine.originalLine - 1;
104+
hunkLine = await Container.git.getDiffForLine(uri, editorLine, ref, undefined, originalFileName);
105+
106+
// If we didn't find a diff & ref is undefined (meaning uncommitted), check for a staged diff
107+
if (hunkLine === undefined && ref === undefined) {
108+
hunkLine = await Container.git.getDiffForLine(
109+
uri,
110+
editorLine,
111+
undefined,
112+
GitService.uncommittedStagedSha,
113+
originalFileName
114+
);
115+
}
116+
}
90117

91-
static changesHoverDiffMessage(
92-
commit: GitCommit,
93-
uri: GitUri,
94-
hunkLine: GitDiffHunkLine | undefined,
95-
editorLine?: number
96-
): MarkdownString | undefined {
97118
if (hunkLine === undefined || commit.previousSha === undefined) return undefined;
98119

99120
const diff = this.getDiffFromHunkLine(hunkLine);
100121

101-
let message: string;
122+
let message;
123+
let previous;
124+
let current;
102125
if (commit.isUncommitted) {
103-
if (uri.isUncommittedStaged) {
104-
message = `[\`Changes\`](${DiffWithCommand.getMarkdownCommandArgs(
105-
commit,
106-
editorLine
107-
)} "Open Changes") &nbsp; ${GlyphChars.Dash} &nbsp; [\`${
108-
commit.previousShortSha
109-
}\`](${ShowQuickCommitDetailsCommand.getMarkdownCommandArgs(
110-
commit.previousSha!
111-
)} "Show Commit Details") ${GlyphChars.ArrowLeftRightLong} _${uri.shortSha}_\n${diff}`;
112-
}
113-
else {
114-
message = `[\`Changes\`](${DiffWithCommand.getMarkdownCommandArgs(
115-
commit,
116-
editorLine
117-
)} "Open Changes") &nbsp; ${GlyphChars.Dash} &nbsp; _uncommitted changes_\n${diff}`;
126+
const diffUris = await commit.getPreviousLineDiffUris(uri, editorLine, documentRef);
127+
if (diffUris === undefined || diffUris.previous === undefined) {
128+
return undefined;
118129
}
130+
131+
message = `[\`Changes\`](${DiffWithCommand.getMarkdownCommandArgs({
132+
lhs: {
133+
sha: diffUris.previous.sha || '',
134+
uri: diffUris.previous.documentUri()
135+
},
136+
rhs: {
137+
sha: diffUris.current.sha || '',
138+
uri: diffUris.current.documentUri()
139+
},
140+
repoPath: commit.repoPath,
141+
line: editorLine
142+
})} "Open Changes")`;
143+
144+
previous =
145+
diffUris.previous.sha === undefined || diffUris.previous.isUncommitted
146+
? `_${GitService.shortenSha(diffUris.previous.sha, {
147+
working: 'Working Tree'
148+
})}_`
149+
: `[\`${GitService.shortenSha(
150+
diffUris.previous.sha || ''
151+
)}\`](${ShowQuickCommitDetailsCommand.getMarkdownCommandArgs(
152+
diffUris.previous.sha || ''
153+
)} "Show Commit Details")`;
154+
155+
current =
156+
diffUris.current.sha === undefined || diffUris.current.isUncommitted
157+
? `_${GitService.shortenSha(diffUris.current.sha, {
158+
working: 'Working Tree'
159+
})}_`
160+
: `[\`${GitService.shortenSha(
161+
diffUris.current.sha || ''
162+
)}\`](${ShowQuickCommitDetailsCommand.getMarkdownCommandArgs(
163+
diffUris.current.sha || ''
164+
)} "Show Commit Details")`;
119165
}
120166
else {
121-
message = `[\`Changes\`](${DiffWithCommand.getMarkdownCommandArgs(
122-
commit,
123-
editorLine
124-
)} "Open Changes") &nbsp; ${GlyphChars.Dash} &nbsp; [\`${
125-
commit.previousShortSha
126-
}\`](${ShowQuickCommitDetailsCommand.getMarkdownCommandArgs(commit.previousSha!)} "Show Commit Details") ${
127-
GlyphChars.ArrowLeftRightLong
128-
} [\`${commit.shortSha}\`](${ShowQuickCommitDetailsCommand.getMarkdownCommandArgs(
167+
message = `[\`Changes\`](${DiffWithCommand.getMarkdownCommandArgs(commit, editorLine)} "Open Changes")`;
168+
169+
previous = `[\`${commit.previousShortSha}\`](${ShowQuickCommitDetailsCommand.getMarkdownCommandArgs(
170+
commit.previousSha!
171+
)} "Show Commit Details")`;
172+
173+
current = `[\`${commit.shortSha}\`](${ShowQuickCommitDetailsCommand.getMarkdownCommandArgs(
129174
commit.sha
130-
)} "Show Commit Details")\n${diff}`;
175+
)} "Show Commit Details")`;
131176
}
132177

178+
message += ` &nbsp; ${GlyphChars.Dash} &nbsp; ${previous} &nbsp;${
179+
GlyphChars.ArrowLeftRightLong
180+
}&nbsp; ${current}\n${diff}`;
181+
133182
const markdown = new MarkdownString(message);
134183
markdown.isTrusted = true;
135184
return markdown;
@@ -140,14 +189,15 @@ export class Annotations {
140189
uri: GitUri,
141190
editorLine: number,
142191
dateFormat: string | null,
143-
annotationType?: FileAnnotationType
192+
annotationType: FileAnnotationType | undefined
144193
): Promise<MarkdownString> {
145194
if (dateFormat === null) {
146195
dateFormat = 'MMMM Do, YYYY h:mma';
147196
}
148197

149-
const [presence, remotes] = await Promise.all([
198+
const [presence, previousLineDiffUris, remotes] = await Promise.all([
150199
Container.vsls.getContactPresence(commit.email),
200+
commit.isUncommitted ? commit.getPreviousLineDiffUris(uri, editorLine, uri.sha) : undefined,
151201
Container.git.getRemotes(commit.repoPath)
152202
]);
153203

@@ -158,6 +208,7 @@ export class Annotations {
158208
line: editorLine,
159209
markdown: true,
160210
presence: presence,
211+
previousLineDiffUris: previousLineDiffUris,
161212
remotes: remotes
162213
})
163214
);
@@ -274,13 +325,22 @@ export class Annotations {
274325

275326
static trailing(
276327
commit: GitCommit,
328+
// uri: GitUri,
329+
// editorLine: number,
277330
format: string,
278331
dateFormat: string | null,
279332
scrollable: boolean = true
280333
): Partial<DecorationOptions> {
334+
// TODO: Enable this once there is better caching
335+
// let diffUris;
336+
// if (commit.isUncommitted) {
337+
// diffUris = await commit.getPreviousLineDiffUris(uri, editorLine, uri.sha);
338+
// }
339+
281340
const message = CommitFormatter.fromTemplate(format, commit, {
282-
truncateMessageAtNewLine: true,
283-
dateFormat: dateFormat
341+
dateFormat: dateFormat,
342+
// previousLineDiffUris: diffUris,
343+
truncateMessageAtNewLine: true
284344
});
285345

286346
return {

‎src/annotations/lineAnnotationController.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,16 @@ export class LineAnnotationController implements Disposable {
185185

186186
const decoration = Annotations.trailing(
187187
state.commit,
188+
// await GitUri.fromUri(editor.document.uri),
189+
// l,
188190
cfg.format,
189191
cfg.dateFormat === null ? Container.config.defaultDateFormat : cfg.dateFormat,
190192
cfg.scrollable
191193
) as DecorationOptions;
192194
decoration.range = editor.document.validateRange(
193195
new Range(l, Number.MAX_SAFE_INTEGER, l, Number.MAX_SAFE_INTEGER)
194196
);
197+
195198
decorations.push(decoration);
196199
}
197200

‎src/annotations/recentChangesAnnotationProvider.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export class RecentChangesAnnotationProvider extends AnnotationProviderBase {
7373
}
7474

7575
if (cfg.hovers.annotations.changes) {
76-
message = Annotations.changesHoverDiffMessage(commit, this._uri, hunkLine, count);
76+
message = await Annotations.changesHoverMessage(commit, this._uri, count, hunkLine);
7777
if (message === undefined) continue;
7878
}
7979
}

‎src/commands/diffLineWithPrevious.ts‎

Lines changed: 3 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -31,66 +31,12 @@ export class DiffLineWithPreviousCommand extends ActiveEditorCommand {
3131

3232
const gitUri = args.commit !== undefined ? GitUri.fromCommit(args.commit) : await GitUri.fromUri(uri);
3333

34-
if (gitUri.sha === undefined || gitUri.isUncommitted) {
35-
const blame =
36-
editor && editor.document.isDirty
37-
? await Container.git.getBlameForLineContents(gitUri, args.line, editor.document.getText())
38-
: await Container.git.getBlameForLine(gitUri, args.line);
39-
if (blame === undefined) {
40-
return Messages.showFileNotUnderSourceControlWarningMessage('Unable to open compare');
41-
}
42-
43-
// If the line is uncommitted, change the previous commit
44-
if (blame.commit.isUncommitted) {
45-
// Since there could be a change in the line number, update it
46-
args.line = blame.line.originalLine - 1;
47-
48-
try {
49-
const previous = await Container.git.getPreviousUri(
50-
gitUri.repoPath!,
51-
gitUri,
52-
gitUri.sha,
53-
0,
54-
args.line
55-
);
56-
57-
if (previous === undefined) {
58-
return Messages.showCommitHasNoPreviousCommitWarningMessage();
59-
}
60-
61-
const diffArgs: DiffWithCommandArgs = {
62-
repoPath: gitUri.repoPath!,
63-
lhs: {
64-
sha: previous.sha || '',
65-
uri: previous.documentUri()
66-
},
67-
rhs: {
68-
sha: gitUri.sha || '',
69-
uri: gitUri.documentUri()
70-
},
71-
line: args.line,
72-
showOptions: args.showOptions
73-
};
74-
return commands.executeCommand(Commands.DiffWith, diffArgs);
75-
}
76-
catch (ex) {
77-
Logger.error(
78-
ex,
79-
'DiffLineWithPreviousCommand',
80-
`getPreviousUri(${gitUri.repoPath}, ${gitUri.fsPath}, ${gitUri.sha})`
81-
);
82-
return Messages.showGenericErrorMessage('Unable to open compare');
83-
}
84-
}
85-
}
86-
8734
try {
88-
const diffUris = await Container.git.getPreviousDiffUris(
35+
const diffUris = await Container.git.getPreviousLineDiffUris(
8936
gitUri.repoPath!,
9037
gitUri,
91-
gitUri.sha,
92-
0,
93-
args.line
38+
args.line,
39+
gitUri.sha
9440
);
9541

9642
if (diffUris === undefined || diffUris.previous === undefined) {

‎src/commands/externalDiff.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export class ExternalDiffCommand extends Command {
7777
args.files = [
7878
{
7979
uri: GitUri.fromFile(context.node.file, context.node.file.repoPath || context.node.repoPath),
80-
staged: context.node.commit.isStagedUncommitted || context.node.file.indexStatus !== undefined,
80+
staged: context.node.commit.isUncommittedStaged || context.node.file.indexStatus !== undefined,
8181
ref1: ref1,
8282
ref2: ref2
8383
}

0 commit comments

Comments
 (0)