Merge branch 'master' into grokys/pr-details-busy

Author: grokys

src/GitHub.App/GitHub.App.csproj

@@ -132,6 +132,7 @@
     <Reference Include="WindowsBase" />
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="ViewModels\ViewModelBase.cs" />
     <None Include="..\..\script\Key.snk" Condition="$(Buildtype) == 'Internal'">
       <Link>Key.snk</Link>
     </None>

src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs

@@ -75,6 +75,7 @@ public PullRequestDetailViewModelDesigner()
         public string TargetBranchDisplayName { get; set; }
         public bool IsLoading { get; }
         public bool IsBusy { get; }
+        public bool IsCheckedOut { get; }
         public bool IsFromFork { get; }
         public string Body { get; }
         public IReadOnlyList<IPullRequestChangeNode> ChangedFilesTree => changedFilesTree;
@@ -89,12 +90,12 @@ public PullRequestDetailViewModelDesigner()
         public ReactiveCommand<object> OpenFile { get; }
         public ReactiveCommand<object> DiffFile { get; }
 
-        public Task<string> ExtractFile(IPullRequestFileNode file)
+        public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
         {
             return null;
         }
 
-        public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
+        public string GetLocalFilePath(IPullRequestFileNode file)
         {
             return null;
         }

src/GitHub.App/Services/DialogService.cs

@@ -30,7 +30,7 @@ public Task<CloneDialogResult> ShowCloneDialog([AllowNull] IConnection connectio
             {
                 var vm = x.View.ViewModel as IBaseCloneViewModel;
 
-                ((IHasDone)x.View.ViewModel).Done.Subscribe(_ =>
+                vm.Done.Subscribe(_ =>
                 {
                     basePath = vm?.BaseRepositoryPath;
                     repository = vm?.SelectedRepository;
@@ -58,7 +58,7 @@ public Task<string> ShowReCloneDialog(IRepositoryModel repository)
                     vm.SelectedRepository = repository;
                 }
 
-                ((IHasDone)x.View.ViewModel).Done.Subscribe(_ =>
+                vm.Done.Subscribe(_ =>
                 {
                     basePath = vm?.BaseRepositoryPath;
                 });

src/GitHub.App/Services/PullRequestService.cs

@@ -287,7 +287,8 @@ public IObservable<Tuple<string, string>> ExtractDiffFiles(
             IModelService modelService,
             IPullRequestModel pullRequest,
             string fileName,
-            string fileSha)
+            string fileSha,
+            bool isPullRequestBranchCheckedOut)
         {
             return Observable.Defer(async () =>
             {
@@ -300,7 +301,9 @@ public IObservable<Tuple<string, string>> ExtractDiffFiles(
 
                 // The right file - if it comes from a fork - may not be fetched so fall back to
                 // getting the file contents from the model service.
-                var right = await GetFileFromRepositoryOrApi(repository, repo, modelService, pullRequest.Head.Sha, fileName, fileSha);
+                var right = isPullRequestBranchCheckedOut ?
+                    Path.Combine(repository.LocalPath, fileName) :
+                    await GetFileFromRepositoryOrApi(repository, repo, modelService, pullRequest.Head.Sha, fileName, fileSha);
 
                 if (left == null)
                 {

src/GitHub.App/ViewModels/DialogViewModelBase.cs

@@ -1,17 +1,15 @@
-using System.Windows.Input;
-using ReactiveUI;
-using NullGuard;
-using GitHub.UI;
-using System;
+using System;
 using System.Reactive;
 using GitHub.Extensions.Reactive;
+using NullGuard;
+using ReactiveUI;
 
 namespace GitHub.ViewModels
 {
     /// <summary>
     /// Base class for view models that can be dismissed, such as dialogs.
     /// </summary>
-    public abstract class DialogViewModelBase : ReactiveObject, IDialogViewModel, IHasBusy
+    public abstract class DialogViewModelBase : ViewModelBase, IDialogViewModel, IHasBusy
     {
         protected ObservableAsPropertyHelper<bool> isShowing;
         string title;
@@ -51,10 +49,5 @@ public bool IsBusy
 
         /// <inheritdoc/>
         IObservable<Unit> IHasCancel.Cancel => Cancel.SelectUnit();
-
-        /// <inheritdoc/>
-        public virtual void Initialize([AllowNull] ViewWithData data)
-        {
-        }
     }
 }

src/GitHub.App/ViewModels/PanePageViewModelBase.cs

@@ -7,7 +7,7 @@ namespace GitHub.ViewModels
     /// <summary>
     /// Base class for view models that appear as a page in a navigable pane, such as the GitHub pane.
     /// </summary>
-    public class PanePageViewModelBase : ReactiveObject, IPanePageViewModel
+    public class PanePageViewModelBase : ViewModelBase, IPanePageViewModel
     {
         string title;
 
@@ -25,10 +25,5 @@ public string Title
             get { return title; }
             protected set { this.RaiseAndSetIfChanged(ref title, value); }
         }
-
-        /// <inheritdoc/>
-        public virtual void Initialize([AllowNull] ViewWithData data)
-        {
-        }
     }
 }

src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs

@@ -41,6 +41,7 @@ public class PullRequestDetailViewModel : PanePageViewModelBase, IPullRequestDet
         string operationError;
         bool isBusy;
         bool isLoading;
+        bool isCheckedOut;
         bool isFromFork;
         bool isInCheckout;
 
@@ -105,7 +106,7 @@ public PullRequestDetailViewModel(
             SubscribeOperationError(Push);
 
             OpenOnGitHub = ReactiveCommand.Create();
-            OpenFile = ReactiveCommand.Create();
+            OpenFile = ReactiveCommand.Create(this.WhenAnyValue(x => x.IsCheckedOut));
             DiffFile = ReactiveCommand.Create();
         }
 
@@ -156,6 +157,14 @@ public bool IsBusy
             private set { this.RaiseAndSetIfChanged(ref isBusy, value); }
         }
 
+        /// Gets a value indicating whether the pull request branch is checked out.
+        /// </summary>
+        public bool IsCheckedOut
+        {
+            get { return isCheckedOut; }
+            private set { this.RaiseAndSetIfChanged(ref isCheckedOut, value); }
+        }
+
         /// <summary>
         /// Gets a value indicating whether the view model is loading.
         /// </summary>
@@ -289,9 +298,10 @@ public async Task Load(IPullRequestModel pullRequest)
             ChangedFilesTree = CreateChangedFilesTree(pullRequest, changes).Children.ToList();
 
             var localBranches = await pullRequestsService.GetLocalBranches(repository, pullRequest).ToList();
-            var isCheckedOut = localBranches.Contains(repository.CurrentBranch);
 
-            if (isCheckedOut)
+            IsCheckedOut = localBranches.Contains(repository.CurrentBranch);
+
+            if (IsCheckedOut)
             {
                 var divergence = await pullRequestsService.CalculateHistoryDivergence(repository, Model.Number);
                 var pullEnabled = divergence.BehindBy > 0;
@@ -360,25 +370,24 @@ public async Task Load(IPullRequestModel pullRequest)
         }
 
         /// <summary>
-        /// Gets the specified file as it appears in the pull request.
+        /// Gets the before and after files needed for viewing a diff.
         /// </summary>
-        /// <param name="file">The file or directory node.</param>
-        /// <returns>The path to the extracted file.</returns>
-        public Task<string> ExtractFile(IPullRequestFileNode file)
+        /// <param name="file">The changed file.</param>
+        /// <returns>A tuple containing the full path to the before and after files.</returns>
+        public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
         {
             var path = Path.Combine(file.DirectoryPath, file.FileName);
-            return pullRequestsService.ExtractFile(repository, modelService, model.Head.Sha, path, file.Sha).ToTask();
+            return pullRequestsService.ExtractDiffFiles(repository, modelService, model, path, file.Sha, IsCheckedOut).ToTask();
         }
 
         /// <summary>
-        /// Gets the before and after files needed for viewing a diff.
+        /// Gets the full path to a file in the working directory.
         /// </summary>
-        /// <param name="file">The changed file.</param>
-        /// <returns>A tuple containing the full path to the before and after files.</returns>
-        public Task<Tuple<string, string>> ExtractDiffFiles(IPullRequestFileNode file)
+        /// <param name="file">The file.</param>
+        /// <returns>The full path to the file in the working directory.</returns>
+        public string GetLocalFilePath(IPullRequestFileNode file)
         {
-            var path = Path.Combine(file.DirectoryPath, file.FileName);
-            return pullRequestsService.ExtractDiffFiles(repository, modelService, model, path, file.Sha).ToTask();
+            return Path.Combine(repository.LocalPath, file.DirectoryPath, file.FileName);
         }
 
         void SubscribeOperationError(ReactiveCommand<Unit> command)

src/GitHub.App/ViewModels/PullRequestListViewModel.cs

@@ -226,8 +226,8 @@ public IAccount EmptyUser
             get { return emptyUser; }
         }
 
-        readonly Subject<ViewWithData> load = new Subject<ViewWithData>();
-        public IObservable<ViewWithData> Navigate => load;
+        readonly Subject<ViewWithData> navigate = new Subject<ViewWithData>();
+        public IObservable<ViewWithData> Navigate => navigate;
 
         public ReactiveCommand<object> OpenPullRequest { get; }
         public ReactiveCommand<object> CreatePullRequest { get; }
@@ -263,13 +263,13 @@ void SaveSettings()
         void DoOpenPullRequest(object pullRequest)
         {
             var d = new ViewWithData(UIControllerFlow.PullRequestDetail) { Data = pullRequest };
-            load.OnNext(d);
+            navigate.OnNext(d);
         }
 
         void DoCreatePullRequest()
         {
             var d = new ViewWithData(UIControllerFlow.PullRequestCreation);
-            load.OnNext(d);
+            navigate.OnNext(d);
         }
     }
 }

src/GitHub.App/ViewModels/ViewModelBase.cs

@@ -0,0 +1,18 @@
+using System;
+using GitHub.UI;
+using NullGuard;
+using ReactiveUI;
+
+namespace GitHub.ViewModels
+{
+    /// <summary>
+    /// Base class for view models.
+    /// </summary>
+    public abstract class ViewModelBase : ReactiveObject, IViewModel
+    {
+        /// <inheritdoc/>
+        public virtual void Initialize([AllowNull] ViewWithData data)
+        {
+        }
+    }
+}

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

@@ -120,13 +120,18 @@ IObservable<string> ExtractFile(
         /// <param name="pullRequest">The pull request details.</param>
         /// <param name="fileName">The filename relative to the repository root.</param>
         /// <param name="fileSha">The SHA of the file in the pull request.</param>
-        /// <returns>The filenames of the left and right files for the diff.</returns>
+        /// <param name="isPullRequestBranchCheckedOut">
+        /// Whether the pull request branch is currently checked out. If so the right file returned
+        /// will be the path to the file in the working directory.
+        /// </param>
+        /// <returns>The paths of the left and right files for the diff.</returns>
         IObservable<Tuple<string, string>> ExtractDiffFiles(
             ILocalRepositoryModel repository,
             IModelService modelService,
             IPullRequestModel pullRequest,
             string fileName,
-            string fileSha);
+            string fileSha,
+            bool isPullRequestBranchCheckedOut);
 
         /// <summary>
         /// Remotes all unused remotes that were created by GitHub for Visual Studio to track PRs