Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit 7e0195f

Browse files
jcansdalejcansdale
authored
Merge pull request #1036 from github/fixes/817-pr-details-context-menu
Added extra items to file menu in PR details view.
2 parents bfc4839 + 6a87ad1 commit 7e0195f

File tree

10 files changed

+273
-185
lines changed

10 files changed

+273
-185
lines changed

‎src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.Diagnostics.CodeAnalysis;
44
using System.Reactive;
5+
using System.Text;
56
using System.Threading.Tasks;
67
using GitHub.Models;
78
using GitHub.Services;
@@ -93,14 +94,18 @@ public PullRequestDetailViewModelDesigner()
9394
public ReactiveCommand<Unit> Pull { get; }
9495
public ReactiveCommand<Unit> Push { get; }
9596
public ReactiveCommand<object> OpenOnGitHub { get; }
96-
public ReactiveCommand<object> OpenFile { get; }
9797
public ReactiveCommand<object> DiffFile { get; }
98+
public ReactiveCommand<object> DiffFileWithWorkingDirectory { get; }
99+
public ReactiveCommand<object> OpenFileInWorkingDirectory { get; }
100+
public ReactiveCommand<object> ViewFile { get; }
98101

99-
public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
102+
public Task<string> ExtractFile(IPullRequestFileNode file, bool head, Encoding encoding)
100103
{
101104
return null;
102105
}
103106

107+
public Encoding GetEncoding(string path) => Encoding.UTF8;
108+
104109
public string GetLocalFilePath(IPullRequestFileNode file)
105110
{
106111
return null;

‎src/GitHub.App/Services/PullRequestService.cs‎

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
using System.Reactive;
1515
using System.Collections.Generic;
1616
using LibGit2Sharp;
17-
using PullRequest = Octokit.PullRequest;
1817
using System.Diagnostics;
1918

2019
namespace GitHub.Services
@@ -290,51 +289,51 @@ public IObservable<Tuple<string, int>> GetPullRequestForCurrentBranch(ILocalRepo
290289
});
291290
}
292291

293-
public IObservable<Tuple<string, string>> ExtractDiffFiles(
292+
public IObservable<string> ExtractFile(
294293
ILocalRepositoryModel repository,
295294
IPullRequestModel pullRequest,
296295
string fileName,
297-
bool isPullRequestBranchCheckedOut)
296+
bool head,
297+
Encoding encoding)
298298
{
299299
return Observable.Defer(async () =>
300300
{
301301
var repo = gitService.GetRepository(repository.LocalPath);
302-
var baseUrl = pullRequest.Base.RepositoryCloneUrl;
303-
var headUrl = pullRequest.Head.RepositoryCloneUrl;
304-
var baseSha = pullRequest.Base.Sha;
305-
var headSha = pullRequest.Head.Sha;
306-
var baseRef = pullRequest.Base.Ref;
307-
var headRef = pullRequest.Head.Ref;
308-
string mergeBase = await gitClient.GetPullRequestMergeBase(
309-
repo, baseUrl, headUrl, baseSha, headSha, baseRef, headRef);
310-
if (mergeBase == null)
311-
{
312-
throw new FileNotFoundException($"Couldn't find merge base between {baseSha} and {headSha}.");
313-
}
302+
var remote = await gitClient.GetHttpRemote(repo, "origin");
303+
string sha;
314304

315-
string left;
316-
string right;
317-
if (isPullRequestBranchCheckedOut)
305+
if (head)
318306
{
319-
right = Path.Combine(repository.LocalPath, fileName);
320-
left = await ExtractToTempFile(repo, mergeBase, fileName, GetEncoding(right));
307+
sha = pullRequest.Head.Sha;
321308
}
322309
else
323310
{
324-
left = await ExtractToTempFile(repo, mergeBase, fileName, Encoding.UTF8);
325-
right = await ExtractToTempFile(repo, headSha, fileName, Encoding.UTF8);
311+
sha = await gitClient.GetPullRequestMergeBase(
312+
repo,
313+
pullRequest.Base.RepositoryCloneUrl,
314+
pullRequest.Head.RepositoryCloneUrl,
315+
pullRequest.Base.Sha,
316+
pullRequest.Head.Sha,
317+
pullRequest.Base.Ref,
318+
pullRequest.Head.Ref);
319+
320+
if (sha == null)
321+
{
322+
throw new NotFoundException($"Couldn't find merge base between {pullRequest.Base.Sha} and {pullRequest.Head.Sha}.");
323+
}
326324
}
327325

328-
return Observable.Return(Tuple.Create(left, right));
326+
var file = await ExtractToTempFile(repo, pullRequest.Number, sha, fileName, encoding);
327+
return Observable.Return(file);
329328
});
330329
}
331330

332-
static Encoding GetEncoding(string file)
331+
public Encoding GetEncoding(string path)
333332
{
334-
if (File.Exists(file))
333+
if (File.Exists(path))
335334
{
336335
var encoding = Encoding.UTF8;
337-
if (HasPreamble(file, encoding))
336+
if (HasPreamble(path, encoding))
338337
{
339338
return encoding;
340339
}
@@ -413,9 +412,27 @@ string CreateUniqueRemoteName(IRepository repo, string name)
413412
return uniqueName;
414413
}
415414

416-
async Task<string> ExtractToTempFile(IRepository repo, string commitSha, string fileName, Encoding encoding)
415+
async Task<string> ExtractToTempFile(
416+
IRepository repo,
417+
int pullRequestNumber,
418+
string commitSha,
419+
string fileName,
420+
Encoding encoding)
417421
{
418-
var contents = await gitClient.ExtractFile(repo, commitSha, fileName) ?? string.Empty;
422+
string contents;
423+
424+
try
425+
{
426+
contents = await gitClient.ExtractFile(repo, commitSha, fileName) ?? string.Empty;
427+
}
428+
catch (FileNotFoundException)
429+
{
430+
var pullHeadRef = $"refs/pull/{pullRequestNumber}/head";
431+
var remote = await gitClient.GetHttpRemote(repo, "origin");
432+
await gitClient.Fetch(repo, remote.Name, commitSha, pullHeadRef);
433+
contents = await gitClient.ExtractFile(repo, commitSha, fileName) ?? string.Empty;
434+
}
435+
419436
return CreateTempFile(fileName, commitSha, contents, encoding);
420437
}
421438

@@ -427,6 +444,7 @@ static string CreateTempFile(string fileName, string commitSha, string contents,
427444

428445
Directory.CreateDirectory(tempDir);
429446
File.WriteAllText(tempFile, contents, encoding);
447+
File.SetAttributes(tempFile, FileAttributes.ReadOnly);
430448
return tempFile;
431449
}
432450

‎src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Reactive;
77
using System.Reactive.Linq;
88
using System.Reactive.Threading.Tasks;
9+
using System.Text;
910
using System.Threading.Tasks;
1011
using GitHub.App;
1112
using GitHub.Exports;
@@ -112,8 +113,10 @@ public PullRequestDetailViewModel(
112113
SubscribeOperationError(Push);
113114

114115
OpenOnGitHub = ReactiveCommand.Create();
115-
OpenFile = ReactiveCommand.Create(this.WhenAnyValue(x => x.IsCheckedOut));
116116
DiffFile = ReactiveCommand.Create();
117+
DiffFileWithWorkingDirectory = ReactiveCommand.Create(this.WhenAnyValue(x => x.IsCheckedOut));
118+
OpenFileInWorkingDirectory = ReactiveCommand.Create(this.WhenAnyValue(x => x.IsCheckedOut));
119+
ViewFile = ReactiveCommand.Create();
117120
}
118121

119122
/// <summary>
@@ -293,14 +296,25 @@ public IReadOnlyList<IPullRequestChangeNode> ChangedFilesTree
293296
public ReactiveCommand<object> OpenOnGitHub { get; }
294297

295298
/// <summary>
296-
/// Gets a command that opens a <see cref="IPullRequestFileNode"/>.
299+
/// Gets a command that diffs an <see cref="IPullRequestFileNode"/> between BASE and HEAD.
297300
/// </summary>
298-
public ReactiveCommand<object> OpenFile { get; }
301+
public ReactiveCommand<object> DiffFile { get; }
299302

300303
/// <summary>
301-
/// Gets a command that diffs a <see cref="IPullRequestFileNode"/>.
304+
/// Gets a command that diffs an <see cref="IPullRequestFileNode"/> between the version in
305+
/// the working directory and HEAD.
302306
/// </summary>
303-
public ReactiveCommand<object> DiffFile { get; }
307+
public ReactiveCommand<object> DiffFileWithWorkingDirectory { get; }
308+
309+
/// <summary>
310+
/// Gets a command that opens an <see cref="IPullRequestFileNode"/> from disk.
311+
/// </summary>
312+
public ReactiveCommand<object> OpenFileInWorkingDirectory { get; }
313+
314+
/// <summary>
315+
/// Gets a command that opens an <see cref="IPullRequestFileNode"/> as it appears in the PR.
316+
/// </summary>
317+
public ReactiveCommand<object> ViewFile { get; }
304318

305319
/// <summary>
306320
/// Initializes the view model with new data.
@@ -455,16 +469,27 @@ public async Task Load(IRemoteRepositoryModel remoteRepository, IPullRequestMode
455469
}
456470

457471
/// <summary>
458-
/// Gets the before and after files needed for viewing a diff.
472+
/// Gets a file as it appears in the pull request.
459473
/// </summary>
460474
/// <param name="file">The changed file.</param>
461-
/// <returns>A tuple containing the full path to the before and after files.</returns>
462-
public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
475+
/// <param name="head">
476+
/// If true, gets the file at the PR head, otherwise gets the file at the PR merge base.
477+
/// </param>
478+
/// <param name="encoding">The encoding to use.</param>
479+
/// <returns>The path to a temporary file.</returns>
480+
public Task<string> ExtractFile(IPullRequestFileNode file, bool head, Encoding encoding)
463481
{
464482
var path = Path.Combine(file.DirectoryPath, file.FileName);
465-
return pullRequestsService.ExtractDiffFiles(LocalRepository, model, path, IsCheckedOut).ToTask();
483+
return pullRequestsService.ExtractFile(LocalRepository, model, path, head, encoding).ToTask();
466484
}
467485

486+
/// <summary>
487+
/// Gets the encoding for the specified file.
488+
/// </summary>
489+
/// <param name="path">The path to the file.</param>
490+
/// <returns>The file's encoding</returns>
491+
public Encoding GetEncoding(string path) => pullRequestsService.GetEncoding(path);
492+
468493
/// <summary>
469494
/// Gets the full path to a file in the working directory.
470495
/// </summary>

‎src/GitHub.Exports.Reactive/Services/IPullRequestService.cs‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Reactive;
3+
using System.Text;
34
using GitHub.Models;
45
using LibGit2Sharp;
56
using Octokit;
@@ -121,23 +122,27 @@ IObservable<IPullRequestModel> CreatePullRequest(IRepositoryHost host,
121122
IObservable<Tuple<string, int>> GetPullRequestForCurrentBranch(ILocalRepositoryModel repository);
122123

123124
/// <summary>
124-
/// Gets the left and right files for a diff.
125+
/// Gets the encoding for the specified file.
126+
/// </summary>
127+
/// <param name="path">The path to the file.</param>
128+
/// <returns>The file's encoding</returns>
129+
Encoding GetEncoding(string path);
130+
131+
/// <summary>
132+
/// Gets a file as it appears in a pull request.
125133
/// </summary>
126134
/// <param name="repository">The repository.</param>
127-
/// <param name="modelService">A model service to use as a cache if the file is not fetched.</param>
128135
/// <param name="pullRequest">The pull request details.</param>
129136
/// <param name="fileName">The filename relative to the repository root.</param>
130-
/// <param name="fileSha">The SHA of the file in the pull request.</param>
131-
/// <param name="isPullRequestBranchCheckedOut">
132-
/// Whether the pull request branch is currently checked out. If so the right file returned
133-
/// will be the path to the file in the working directory.
134-
/// </param>
137+
/// <param name="head">If true, gets the file at the PR head, otherwise gets the file at the PR base.</param>
138+
/// <param name="encoding">The encoding to use.</param>
135139
/// <returns>The paths of the left and right files for the diff.</returns>
136-
IObservable<Tuple<string, string>> ExtractDiffFiles(
140+
IObservable<string> ExtractFile(
137141
ILocalRepositoryModel repository,
138142
IPullRequestModel pullRequest,
139143
string fileName,
140-
bool isPullRequestBranchCheckedOut);
144+
bool head,
145+
Encoding encoding);
141146

142147
/// <summary>
143148
/// Remotes all unused remotes that were created by GitHub for Visual Studio to track PRs

‎src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs‎

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Reactive;
4+
using System.Text;
45
using System.Threading.Tasks;
56
using GitHub.Models;
67
using GitHub.Services;
@@ -161,21 +162,43 @@ public interface IPullRequestDetailViewModel : IViewModel, IHasLoading, IHasBusy
161162
ReactiveCommand<object> OpenOnGitHub { get; }
162163

163164
/// <summary>
164-
/// Gets a command that opens a <see cref="IPullRequestFileNode"/>.
165+
/// Gets a command that diffs an <see cref="IPullRequestFileNode"/> between BASE and HEAD.
165166
/// </summary>
166-
ReactiveCommand<object> OpenFile { get; }
167+
ReactiveCommand<object> DiffFile { get; }
167168

168169
/// <summary>
169-
/// Gets a command that diffs a <see cref="IPullRequestFileNode"/>.
170+
/// Gets a command that diffs an <see cref="IPullRequestFileNode"/> between the version in
171+
/// the working directory and HEAD.
170172
/// </summary>
171-
ReactiveCommand<object> DiffFile { get; }
173+
ReactiveCommand<object> DiffFileWithWorkingDirectory { get; }
174+
175+
/// <summary>
176+
/// Gets a command that opens an <see cref="IPullRequestFileNode"/> from disk.
177+
/// </summary>
178+
ReactiveCommand<object> OpenFileInWorkingDirectory { get; }
172179

173180
/// <summary>
174-
/// Gets the before and after files needed for viewing a diff.
181+
/// Gets a command that opens an <see cref="IPullRequestFileNode"/> as it appears in the PR.
182+
/// </summary>
183+
ReactiveCommand<object> ViewFile { get; }
184+
185+
/// <summary>
186+
/// Gets a file as it appears in the pull request.
175187
/// </summary>
176188
/// <param name="file">The changed file.</param>
177-
/// <returns>A tuple containing the full path to the before and after files.</returns>
178-
Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file);
189+
/// <param name="head">
190+
/// If true, gets the file at the PR head, otherwise gets the file at the PR merge base.
191+
/// </param>
192+
/// <param name="encoding">The encoding to use.</param>
193+
/// <returns>The path to a temporary file.</returns>
194+
Task<string> ExtractFile(IPullRequestFileNode file, bool head, Encoding encoding);
195+
196+
/// <summary>
197+
/// Gets the encoding for the specified file.
198+
/// </summary>
199+
/// <param name="path">The path to the file.</param>
200+
/// <returns>The file's encoding</returns>
201+
Encoding GetEncoding(string path);
179202

180203
/// <summary>
181204
/// Gets the full path to a file in the working directory.

0 commit comments

Comments
 (0)