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

Commit 2b73ec5

Browse files
jcansdalejcansdale
committed
Merge branch 'fixes/1149-comments-not-displaying' of https://github.com/github/VisualStudio into fixes/1149-comments-not-displaying
2 parents aac9adb + 0871007 commit 2b73ec5

File tree

4 files changed

+88
-67
lines changed

4 files changed

+88
-67
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,6 @@ static string CreateTempFile(string fileName, string commitSha, string contents,
446446

447447
Directory.CreateDirectory(tempDir);
448448
File.WriteAllText(tempFile, contents, encoding);
449-
File.SetAttributes(tempFile, FileAttributes.ReadOnly);
450449
return tempFile;
451450
}
452451

‎src/GitHub.InlineReviews/Commands/InlineCommentNavigationCommand.cs‎

Lines changed: 67 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using GitHub.Extensions;
66
using GitHub.InlineReviews.Services;
77
using GitHub.InlineReviews.Tags;
8+
using GitHub.VisualStudio;
89
using Microsoft.VisualStudio;
910
using Microsoft.VisualStudio.ComponentModelHost;
1011
using Microsoft.VisualStudio.Editor;
@@ -98,72 +99,83 @@ protected int GetCursorPoint(ITextView textView, int lineNumber)
9899
/// </remarks>
99100
protected IEnumerable<ITextView> GetCurrentTextViews()
100101
{
101-
var serviceProvider = Package;
102-
var monitorSelection = (IVsMonitorSelection)serviceProvider.GetService(typeof(SVsShellMonitorSelection));
103-
if (monitorSelection == null)
104-
{
105-
yield break;
106-
}
102+
var result = new List<ITextView>();
107103

108-
object curDocument;
109-
if (ErrorHandler.Failed(monitorSelection.GetCurrentElementValue((uint)VSConstants.VSSELELEMID.SEID_DocumentFrame, out curDocument)))
104+
try
110105
{
111-
yield break;
112-
}
106+
var serviceProvider = Package;
107+
var monitorSelection = (IVsMonitorSelection)serviceProvider.GetService(typeof(SVsShellMonitorSelection));
108+
if (monitorSelection == null)
109+
{
110+
return result;
111+
}
113112

114-
IVsWindowFrame frame = curDocument as IVsWindowFrame;
115-
if (frame == null)
116-
{
117-
yield break;
118-
}
113+
object curDocument;
114+
if (ErrorHandler.Failed(monitorSelection.GetCurrentElementValue((uint)VSConstants.VSSELELEMID.SEID_DocumentFrame, out curDocument)))
115+
{
116+
return result;
117+
}
119118

120-
object docView = null;
121-
if (ErrorHandler.Failed(frame.GetProperty((int)__VSFPROPID.VSFPROPID_DocView, out docView)))
122-
{
123-
yield break;
124-
}
119+
IVsWindowFrame frame = curDocument as IVsWindowFrame;
120+
if (frame == null)
121+
{
122+
return result;
123+
}
125124

126-
if (docView is IVsDifferenceCodeWindow)
127-
{
128-
var diffWindow = (IVsDifferenceCodeWindow)docView;
129-
130-
switch (diffWindow.DifferenceViewer.ViewMode)
125+
object docView = null;
126+
if (ErrorHandler.Failed(frame.GetProperty((int)__VSFPROPID.VSFPROPID_DocView, out docView)))
131127
{
132-
case DifferenceViewMode.Inline:
133-
yield return diffWindow.DifferenceViewer.InlineView;
134-
break;
135-
case DifferenceViewMode.SideBySide:
136-
switch (diffWindow.DifferenceViewer.ActiveViewType)
137-
{
138-
case DifferenceViewType.LeftView:
139-
yield return diffWindow.DifferenceViewer.LeftView;
140-
yield return diffWindow.DifferenceViewer.RightView;
141-
break;
142-
case DifferenceViewType.RightView:
143-
yield return diffWindow.DifferenceViewer.RightView;
144-
yield return diffWindow.DifferenceViewer.LeftView;
145-
break;
146-
}
147-
yield return diffWindow.DifferenceViewer.LeftView;
148-
break;
149-
case DifferenceViewMode.RightViewOnly:
150-
yield return diffWindow.DifferenceViewer.RightView;
151-
break;
128+
return result;
152129
}
153-
}
154-
else if (docView is IVsCodeWindow)
155-
{
156-
IVsTextView textView;
157-
if (ErrorHandler.Failed(((IVsCodeWindow)docView).GetPrimaryView(out textView)))
130+
131+
if (docView is IVsDifferenceCodeWindow)
158132
{
159-
yield break;
133+
var diffWindow = (IVsDifferenceCodeWindow)docView;
134+
135+
switch (diffWindow.DifferenceViewer.ViewMode)
136+
{
137+
case DifferenceViewMode.Inline:
138+
result.Add(diffWindow.DifferenceViewer.InlineView);
139+
break;
140+
case DifferenceViewMode.SideBySide:
141+
switch (diffWindow.DifferenceViewer.ActiveViewType)
142+
{
143+
case DifferenceViewType.LeftView:
144+
result.Add(diffWindow.DifferenceViewer.LeftView);
145+
result.Add(diffWindow.DifferenceViewer.RightView);
146+
break;
147+
case DifferenceViewType.RightView:
148+
result.Add(diffWindow.DifferenceViewer.RightView);
149+
result.Add(diffWindow.DifferenceViewer.LeftView);
150+
break;
151+
}
152+
result.Add(diffWindow.DifferenceViewer.LeftView);
153+
break;
154+
case DifferenceViewMode.RightViewOnly:
155+
result.Add(diffWindow.DifferenceViewer.RightView);
156+
break;
157+
}
160158
}
159+
else if (docView is IVsCodeWindow)
160+
{
161+
IVsTextView textView;
162+
if (ErrorHandler.Failed(((IVsCodeWindow)docView).GetPrimaryView(out textView)))
163+
{
164+
return result;
165+
}
161166

162-
var model = (IComponentModel)serviceProvider.GetService(typeof(SComponentModel));
163-
var adapterFactory = model.GetService<IVsEditorAdaptersFactoryService>();
164-
var wpfTextView = adapterFactory.GetWpfTextView(textView);
165-
yield return wpfTextView;
167+
var model = (IComponentModel)serviceProvider.GetService(typeof(SComponentModel));
168+
var adapterFactory = model.GetService<IVsEditorAdaptersFactoryService>();
169+
var wpfTextView = adapterFactory.GetWpfTextView(textView);
170+
result.Add(wpfTextView);
171+
}
172+
}
173+
catch (Exception e)
174+
{
175+
VsOutputLogger.WriteLine("Exception in InlineCommentNavigationCommand.GetCurrentTextViews(): {0}", e);
166176
}
177+
178+
return result;
167179
}
168180

169181
/// <summary>

‎src/GitHub.InlineReviews/Tags/InlineCommentTagger.cs‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.IO;
43
using System.Linq;
54
using System.Reactive.Linq;
65
using System.Reactive.Subjects;
@@ -15,8 +14,7 @@
1514
using Microsoft.VisualStudio.Text.Tagging;
1615
using ReactiveUI;
1716
using System.Collections;
18-
using GitHub.Helpers;
19-
17+
using GitHub.VisualStudio;
2018
namespace GitHub.InlineReviews.Tags
2119
{
2220
/// <summary>
@@ -78,7 +76,7 @@ public InlineCommentTagger(
7876
signalRebuild = new Subject<ITextSnapshot>();
7977
signalRebuild.Throttle(TimeSpan.FromMilliseconds(500))
8078
.ObserveOn(RxApp.MainThreadScheduler)
81-
.Subscribe(x => Rebuild(x).Forget());
79+
.Subscribe(x => ForgetWithLogging(Rebuild(x)));
8280

8381
this.buffer.Changed += Buffer_Changed;
8482
}
@@ -137,8 +135,8 @@ public IEnumerable<ITagSpan<InlineCommentTag>> GetTags(NormalizedSnapshotSpanCol
137135
{
138136
var lineNumber = (leftHandSide ? line.OldLineNumber : line.NewLineNumber) - 1;
139137

140-
if (lineNumber >= startLine &&
141-
lineNumber <= endLine &&
138+
if (lineNumber >= startLine &&
139+
lineNumber <= endLine &&
142140
!linesWithComments[lineNumber - startLine]
143141
&& (!leftHandSide || line.Type == DiffChangeType.Delete))
144142
{
@@ -187,16 +185,21 @@ void Initialize()
187185

188186
if (session == null)
189187
{
190-
managerSubscription = sessionManager.WhenAnyValue(x => x.CurrentSession).Subscribe(SessionChanged);
188+
managerSubscription = sessionManager.WhenAnyValue(x => x.CurrentSession).Subscribe(x => ForgetWithLogging(SessionChanged(x)));
191189
}
192190
else
193191
{
194-
SessionChanged(session);
192+
ForgetWithLogging(SessionChanged(session));
195193
}
196194

197195
initialized = true;
198196
}
199197

198+
static void ForgetWithLogging(Task task)
199+
{
200+
task.Catch(e => VsOutputLogger.WriteLine("Exception caught while executing background task: {0}", e)).Forget();
201+
}
202+
200203
void NotifyTagsChanged()
201204
{
202205
var entireFile = new SnapshotSpan(buffer.CurrentSnapshot, 0, buffer.CurrentSnapshot.Length);
@@ -210,7 +213,7 @@ void NotifyTagsChanged(int lineNumber)
210213
TagsChanged?.Invoke(this, new SnapshotSpanEventArgs(span));
211214
}
212215

213-
async void SessionChanged(IPullRequestSession session)
216+
async Task SessionChanged(IPullRequestSession session)
214217
{
215218
sessionSubscription?.Dispose();
216219
this.session = session;

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ async Task DoOpenFile(IPullRequestFileNode file, bool workingDirectory)
8787
var fullPath = ViewModel.GetLocalFilePath(file);
8888
var fileName = workingDirectory ? fullPath : await ViewModel.ExtractFile(file, true);
8989

90-
using (new NewDocumentStateScope(__VSNEWDOCUMENTSTATE.NDS_Provisional, VSConstants.NewDocumentStateReason.SolutionExplorer))
90+
using (workingDirectory ? null : OpenInProvisionalTab())
9191
{
9292
var window = Services.Dte.ItemOperations.OpenFile(fileName);
9393
window.Document.ReadOnly = !workingDirectory;
@@ -127,7 +127,7 @@ async Task DoDiffFile(IPullRequestFileNode file, bool workingDirectory)
127127
}
128128

129129
IVsWindowFrame frame;
130-
using (new NewDocumentStateScope(__VSNEWDOCUMENTSTATE.NDS_Provisional, VSConstants.NewDocumentStateReason.SolutionExplorer))
130+
using (OpenInProvisionalTab())
131131
{
132132
var tooltip = $"{leftLabel}\nvs.\n{rightLabel}";
133133

@@ -315,5 +315,12 @@ void OpenHyperlink(object sender, ExecutedRoutedEventArgs e)
315315
VisualStudioBrowser.OpenUrl(uri);
316316
}
317317
}
318+
319+
static IDisposable OpenInProvisionalTab()
320+
{
321+
return new NewDocumentStateScope
322+
(__VSNEWDOCUMENTSTATE.NDS_Provisional,
323+
VSConstants.NewDocumentStateReason.SolutionExplorer);
324+
}
318325
}
319326
}

0 commit comments

Comments
 (0)