Skip to content

Commit 8bbe8bc

Browse files
PureWeenCopilot
andcommitted
fix: address review — eliminate async void and Task.Delay race
- Replace async void SetViewMode with sync method + _pendingEditorInit set. CodeMirror initialization now happens in OnAfterRenderAsync (guaranteed DOM availability) instead of Task.Delay(50) (race-prone). - Add 4 edge case tests: deleted file, empty hunks, multi-hunk, context-only diffs for ReconstructOriginal/ReconstructModified. - Tests: 3136/3136 pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c2fb535 commit 8bbe8bc

2 files changed

Lines changed: 122 additions & 7 deletions

File tree

PolyPilot.Tests/DiffParserTests.cs

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,4 +430,104 @@ public void ReconstructOriginalAndModified_NewFile_ModifiedHasAllLines()
430430
Assert.Contains("line 2", modified);
431431
Assert.Contains("line 3", modified);
432432
}
433+
434+
[Fact]
435+
public void ReconstructOriginalAndModified_DeletedFile_OriginalHasAllLines()
436+
{
437+
var diff = """
438+
diff --git a/old.txt b/old.txt
439+
deleted file mode 100644
440+
--- a/old.txt
441+
+++ /dev/null
442+
@@ -1,3 +0,0 @@
443+
-line A
444+
-line B
445+
-line C
446+
""";
447+
var files = DiffParser.Parse(diff);
448+
var original = DiffParser.ReconstructOriginal(files[0]);
449+
var modified = DiffParser.ReconstructModified(files[0]);
450+
451+
Assert.Contains("line A", original);
452+
Assert.Contains("line B", original);
453+
Assert.Contains("line C", original);
454+
Assert.Equal("", modified);
455+
}
456+
457+
[Fact]
458+
public void ReconstructOriginalAndModified_EmptyHunks_ReturnsEmpty()
459+
{
460+
var file = new DiffFile { FileName = "empty.txt", Hunks = new List<DiffHunk>() };
461+
var original = DiffParser.ReconstructOriginal(file);
462+
var modified = DiffParser.ReconstructModified(file);
463+
464+
Assert.Equal("", original);
465+
Assert.Equal("", modified);
466+
}
467+
468+
[Fact]
469+
public void ReconstructOriginalAndModified_MultipleHunks_CombinesAll()
470+
{
471+
var diff = """
472+
diff --git a/multi.cs b/multi.cs
473+
--- a/multi.cs
474+
+++ b/multi.cs
475+
@@ -1,3 +1,3 @@
476+
using System;
477+
-using Old;
478+
+using New;
479+
class A {}
480+
@@ -10,3 +10,4 @@
481+
void Foo() {
482+
- Bar();
483+
+ Baz();
484+
+ Qux();
485+
}
486+
""";
487+
var files = DiffParser.Parse(diff);
488+
var original = DiffParser.ReconstructOriginal(files[0]);
489+
var modified = DiffParser.ReconstructModified(files[0]);
490+
491+
// Original should have both hunks' context+removed lines
492+
Assert.Contains("using Old;", original);
493+
Assert.Contains("Bar();", original);
494+
Assert.DoesNotContain("using New;", original);
495+
Assert.DoesNotContain("Baz();", original);
496+
Assert.DoesNotContain("Qux();", original);
497+
498+
// Modified should have both hunks' context+added lines
499+
Assert.Contains("using New;", modified);
500+
Assert.Contains("Baz();", modified);
501+
Assert.Contains("Qux();", modified);
502+
Assert.DoesNotContain("using Old;", modified);
503+
Assert.DoesNotContain("Bar();", modified);
504+
}
505+
506+
[Fact]
507+
public void ReconstructOriginalAndModified_ContextOnly_BothIdentical()
508+
{
509+
// A diff with only context lines (no real changes) — e.g., a self-diff view
510+
var file = new DiffFile
511+
{
512+
FileName = "ctx.txt",
513+
Hunks = new List<DiffHunk>
514+
{
515+
new()
516+
{
517+
Lines = new List<DiffLine>
518+
{
519+
new() { Type = DiffLineType.Context, Content = "line 1" },
520+
new() { Type = DiffLineType.Context, Content = "line 2" },
521+
new() { Type = DiffLineType.Context, Content = "line 3" },
522+
}
523+
}
524+
}
525+
};
526+
var original = DiffParser.ReconstructOriginal(file);
527+
var modified = DiffParser.ReconstructModified(file);
528+
529+
Assert.Equal(original, modified);
530+
Assert.Contains("line 1", original);
531+
Assert.Contains("line 3", original);
532+
}
433533
}

PolyPilot/Components/DiffView.razor

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ else
9292
private string? _lastRawDiff;
9393
private readonly Dictionary<int, DiffViewMode> _viewModes = new();
9494
private readonly Dictionary<int, int> _cmInstances = new();
95+
private readonly HashSet<int> _pendingEditorInit = new();
9596
private readonly string _instancePrefix = Guid.NewGuid().ToString("N")[..8];
9697

9798
private enum DiffViewMode { Table, Editor }
@@ -105,22 +106,36 @@ else
105106
_ = DisposeAllCmInstancesAsync();
106107
_viewModes.Clear();
107108
_cmInstances.Clear();
109+
_pendingEditorInit.Clear();
108110
}
109111
}
110112

111-
private async void SetViewMode(int fileIdx, DiffViewMode mode)
113+
private void SetViewMode(int fileIdx, DiffViewMode mode)
112114
{
113-
_viewModes[fileIdx] = mode;
114-
StateHasChanged();
115-
116115
if (mode == DiffViewMode.Editor)
117116
{
118-
await Task.Delay(50);
119-
await InitCodeMirrorForFile(fileIdx);
117+
_viewModes[fileIdx] = mode;
118+
_pendingEditorInit.Add(fileIdx);
119+
// StateHasChanged triggers OnAfterRenderAsync where we init CM
120120
}
121121
else
122122
{
123-
await DisposeCmInstance(fileIdx);
123+
_viewModes[fileIdx] = mode;
124+
_ = DisposeCmInstance(fileIdx);
125+
}
126+
}
127+
128+
protected override async Task OnAfterRenderAsync(bool firstRender)
129+
{
130+
if (_pendingEditorInit.Count > 0)
131+
{
132+
var pending = _pendingEditorInit.ToArray();
133+
_pendingEditorInit.Clear();
134+
135+
foreach (var fileIdx in pending)
136+
{
137+
await InitCodeMirrorForFile(fileIdx);
138+
}
124139
}
125140
}
126141

0 commit comments

Comments
 (0)