Skip to content

Commit 36472be

Browse files
committed
code_review: PR #2474
- Use `MemoryStream.TryGetBuffer` instead of `MemoryStream.ToArray()` to avoid data-copy - Parse chunk body first when the last readed line is a chunk body. This is because that the number of chunk body lines never less than the chunk indicator - Use const strings instead of hardcoded inline check - Avoid manually checking the prefix when the next step is checking with compiled regex expression. It also checks the start/prefix of given string - Since the submodule change in a typed-change diff may be the second `diff --git` block, we can not using `_result.TextDiff.Lines.Count != 1` Signed-off-by: leo <longshuang@msn.cn>
1 parent a30cf17 commit 36472be

2 files changed

Lines changed: 126 additions & 142 deletions

File tree

‎src/Commands/Diff.cs‎

Lines changed: 124 additions & 140 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,23 @@ public partial class Diff : Command
1616
[GeneratedRegex(@"^index\s([0-9a-f]{6,64})\.\.([0-9a-f]{6,64})(\s[1-9]{6})?")]
1717
private static partial Regex REG_HASH_CHANGE();
1818

19+
private const char PREFIX_CONTEXT = ' ';
20+
private const char PREFIX_DELETED = '-';
21+
private const char PREFIX_ADDED = '+';
22+
private const char PREFIX_COMMAND = '\\';
23+
24+
private const string FILE_MODE_OLD = "old mode ";
25+
private const string FILE_MODE_NEW = "new mode ";
26+
private const string FILE_MODE_DELETED = "deleted file mode ";
27+
private const string FILE_MODE_ADDED = "new file mode ";
28+
1929
private const string LFS_SPECIFIER = "version https://git-lfs.github.com/spec/";
2030
private const string LFS_OID_PREFIX = "oid sha256:";
2131
private const string LFS_SIZE_PREFIX = "size ";
2232

23-
private enum Indicator
24-
{
25-
ChunkHeader = '@',
26-
Context = ' ',
27-
Old = '-',
28-
New = '+',
29-
Special = '\\',
30-
}
33+
private const string SPECIAL_DIFF_START = "diff ";
34+
private const string SPECIAL_BINARY = "Binary files ";
35+
private const string SPECIAL_NO_NEWLINE = " No newline at end of file";
3136

3237
public Diff(string repo, Models.DiffOption opt, int numContextLines, bool ignoreWhitespace, bool ignoreCRAtEOL)
3338
{
@@ -59,22 +64,25 @@ public Diff(string repo, Models.DiffOption opt, int numContextLines, bool ignore
5964
using var ms = new MemoryStream();
6065
await proc.StandardOutput.BaseStream.CopyToAsync(ms, CancellationToken).ConfigureAwait(false);
6166

62-
var bytes = ms.ToArray();
63-
var start = 0;
64-
while (start < bytes.Length)
67+
if (ms.TryGetBuffer(out var buffer))
6568
{
66-
var end = Array.IndexOf(bytes, (byte)'\n', start);
67-
if (end < 0)
69+
var start = buffer.Offset;
70+
var end = buffer.Offset + buffer.Count;
71+
while (start < end)
6872
{
69-
ParseLine(bytes[start..]);
70-
break;
71-
}
73+
var lineEnd = Array.IndexOf(buffer.Array, (byte)'\n', start);
74+
if (lineEnd < 0)
75+
{
76+
ParseLine(buffer[start..]);
77+
break;
78+
}
7279

73-
ParseLine(bytes[start..end]);
74-
if (_result.IsBinary)
75-
break;
80+
ParseLine(buffer[start..lineEnd]);
81+
if (_result.IsBinary)
82+
break;
7683

77-
start = end + 1;
84+
start = lineEnd + 1;
85+
}
7886
}
7987

8088
await proc.WaitForExitAsync(CancellationToken).ConfigureAwait(false);
@@ -106,199 +114,175 @@ public Diff(string repo, Models.DiffOption opt, int numContextLines, bool ignore
106114
return _result;
107115
}
108116

109-
private void ParseLine(byte[] lineBytes)
117+
private void ParseLine(ArraySegment<byte> lineBytes)
110118
{
111-
var line = Encoding.UTF8.GetString(lineBytes);
119+
// Decode line bytes to UTF-8 string
120+
var line = Encoding.UTF8.GetString(lineBytes.Array, lineBytes.Offset, lineBytes.Count);
112121
if (line.Length == 0)
113122
return;
114123

115-
if (ParseChunkStartLine(line, lineBytes))
116-
return;
124+
// If we are reading a chunk body, try to read the current line as body first (because
125+
// the number of chunk body is greater than the number of chunk indicator in most time.
126+
if (_isInChunk)
127+
{
128+
if (ParseChunkBodyLine(line, lineBytes[1..]))
129+
return;
130+
131+
ProcessInlineHighlights();
132+
_isInChunk = false;
133+
}
117134

118-
if (ParseChunkBodyLine(line[0], line.Substring(1), lineBytes[1..]))
135+
// If the current line is not a chunk body, try to parse it as chunk indicator
136+
if (ParseChunkStartLine(line))
119137
return;
120138

139+
// Fallback to diff headers to support type-changed diff (multiple headers).
121140
ParseDiffHeaderLine(line);
122141
}
123142

124143
private void ParseDiffHeaderLine(string line)
125144
{
126-
if (line.StartsWith("diff"))
145+
if (line.StartsWith(SPECIAL_DIFF_START, StringComparison.Ordinal))
127146
return;
128147

148+
if (line.StartsWith(SPECIAL_BINARY, StringComparison.Ordinal))
149+
_result.IsBinary = true;
150+
129151
if (ParseFileModeChange(line))
130152
return;
131153

132-
if (line.StartsWith("index"))
154+
var match = REG_HASH_CHANGE().Match(line);
155+
if (match.Success)
133156
{
134-
var match = REG_HASH_CHANGE().Match(line);
135-
if (match.Success)
136-
{
137-
// NOTE: For a TypeChanged file we receive two full sets of diff-lines within
138-
// the same diff output, indicating a 'deleted file' followed by a 'new file' .
139-
// We then keep the oldest Old hash and the newest New hash.
140-
if (string.IsNullOrEmpty(_result.OldHash))
141-
_result.OldHash = match.Groups[1].Value;
142-
_result.NewHash = match.Groups[2].Value;
143-
}
157+
if (string.IsNullOrEmpty(_result.OldHash))
158+
_result.OldHash = match.Groups[1].Value;
159+
_result.NewHash = match.Groups[2].Value;
144160
return;
145161
}
146-
147-
if (line.StartsWith("Binary", StringComparison.Ordinal))
148-
_result.IsBinary = true;
149162
}
150163

151-
private bool ParseChunkStartLine(System.String line, byte[] lineBytes)
164+
private bool ParseChunkStartLine(string line)
152165
{
153-
if (line[0] == (char)Indicator.ChunkHeader)
166+
var match = REG_INDICATOR().Match(line);
167+
if (match.Success)
154168
{
155-
if (_isInChunk)
156-
{
157-
ProcessInlineHighlights();
158-
_isInChunk = false;
159-
}
160-
161-
var match = REG_INDICATOR().Match(line);
162-
if (match.Success)
163-
{
164-
_oldLine = int.Parse(match.Groups[1].Value);
165-
_newLine = int.Parse(match.Groups[2].Value);
166-
_last = new Models.TextDiffLine(Models.TextDiffLineType.Indicator, line, lineBytes, 0, 0);
167-
_result.TextDiff.Lines.Add(_last);
168-
169-
_isInChunk = true;
170-
return true;
171-
}
169+
_oldLine = int.Parse(match.Groups[1].Value);
170+
_newLine = int.Parse(match.Groups[2].Value);
171+
_last = new Models.TextDiffLine(Models.TextDiffLineType.Indicator, line, null, 0, 0);
172+
_result.TextDiff.Lines.Add(_last);
173+
_isInChunk = true;
174+
return true;
172175
}
176+
173177
return false;
174178
}
175179

176-
private bool ParseChunkBodyLine(char ch, string line, byte[] rawContent)
180+
private bool ParseChunkBodyLine(string line, ArraySegment<byte> lineBytes)
177181
{
178-
if (_isInChunk)
179-
{
180-
if (ParseLFSChange(ch, line))
181-
return true;
182+
var prefix = line[0];
183+
var content = line.Substring(1);
184+
if (ParseLFSChange(prefix, content))
185+
return true;
182186

183-
if (ch == (char)Indicator.Old)
184-
{
185-
_result.TextDiff.DeletedLines++;
186-
_last = new Models.TextDiffLine(Models.TextDiffLineType.Deleted, line, rawContent, _oldLine, 0);
187-
_deleted.Add(_last);
188-
_oldLine++;
189-
return true;
190-
}
187+
if (prefix == PREFIX_DELETED)
188+
{
189+
_result.TextDiff.DeletedLines++;
190+
_last = new Models.TextDiffLine(Models.TextDiffLineType.Deleted, content, lineBytes.ToArray(), _oldLine, 0);
191+
_deleted.Add(_last);
192+
_oldLine++;
193+
return true;
194+
}
191195

192-
if (ch == (char)Indicator.New)
193-
{
194-
_result.TextDiff.AddedLines++;
195-
_last = new Models.TextDiffLine(Models.TextDiffLineType.Added, line, rawContent, 0, _newLine);
196-
_added.Add(_last);
197-
_newLine++;
198-
return true;
199-
}
196+
if (prefix == PREFIX_ADDED)
197+
{
198+
_result.TextDiff.AddedLines++;
199+
_last = new Models.TextDiffLine(Models.TextDiffLineType.Added, content, lineBytes.ToArray(), 0, _newLine);
200+
_added.Add(_last);
201+
_newLine++;
202+
return true;
203+
}
200204

201-
if (ch == (char)Indicator.Context)
202-
{
203-
ProcessInlineHighlights();
205+
if (prefix == PREFIX_CONTEXT)
206+
{
207+
ProcessInlineHighlights();
204208

205-
_last = new Models.TextDiffLine(Models.TextDiffLineType.Normal, line, rawContent, _oldLine, _newLine);
206-
_result.TextDiff.Lines.Add(_last);
207-
_oldLine++;
208-
_newLine++;
209-
return true;
210-
}
209+
_last = new Models.TextDiffLine(Models.TextDiffLineType.Normal, content, lineBytes.ToArray(), _oldLine, _newLine);
210+
_result.TextDiff.Lines.Add(_last);
211+
_oldLine++;
212+
_newLine++;
213+
return true;
214+
}
211215

212-
if (ch == (char)Indicator.Special)
213-
{
214-
if (line.Equals(" No newline at end of file", StringComparison.Ordinal))
215-
_last.NoNewLineEndOfFile = true;
216-
return true;
217-
}
216+
if (prefix == PREFIX_COMMAND)
217+
{
218+
if (content.Equals(SPECIAL_NO_NEWLINE, StringComparison.Ordinal))
219+
_last.NoNewLineEndOfFile = true;
220+
return true;
218221
}
219222

220-
ProcessInlineHighlights();
221-
_isInChunk = false;
222223
return false;
223224
}
224225

225-
private int ParseFileModeNumber(string fileModeStr)
226-
{
227-
int fileMode = 0;
228-
Int32.TryParse(fileModeStr, out fileMode);
229-
return fileMode;
230-
}
231-
232226
private bool ParseFileModeChange(string line)
233227
{
234-
if (line.StartsWith("old mode ", StringComparison.Ordinal))
228+
if (line.StartsWith(FILE_MODE_OLD, StringComparison.Ordinal))
235229
{
236-
_result.OldMode = ParseFileMode(line.Substring(9));
230+
_result.OldMode = int.Parse(line.AsSpan(9));
237231
return true;
238232
}
239233

240-
if (line.StartsWith("new mode ", StringComparison.Ordinal))
234+
if (line.StartsWith(FILE_MODE_NEW, StringComparison.Ordinal))
241235
{
242-
_result.NewMode = ParseFileMode(line.Substring(9));
236+
_result.NewMode = int.Parse(line.AsSpan(9));
243237
return true;
244238
}
245239

246-
if (line.StartsWith("deleted file mode ", StringComparison.Ordinal))
240+
if (line.StartsWith(FILE_MODE_DELETED, StringComparison.Ordinal))
247241
{
248-
_result.OldMode = ParseFileMode(line.Substring(18));
242+
_result.OldMode = int.Parse(line.AsSpan(18));
249243
return true;
250244
}
251245

252-
if (line.StartsWith("new file mode ", StringComparison.Ordinal))
246+
if (line.StartsWith(FILE_MODE_ADDED, StringComparison.Ordinal))
253247
{
254-
_result.NewMode = ParseFileMode(line.Substring(14));
248+
_result.NewMode = int.Parse(line.AsSpan(14));
255249
return true;
256250
}
257251

258252
return false;
259253
}
260254

261-
private int ParseFileMode(string content)
262-
{
263-
int mode = 0;
264-
int.TryParse(content, out mode);
265-
return mode;
266-
}
267-
268-
private bool ParseLFSChange(char ch, string line)
255+
private bool ParseLFSChange(char prefix, string content)
269256
{
270257
if (_result.IsLFS)
271258
{
272-
if (ch == (char)Indicator.Old)
259+
if (prefix == PREFIX_DELETED)
273260
{
274-
if (line.StartsWith(LFS_OID_PREFIX, StringComparison.Ordinal))
275-
_result.LFSDiff.Old.Oid = line.Substring(11);
276-
else if (line.StartsWith(LFS_SIZE_PREFIX, StringComparison.Ordinal))
277-
_result.LFSDiff.Old.Size = long.Parse(line.AsSpan(5));
261+
if (content.StartsWith(LFS_OID_PREFIX, StringComparison.Ordinal))
262+
_result.LFSDiff.Old.Oid = content.Substring(11);
263+
else if (content.StartsWith(LFS_SIZE_PREFIX, StringComparison.Ordinal))
264+
_result.LFSDiff.Old.Size = long.Parse(content.AsSpan(5));
278265
}
279-
else if (ch == (char)Indicator.New)
266+
else if (prefix == PREFIX_ADDED)
280267
{
281-
if (line.StartsWith(LFS_OID_PREFIX, StringComparison.Ordinal))
282-
_result.LFSDiff.New.Oid = line.Substring(11);
283-
else if (line.StartsWith(LFS_SIZE_PREFIX, StringComparison.Ordinal))
284-
_result.LFSDiff.New.Size = long.Parse(line.AsSpan(5));
268+
if (content.StartsWith(LFS_OID_PREFIX, StringComparison.Ordinal))
269+
_result.LFSDiff.New.Oid = content.Substring(11);
270+
else if (content.StartsWith(LFS_SIZE_PREFIX, StringComparison.Ordinal))
271+
_result.LFSDiff.New.Size = long.Parse(content.AsSpan(5));
285272
}
286-
else if (ch == (char)Indicator.Context)
273+
else if (prefix == PREFIX_CONTEXT)
287274
{
288-
if (line.StartsWith(LFS_SIZE_PREFIX, StringComparison.Ordinal))
289-
_result.LFSDiff.New.Size = _result.LFSDiff.Old.Size = long.Parse(line.AsSpan(5));
275+
if (content.StartsWith(LFS_SIZE_PREFIX, StringComparison.Ordinal))
276+
_result.LFSDiff.New.Size = _result.LFSDiff.Old.Size = long.Parse(content.AsSpan(5));
290277
}
291278
return true;
292279
}
293280

294-
if (_result.TextDiff.Lines.Count != 1)
295-
return false;
296-
297-
if ((_oldLine == 1 && _newLine == 1 && ch == (char)Indicator.Context) ||
298-
(_oldLine == 1 && _newLine == 0 && ch == (char)Indicator.Old) ||
299-
(_oldLine == 0 && _newLine == 1 && ch == (char)Indicator.New))
281+
if ((_oldLine == 1 && _newLine == 1 && prefix == PREFIX_CONTEXT) ||
282+
(_oldLine == 1 && _newLine == 0 && prefix == PREFIX_DELETED) ||
283+
(_oldLine == 0 && _newLine == 1 && prefix == PREFIX_ADDED))
300284
{
301-
if (line.StartsWith(LFS_SPECIFIER, StringComparison.Ordinal))
285+
if (content.StartsWith(LFS_SPECIFIER, StringComparison.Ordinal))
302286
{
303287
_result.IsLFS = true;
304288
_result.LFSDiff = new Models.LFSDiff();

‎src/Models/DiffResult.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ public class TextDiffLine
3333
public string NewLine => NewLineNumber == 0 ? string.Empty : NewLineNumber.ToString();
3434

3535
public TextDiffLine() { }
36-
public TextDiffLine(TextDiffLineType type, string line, byte[] rawContent, int oldLine, int newLine)
36+
public TextDiffLine(TextDiffLineType type, string content, byte[] rawContent, int oldLine, int newLine)
3737
{
3838
Type = type;
39-
Content = line;
39+
Content = content;
4040
RawContent = rawContent;
4141
OldLineNumber = oldLine;
4242
NewLineNumber = newLine;

0 commit comments

Comments
 (0)