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

Commit f051dca

Browse files
grokysgrokys
authored
Merge pull request #1037 from github/jcansdale/shana-comments
Incorporate feedback from shana's inline-comments PR review
2 parents bf27bc6 + 64bece1 commit f051dca

File tree

10 files changed

+114
-102
lines changed

10 files changed

+114
-102
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ public Task<string> ExtractFile(IRepository repository, string commitSha, string
296296
return Task.Factory.StartNew(() =>
297297
{
298298
var commit = repository.Lookup<Commit>(commitSha);
299-
if(commit == null)
299+
if (commit == null)
300300
{
301301
throw new FileNotFoundException("Couldn't find '" + fileName + "' at commit " + commitSha + ".");
302302
}

‎src/GitHub.Extensions/StringExtensions.cs‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -180,21 +180,6 @@ public static string Wrap(this string text, int maxLength = 72)
180180
return sb.ToString();
181181
}
182182

183-
public static bool EqualsIgnoringLineEndings(this string a, string b)
184-
{
185-
if (ReferenceEquals(a, b))
186-
return true;
187-
188-
if (a == null || b == null)
189-
return false;
190-
191-
// TODO: Write a non-allocating comparison.
192-
a = a.Replace("\r\n", "\n");
193-
b = b.Replace("\r\n", "\n");
194-
195-
return a == b;
196-
}
197-
198183
public static Uri ToUriSafe(this string url)
199184
{
200185
Uri uri;

‎src/GitHub.InlineReviews/Glyph/GlyphData.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55

66
namespace GitHub.InlineReviews.Glyph.Implementation
77
{
8+
/// <summary>
9+
/// Information about the position of a glyph.
10+
/// </summary>
11+
/// <typeparam name="TGlyphTag">The type of glyph tag we're dealing with.</typeparam>
812
internal class GlyphData<TGlyphTag>
913
{
1014
double deltaY;

‎src/GitHub.InlineReviews/Glyph/GlyphMargin.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212

1313
namespace GitHub.InlineReviews.Glyph
1414
{
15+
/// <summary>
16+
/// Responsibe for updating the margin when tags change.
17+
/// </summary>
18+
/// <typeparam name="TGlyphTag">The type of glyph tag we're managing.</typeparam>
1519
public sealed class GlyphMargin<TGlyphTag> : IWpfTextViewMargin, ITextViewMargin, IDisposable where TGlyphTag: ITag
1620
{
1721
bool handleZoom;

‎src/GitHub.InlineReviews/Glyph/GlyphMarginVisualManager.cs‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212

1313
namespace GitHub.InlineReviews.Glyph.Implementation
1414
{
15+
/// <summary>
16+
/// Manage the MarginVisual element.
17+
/// </summary>
18+
/// <typeparam name="TGlyphTag">The type of tag we're managing.</typeparam>
1519
internal class GlyphMarginVisualManager<TGlyphTag> where TGlyphTag: ITag
1620
{
1721
readonly IEditorFormatMap editorFormatMap;
@@ -52,8 +56,6 @@ public GlyphMarginVisualManager(IWpfTextView textView, IGlyphFactory<TGlyphTag>
5256
}
5357
}
5458

55-
public FrameworkElement MarginVisual => glyphMarginGrid;
56-
5759
public void AddGlyph(TGlyphTag tag, SnapshotSpan span)
5860
{
5961
var textViewLines = textView.TextViewLines;
@@ -132,6 +134,7 @@ public void SetSnapshotAndUpdate(ITextSnapshot snapshot, IList<ITextViewLine> ne
132134
glyphs = dictionary;
133135
}
134136
}
137+
135138
static ITextViewLine GetStartingLine(IList<ITextViewLine> lines, Span span)
136139
{
137140
if (lines.Count > 0)
@@ -191,6 +194,8 @@ void UpdateBackgroundColor()
191194
ImageThemingUtilities.SetImageBackgroundColor(glyphMarginGrid, backgroundColor);
192195
}
193196
}
197+
198+
public FrameworkElement MarginVisual => glyphMarginGrid;
194199
}
195200
}
196201

‎src/GitHub.InlineReviews/Glyph/IGlyphFactory.cs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,24 @@
66

77
namespace GitHub.InlineReviews.Glyph
88
{
9+
/// <summary>
10+
/// A factory for a type of tag (or multiple subtypes).
11+
/// </summary>
12+
/// <typeparam name="TGlyphTag"></typeparam>
913
public interface IGlyphFactory<TGlyphTag> where TGlyphTag : ITag
1014
{
15+
/// <summary>
16+
/// Create a glyph element for a particular line and tag.
17+
/// </summary>
18+
/// <param name="line">The line.</param>
19+
/// <param name="tag">The tag.</param>
20+
/// <returns></returns>
1121
UIElement GenerateGlyph(IWpfTextViewLine line, TGlyphTag tag);
1222

23+
/// <summary>
24+
/// A list of tag subtypes this is a factory for.
25+
/// </summary>
26+
/// <returns></returns>
1327
IEnumerable<Type> GetTagTypes();
1428
}
1529
}

‎src/GitHub.InlineReviews/Peek/InlineCommentPeekResult.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
namespace GitHub.InlineReviews.Peek
77
{
8-
class InlineCommentPeekResult : IPeekResult
8+
sealed class InlineCommentPeekResult : IPeekResult
99
{
1010
public InlineCommentPeekResult(InlineCommentPeekViewModel viewModel)
1111
{

‎src/GitHub.InlineReviews/Services/PullRequestSession.cs‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -48,27 +48,6 @@ public PullRequestSession(
4848
Repository = repository;
4949
}
5050

51-
/// <inheritdoc/>
52-
public bool IsCheckedOut
53-
{
54-
get { return isCheckedOut; }
55-
internal set { this.RaiseAndSetIfChanged(ref isCheckedOut, value); }
56-
}
57-
58-
/// <inheritdoc/>
59-
public IAccount User { get; }
60-
61-
/// <inheritdoc/>
62-
public IPullRequestModel PullRequest { get; private set; }
63-
64-
/// <inheritdoc/>
65-
public ILocalRepositoryModel Repository { get; }
66-
67-
IEnumerable<string> FilePaths
68-
{
69-
get { return PullRequest.ChangedFiles.Select(x => x.FileName); }
70-
}
71-
7251
/// <inheritdoc/>
7352
public async Task AddComment(IPullRequestReviewCommentModel comment)
7453
{
@@ -285,5 +264,26 @@ int GetUpdatedLineNumber(IInlineCommentThreadModel thread, IEnumerable<DiffChunk
285264

286265
return -1;
287266
}
267+
268+
/// <inheritdoc/>
269+
public bool IsCheckedOut
270+
{
271+
get { return isCheckedOut; }
272+
internal set { this.RaiseAndSetIfChanged(ref isCheckedOut, value); }
273+
}
274+
275+
/// <inheritdoc/>
276+
public IAccount User { get; }
277+
278+
/// <inheritdoc/>
279+
public IPullRequestModel PullRequest { get; private set; }
280+
281+
/// <inheritdoc/>
282+
public ILocalRepositoryModel Repository { get; }
283+
284+
IEnumerable<string> FilePaths
285+
{
286+
get { return PullRequest.ChangedFiles.Select(x => x.FileName); }
287+
}
288288
}
289289
}

‎src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs‎

Lines changed: 60 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -89,66 +89,6 @@ public CommentViewModel(
8989
{
9090
}
9191

92-
/// <inheritdoc/>
93-
public int Id { get; private set; }
94-
95-
/// <inheritdoc/>
96-
public string Body
97-
{
98-
get { return body; }
99-
set { this.RaiseAndSetIfChanged(ref body, value); }
100-
}
101-
102-
/// <inheritdoc/>
103-
public string ErrorMessage
104-
{
105-
get { return this.errorMessage; }
106-
private set { this.RaiseAndSetIfChanged(ref errorMessage, value); }
107-
}
108-
109-
/// <inheritdoc/>
110-
public CommentEditState EditState
111-
{
112-
get { return state; }
113-
private set { this.RaiseAndSetIfChanged(ref state, value); }
114-
}
115-
116-
/// <inheritdoc/>
117-
public bool IsReadOnly
118-
{
119-
get { return isReadOnly; }
120-
set { this.RaiseAndSetIfChanged(ref isReadOnly, value); }
121-
}
122-
123-
/// <inheritdoc/>
124-
public DateTimeOffset UpdatedAt
125-
{
126-
get { return updatedAt; }
127-
private set { this.RaiseAndSetIfChanged(ref updatedAt, value); }
128-
}
129-
130-
/// <summary>
131-
/// Gets the current user.
132-
/// </summary>
133-
public IAccount CurrentUser { get; }
134-
135-
/// <summary>
136-
/// Gets the thread that the comment is a part of.
137-
/// </summary>
138-
public ICommentThreadViewModel Thread { get; }
139-
140-
/// <inheritdoc/>
141-
public IAccount User { get; }
142-
143-
/// <inheritdoc/>
144-
public ReactiveCommand<object> BeginEdit { get; }
145-
146-
/// <inheritdoc/>
147-
public ReactiveCommand<object> CancelEdit { get; }
148-
149-
/// <inheritdoc/>
150-
public ReactiveCommand<Unit> CommitEdit { get; }
151-
15292
/// <summary>
15393
/// Creates a placeholder comment which can be used to add a new comment to a thread.
15494
/// </summary>
@@ -208,5 +148,65 @@ async Task DoCommitEdit(object unused)
208148
ErrorMessage = e.Message;
209149
}
210150
}
151+
152+
/// <inheritdoc/>
153+
public int Id { get; private set; }
154+
155+
/// <inheritdoc/>
156+
public string Body
157+
{
158+
get { return body; }
159+
set { this.RaiseAndSetIfChanged(ref body, value); }
160+
}
161+
162+
/// <inheritdoc/>
163+
public string ErrorMessage
164+
{
165+
get { return this.errorMessage; }
166+
private set { this.RaiseAndSetIfChanged(ref errorMessage, value); }
167+
}
168+
169+
/// <inheritdoc/>
170+
public CommentEditState EditState
171+
{
172+
get { return state; }
173+
private set { this.RaiseAndSetIfChanged(ref state, value); }
174+
}
175+
176+
/// <inheritdoc/>
177+
public bool IsReadOnly
178+
{
179+
get { return isReadOnly; }
180+
set { this.RaiseAndSetIfChanged(ref isReadOnly, value); }
181+
}
182+
183+
/// <inheritdoc/>
184+
public DateTimeOffset UpdatedAt
185+
{
186+
get { return updatedAt; }
187+
private set { this.RaiseAndSetIfChanged(ref updatedAt, value); }
188+
}
189+
190+
/// <summary>
191+
/// Gets the current user.
192+
/// </summary>
193+
public IAccount CurrentUser { get; }
194+
195+
/// <summary>
196+
/// Gets the thread that the comment is a part of.
197+
/// </summary>
198+
public ICommentThreadViewModel Thread { get; }
199+
200+
/// <inheritdoc/>
201+
public IAccount User { get; }
202+
203+
/// <inheritdoc/>
204+
public ReactiveCommand<object> BeginEdit { get; }
205+
206+
/// <inheritdoc/>
207+
public ReactiveCommand<object> CancelEdit { get; }
208+
209+
/// <inheritdoc/>
210+
public ReactiveCommand<Unit> CommitEdit { get; }
211211
}
212212
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ void ApplyContextMenuBinding<TItem>(object sender, ContextMenuEventArgs e) where
197197
}
198198
}
199199

200-
private void ViewCommentsClick(object sender, RoutedEventArgs e)
200+
void ViewCommentsClick(object sender, RoutedEventArgs e)
201201
{
202202
var model = (object)ViewModel.Model;
203203
Services.Dte.Commands.Raise(
@@ -207,7 +207,7 @@ private void ViewCommentsClick(object sender, RoutedEventArgs e)
207207
null);
208208
}
209209

210-
private async void ViewFileCommentsClick(object sender, RoutedEventArgs e)
210+
async void ViewFileCommentsClick(object sender, RoutedEventArgs e)
211211
{
212212
try
213213
{

0 commit comments

Comments
 (0)