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

Commit 1c4c1be

Browse files
grokysgrokys
committed
Merge branch 'master' into grokys/refactor-idialogview
 Conflicts: src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs
2 parents 373dea5 + dd0c5ea commit 1c4c1be

File tree

7 files changed

+71
-46
lines changed

7 files changed

+71
-46
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ public PullRequestDetailViewModelDesigner()
7171
public string SourceBranchDisplayName { get; set; }
7272
public string TargetBranchDisplayName { get; set; }
7373
public bool IsBusy { get; }
74+
public bool IsCheckedOut { get; }
7475
public bool IsFromFork { get; }
7576
public string Body { get; }
7677
public IReactiveList<IPullRequestChangeNode> ChangedFilesTree { get; }
@@ -85,12 +86,12 @@ public PullRequestDetailViewModelDesigner()
8586
public ReactiveCommand<object> OpenFile { get; }
8687
public ReactiveCommand<object> DiffFile { get; }
8788

88-
public Task<string> ExtractFile(IPullRequestFileNode file)
89+
public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
8990
{
9091
return null;
9192
}
9293

93-
public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
94+
public string GetLocalFilePath(IPullRequestFileNode file)
9495
{
9596
return null;
9697
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,8 @@ public IObservable<Tuple<string, string>> ExtractDiffFiles(
287287
IModelService modelService,
288288
IPullRequestModel pullRequest,
289289
string fileName,
290-
string fileSha)
290+
string fileSha,
291+
bool isPullRequestBranchCheckedOut)
291292
{
292293
return Observable.Defer(async () =>
293294
{
@@ -300,7 +301,9 @@ public IObservable<Tuple<string, string>> ExtractDiffFiles(
300301

301302
// The right file - if it comes from a fork - may not be fetched so fall back to
302303
// getting the file contents from the model service.
303-
var right = await GetFileFromRepositoryOrApi(repository, repo, modelService, pullRequest.Head.Sha, fileName, fileSha);
304+
var right = isPullRequestBranchCheckedOut ?
305+
Path.Combine(repository.LocalPath, fileName) :
306+
await GetFileFromRepositoryOrApi(repository, repo, modelService, pullRequest.Head.Sha, fileName, fileSha);
304307

305308
if (left == null)
306309
{

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

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ public class PullRequestDetailViewModel : PanePageViewModelBase, IPullRequestDet
3939
IPullRequestUpdateState updateState;
4040
string operationError;
4141
bool isBusy;
42+
bool isCheckedOut;
4243
bool isFromFork;
4344
bool isInCheckout;
4445

@@ -103,7 +104,7 @@ public PullRequestDetailViewModel(
103104
SubscribeOperationError(Push);
104105

105106
OpenOnGitHub = ReactiveCommand.Create();
106-
OpenFile = ReactiveCommand.Create();
107+
OpenFile = ReactiveCommand.Create(this.WhenAnyValue(x => x.IsCheckedOut));
107108
DiffFile = ReactiveCommand.Create();
108109
}
109110

@@ -154,6 +155,14 @@ public bool IsBusy
154155
private set { this.RaiseAndSetIfChanged(ref isBusy, value); }
155156
}
156157

158+
/// Gets a value indicating whether the pull request branch is checked out.
159+
/// </summary>
160+
public bool IsCheckedOut
161+
{
162+
get { return isCheckedOut; }
163+
private set { this.RaiseAndSetIfChanged(ref isCheckedOut, value); }
164+
}
165+
157166
/// <summary>
158167
/// Gets a value indicating whether the pull request comes from a fork.
159168
/// </summary>
@@ -278,9 +287,10 @@ public async Task Load(IPullRequestModel pullRequest)
278287
}
279288

280289
var localBranches = await pullRequestsService.GetLocalBranches(repository, pullRequest).ToList();
281-
var isCheckedOut = localBranches.Contains(repository.CurrentBranch);
282290

283-
if (isCheckedOut)
291+
IsCheckedOut = localBranches.Contains(repository.CurrentBranch);
292+
293+
if (IsCheckedOut)
284294
{
285295
var divergence = await pullRequestsService.CalculateHistoryDivergence(repository, Model.Number);
286296
var pullEnabled = divergence.BehindBy > 0;
@@ -349,25 +359,24 @@ public async Task Load(IPullRequestModel pullRequest)
349359
}
350360

351361
/// <summary>
352-
/// Gets the specified file as it appears in the pull request.
362+
/// Gets the before and after files needed for viewing a diff.
353363
/// </summary>
354-
/// <param name="file">The file or directory node.</param>
355-
/// <returns>The path to the extracted file.</returns>
356-
public Task<string> ExtractFile(IPullRequestFileNode file)
364+
/// <param name="file">The changed file.</param>
365+
/// <returns>A tuple containing the full path to the before and after files.</returns>
366+
public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
357367
{
358368
var path = Path.Combine(file.DirectoryPath, file.FileName);
359-
return pullRequestsService.ExtractFile(repository, modelService, model.Head.Sha, path, file.Sha).ToTask();
369+
return pullRequestsService.ExtractDiffFiles(repository, modelService, model, path, file.Sha, IsCheckedOut).ToTask();
360370
}
361371

362372
/// <summary>
363-
/// Gets the before and after files needed for viewing a diff.
373+
/// Gets the full path to a file in the working directory.
364374
/// </summary>
365-
/// <param name="file">The changed file.</param>
366-
/// <returns>A tuple containing the full path to the before and after files.</returns>
367-
public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
375+
/// <param name="file">The file.</param>
376+
/// <returns>The full path to the file in the working directory.</returns>
377+
public string GetLocalFilePath(IPullRequestFileNode file)
368378
{
369-
var path = Path.Combine(file.DirectoryPath, file.FileName);
370-
return pullRequestsService.ExtractDiffFiles(repository, modelService, model, path, file.Sha).ToTask();
379+
return Path.Combine(repository.LocalPath, file.DirectoryPath, file.FileName);
371380
}
372381

373382
void SubscribeOperationError(ReactiveCommand<Unit> command)

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,18 @@ IObservable<string> ExtractFile(
120120
/// <param name="pullRequest">The pull request details.</param>
121121
/// <param name="fileName">The filename relative to the repository root.</param>
122122
/// <param name="fileSha">The SHA of the file in the pull request.</param>
123-
/// <returns>The filenames of the left and right files for the diff.</returns>
123+
/// <param name="isPullRequestBranchCheckedOut">
124+
/// Whether the pull request branch is currently checked out. If so the right file returned
125+
/// will be the path to the file in the working directory.
126+
/// </param>
127+
/// <returns>The paths of the left and right files for the diff.</returns>
124128
IObservable<Tuple<string, string>> ExtractDiffFiles(
125129
ILocalRepositoryModel repository,
126130
IModelService modelService,
127131
IPullRequestModel pullRequest,
128132
string fileName,
129-
string fileSha);
133+
string fileSha,
134+
bool isPullRequestBranchCheckedOut);
130135

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

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ public interface IPullRequestDetailViewModel : IViewModel, IHasBusy
7979
/// </summary>
8080
string TargetBranchDisplayName { get; }
8181

82+
/// <summary>
83+
/// Gets a value indicating whether the pull request branch is checked out.
84+
/// </summary>
85+
bool IsCheckedOut { get; }
86+
8287
/// <summary>
8388
/// Gets a value indicating whether the pull request comes from a fork.
8489
/// </summary>
@@ -139,18 +144,18 @@ public interface IPullRequestDetailViewModel : IViewModel, IHasBusy
139144
/// </summary>
140145
ReactiveCommand<object> DiffFile { get; }
141146

142-
/// <summary>
143-
/// Gets the specified file as it appears in the pull request.
144-
/// </summary>
145-
/// <param name="file">The file or directory node.</param>
146-
/// <returns>The path to the extracted file.</returns>
147-
Task<string> ExtractFile(IPullRequestFileNode file);
148-
149147
/// <summary>
150148
/// Gets the before and after files needed for viewing a diff.
151149
/// </summary>
152150
/// <param name="file">The changed file.</param>
153151
/// <returns>A tuple containing the full path to the before and after files.</returns>
154152
Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file);
153+
154+
/// <summary>
155+
/// Gets the full path to a file in the working directory.
156+
/// </summary>
157+
/// <param name="file">The file.</param>
158+
/// <returns>The full path to the file in the working directory.</returns>
159+
string GetLocalFilePath(IPullRequestFileNode file);
155160
}
156161
}

‎src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@
286286
<!-- When invoked from the TreeView and the ListView we programmatically bind the
287287
DataContext and update the CommandParameter to the changed file -->
288288
<ContextMenu x:Key="FileContextMenu">
289-
<MenuItem Header="{x:Static prop:Resources.OpenFile}" Command="{Binding OpenFile}"/>
290289
<MenuItem Header="{x:Static prop:Resources.CompareFile}" Command="{Binding DiffFile}"/>
290+
<MenuItem Header="{x:Static prop:Resources.OpenFile}" Command="{Binding OpenFile}"/>
291291
</ContextMenu>
292292
</Grid.Resources>
293293

‎src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public PullRequestDetailView()
4141
this.WhenActivated(d =>
4242
{
4343
d(ViewModel.OpenOnGitHub.Subscribe(_ => DoOpenOnGitHub()));
44-
d(ViewModel.OpenFile.Subscribe(x => DoOpenFile((IPullRequestFileNode)x).Forget()));
44+
d(ViewModel.OpenFile.Subscribe(x => DoOpenFile((IPullRequestFileNode)x)));
4545
d(ViewModel.DiffFile.Subscribe(x => DoDiffFile((IPullRequestFileNode)x).Forget()));
4646
});
4747
}
@@ -65,15 +65,12 @@ void DoOpenOnGitHub()
6565
browser.OpenUrl(url);
6666
}
6767

68-
async Task DoOpenFile(IPullRequestFileNode file)
68+
void DoOpenFile(IPullRequestFileNode file)
6969
{
7070
try
7171
{
72-
var fileName = await ViewModel.ExtractFile(file);
73-
var window = Services.Dte.ItemOperations.OpenFile(fileName);
74-
75-
// If the file we extracted isn't the current file on disk, make the window read-only.
76-
window.Document.ReadOnly = fileName != file.DirectoryPath;
72+
var fileName = ViewModel.GetLocalFilePath(file);
73+
Services.Dte.ItemOperations.OpenFile(fileName);
7774
}
7875
catch (Exception e)
7976
{
@@ -90,19 +87,24 @@ async Task DoDiffFile(IPullRequestFileNode file)
9087
var rightLabel = $"{file.FileName};PR {ViewModel.Model.Number}";
9188
var caption = $"Diff - {file.FileName}";
9289
var tooltip = $"{leftLabel}\nvs.\n{rightLabel}";
90+
var options = __VSDIFFSERVICEOPTIONS.VSDIFFOPT_DetectBinaryFiles |
91+
__VSDIFFSERVICEOPTIONS.VSDIFFOPT_LeftFileIsTemporary;
92+
93+
if (!ViewModel.IsCheckedOut)
94+
{
95+
options |= __VSDIFFSERVICEOPTIONS.VSDIFFOPT_RightFileIsTemporary;
96+
}
9397

9498
Services.DifferenceService.OpenComparisonWindow2(
95-
fileNames.Item1,
96-
fileNames.Item2,
97-
caption,
98-
tooltip,
99-
leftLabel,
100-
rightLabel,
101-
string.Empty,
102-
string.Empty,
103-
(int)(__VSDIFFSERVICEOPTIONS.VSDIFFOPT_DetectBinaryFiles |
104-
__VSDIFFSERVICEOPTIONS.VSDIFFOPT_LeftFileIsTemporary |
105-
__VSDIFFSERVICEOPTIONS.VSDIFFOPT_RightFileIsTemporary));
99+
fileNames.Item1,
100+
fileNames.Item2,
101+
caption,
102+
tooltip,
103+
leftLabel,
104+
rightLabel,
105+
string.Empty,
106+
string.Empty,
107+
(uint)options);
106108
}
107109
catch (Exception e)
108110
{

0 commit comments

Comments
 (0)