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

Commit d27e281

Browse files
grokysgrokys
authored
Merge branch 'master' into fixes/1173-create-git-credential
2 parents 2c20161 + f05ca32 commit d27e281

File tree

7 files changed

+122
-90
lines changed

7 files changed

+122
-90
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.Exports/Models/DiffUtilities.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,27 +82,28 @@ public static IEnumerable<DiffChunk> ParseFragment(string diff)
8282

8383
public static DiffLine Match(IEnumerable<DiffChunk> diff, IList<DiffLine> target)
8484
{
85-
int j = 0;
86-
8785
if (target.Count == 0)
8886
{
8987
return null; // no lines to match
9088
}
9189

9290
foreach (var source in diff)
9391
{
92+
var matches = 0;
9493
for (var i = source.Lines.Count - 1; i >= 0; --i)
9594
{
96-
if (source.Lines[i].Content == target[j].Content)
95+
if (source.Lines[i].Content == target[matches].Content)
9796
{
98-
if (++j == target.Count || i == 0)
97+
matches++;
98+
if (matches == target.Count || i == 0)
9999
{
100-
return source.Lines[i + j - 1];
100+
return source.Lines[i + matches - 1];
101101
}
102102
}
103103
else
104104
{
105-
j = 0;
105+
i += matches;
106+
matches = 0;
106107
}
107108
}
108109
}

‎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: 13 additions & 10 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
}
@@ -102,7 +100,7 @@ public IEnumerable<ITagSpan<InlineCommentTag>> GetTags(NormalizedSnapshotSpanCol
102100
// Sucessful initialization will call NotifyTagsChanged, causing this method to be re-called.
103101
Initialize();
104102
}
105-
else if (file != null)
103+
else if (file != null && session != null)
106104
{
107105
foreach (var span in spans)
108106
{
@@ -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/Controls/RepositoryCloneControl.xaml.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,10 @@ public RepositoryGroupDescription(RepositoryCloneControl owner)
8888

8989
public override object GroupNameFromItem(object item, int level, System.Globalization.CultureInfo culture)
9090
{
91-
Guard.ArgumentNotNull(item, nameof(item));
9291
Guard.ArgumentNotNull(culture, nameof(culture));
9392

9493
var repo = item as IRemoteRepositoryModel;
95-
var name = repo.Owner;
94+
var name = repo?.Owner ?? string.Empty;
9695
RepositoryGroup group;
9796

9897
if (!owner.groups.TryGetValue(name, out group))

‎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
}

‎test/GitHub.InlineReviews.UnitTests/Models/DiffUtilitiesTests.cs‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -185,30 +185,41 @@ public void InvalidDiffLineChangeChar(string line, string expectMessage)
185185

186186
public class TheMatchMethod
187187
{
188+
/// <param name="diffLines">Target diff chunk with header (with '.' as line separator)</param>
189+
/// <param name="matchLines">Diff lines to match (with '.' as line separator)</param>
190+
/// <param name="expectedDiffLineNumber">The DiffLineNumber that the last line of matchLines falls on</param>
188191
[Theory]
189-
[InlineData(" 1", " 1", 0)]
190-
[InlineData(" 1. 2", " 2", 1)]
191-
[InlineData(" 1. 1", " 1", 1)] // match the later line
192+
[InlineData(" 1", " 1", 1)]
193+
[InlineData(" 1. 2", " 2", 2)]
194+
[InlineData(" 1. 1", " 1", 2)] // match the later line
192195
[InlineData("+x", "-x", -1)]
193196
[InlineData("", " x", -1)]
194197
[InlineData(" x", "", -1)]
195198

196-
[InlineData(" 1. 2.", " 2. 1.", 1)] // matched full context
197-
[InlineData(" 1. 2.", " 2. 3.", -1)] // didn't match full context
198-
[InlineData(" 2.", " 2. 1.", 0)] // match if we run out of context lines
199-
public void MatchLine(string lines1, string lines2, int skip /* -1 for no match */)
199+
[InlineData(" 1. 2.", " 1. 2.", 2)] // matched full context
200+
[InlineData(" 1. 2.", " 3. 2.", -1)] // didn't match full context
201+
[InlineData(" 2.", " 1. 2.", 1)] // match if we run out of context lines
202+
203+
// Tests for https://github.com/github/VisualStudio/issues/1149
204+
// Matching algorithm got confused when there was a partial match.
205+
[InlineData("+a.+x.+x.", "+a.+x.", 2)]
206+
[InlineData("+a.+x.+x.", "+a.+x.+x.", 3)]
207+
[InlineData("+a.+x.+x.+b.+x.+x.", "+a.+x.", 2)]
208+
[InlineData("+a.+x.+x.+b.+x.+x.", "+b.+x.", 5)]
209+
[InlineData("+a.+b.+x", "+a.+x.", -1)] // backtrack when there is a failed match
210+
public void MatchLine(string diffLines, string matchLines, int expectedDiffLineNumber /* -1 for no match */)
200211
{
201212
var header = "@@ -1 +1 @@";
202-
lines1 = lines1.Replace(".", "\r\n");
203-
lines2 = lines2.Replace(".", "\r\n");
204-
var chunks1 = DiffUtilities.ParseFragment(header + "\n" + lines1).ToList();
205-
var chunks2 = DiffUtilities.ParseFragment(header + "\n" + lines2).ToList();
206-
var expectLine = (skip != -1) ? chunks1.First().Lines.Skip(skip).First() : null;
207-
var targetLines = chunks2.First().Lines;
213+
diffLines = diffLines.Replace(".", "\r\n");
214+
matchLines = matchLines.Replace(".", "\r\n");
215+
var chunks1 = DiffUtilities.ParseFragment(header + "\n" + diffLines).ToList();
216+
var chunks2 = DiffUtilities.ParseFragment(header + "\n" + matchLines).ToList();
217+
var targetLines = chunks2.First().Lines.Reverse().ToList();
208218

209219
var line = DiffUtilities.Match(chunks1, targetLines);
210220

211-
Assert.Equal(expectLine, line);
221+
var diffLineNumber = (line != null) ? line.DiffLineNumber : -1;
222+
Assert.Equal(expectedDiffLineNumber, diffLineNumber);
212223
}
213224

214225
[Fact]

0 commit comments

Comments
 (0)