fix(ui): offload UnifiedDiffParser.Parse off the UI thread in DiffViewerViewModel
Both call sites (LoadFilesAsync, OnDisplayedDiffChanged) ran the parser synchronously on the dispatcher, freezing the window on a large diff. The Files-mode parse+tree-build now runs via Task.Run behind a ParseOp OperationStatus wired to the toolbar; the planning-mode parse is fire-and-forget with a generation counter so a fast second DisplayedDiff change can't write stale PlanningFiles.
This commit is contained in:
@@ -698,6 +698,9 @@
|
||||
"merging": "Wird zusammengeführt…",
|
||||
"verifying": "Verify-Kommando der Liste läuft… ({0})"
|
||||
},
|
||||
"diff": {
|
||||
"parsing": "Diff wird geparst…"
|
||||
},
|
||||
"review": {
|
||||
"submitting": "Wird zur Prüfung eingereicht…",
|
||||
"rejecting": "Wird abgelehnt…",
|
||||
|
||||
@@ -698,6 +698,9 @@
|
||||
"merging": "Merging…",
|
||||
"verifying": "Running the list's verify command… ({0})"
|
||||
},
|
||||
"diff": {
|
||||
"parsing": "Parsing diff…"
|
||||
},
|
||||
"review": {
|
||||
"submitting": "Submitting for review…",
|
||||
"rejecting": "Rejecting…",
|
||||
|
||||
@@ -98,6 +98,14 @@ public sealed partial class DiffViewerViewModel : ViewModelBase
|
||||
|
||||
public Action? CloseAction { get; set; }
|
||||
|
||||
// Parsing a large diff (UnifiedDiffParser.Parse + DiffTree.Build, both pure CPU work) is
|
||||
// offloaded to a threadpool thread so it never blocks the UI dispatcher.
|
||||
public OperationStatus ParseOp { get; } = new();
|
||||
|
||||
// Guards the fire-and-forget planning parse below against a stale write: a second
|
||||
// DisplayedDiff change while the first parse is still running must not clobber PlanningFiles.
|
||||
private int _planningFilesGeneration;
|
||||
|
||||
public DiffViewerViewModel(GitService git, IWorkerClient worker, AppSettings settings)
|
||||
{
|
||||
_git = git;
|
||||
@@ -179,8 +187,18 @@ public sealed partial class DiffViewerViewModel : ViewModelBase
|
||||
return;
|
||||
}
|
||||
|
||||
var files = UnifiedDiffParser.Parse(raw).ToList();
|
||||
foreach (var node in DiffTree.Build(files))
|
||||
List<DiffFileViewModel> files;
|
||||
List<DiffTreeNodeViewModel> tree;
|
||||
using (ParseOp.Begin(Loc.T("ops.diff.parsing")))
|
||||
{
|
||||
(files, tree) = await Task.Run(() =>
|
||||
{
|
||||
var parsed = UnifiedDiffParser.Parse(raw);
|
||||
return (parsed, DiffTree.Build(parsed));
|
||||
}, ct);
|
||||
}
|
||||
|
||||
foreach (var node in tree)
|
||||
FileTree.Add(node);
|
||||
|
||||
SelectedNode = DiffTree.FirstLeaf(FileTree);
|
||||
@@ -254,10 +272,24 @@ public sealed partial class DiffViewerViewModel : ViewModelBase
|
||||
|
||||
partial void OnIsCombinedModeChanged(bool value) => ToggleCombinedCommand.Execute(null);
|
||||
|
||||
// DisplayedDiff is set from four places (OnSelectedSubtaskChanged plus three branches of
|
||||
// ToggleCombinedAsync), and OnSelectedSubtaskChanged is itself a synchronous, non-awaitable
|
||||
// callback — funneling the offload through this single change handler covers all of them
|
||||
// without touching every call site. Fire-and-forget is safe because the generation counter
|
||||
// below discards a parse whose result is no longer the current DisplayedDiff.
|
||||
partial void OnDisplayedDiffChanged(string value)
|
||||
{
|
||||
var generation = ++_planningFilesGeneration;
|
||||
_ = ApplyParsedPlanningFilesAsync(value, generation);
|
||||
}
|
||||
|
||||
private async Task ApplyParsedPlanningFilesAsync(string diff, int generation)
|
||||
{
|
||||
var files = await Task.Run(() => UnifiedDiffParser.Parse(diff));
|
||||
if (generation != _planningFilesGeneration) return;
|
||||
|
||||
PlanningFiles.Clear();
|
||||
foreach (var file in UnifiedDiffParser.Parse(value))
|
||||
foreach (var file in files)
|
||||
PlanningFiles.Add(file);
|
||||
}
|
||||
|
||||
|
||||
@@ -48,6 +48,7 @@
|
||||
ToolTip.Tip="{loc:Tr modals.diff.wrapLines}">
|
||||
<PathIcon Data="{StaticResource Icon.Wrap}" Width="14" Height="14"/>
|
||||
</ToggleButton>
|
||||
<ctl:OperationIndicator Status="{Binding ParseOp}"/>
|
||||
</StackPanel>
|
||||
|
||||
<!-- Planning toolbar: combined-mode toggle + warning/loading -->
|
||||
|
||||
@@ -140,6 +140,33 @@ public class DiffViewerViewModelTests
|
||||
Assert.Equal("DIFF-B", vm.DisplayedDiff);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Planning_SelectSubtask_PopulatesPlanningFilesAsync()
|
||||
{
|
||||
const string diff = "diff --git a/foo.cs b/foo.cs\n" +
|
||||
"--- a/foo.cs\n" +
|
||||
"+++ b/foo.cs\n" +
|
||||
"@@ -1,1 +1,1 @@\n" +
|
||||
"-old\n" +
|
||||
"+new\n";
|
||||
var fake = new FakePlanningWorker
|
||||
{
|
||||
AggregateResult = new[] { new SubtaskDiffDto("s1", "First", "b1", "base1", "head1", null, diff) }
|
||||
};
|
||||
var vm = new DiffViewerViewModel(null!, fake, TestSettings());
|
||||
vm.ConfigurePlanning("plan-1", "main");
|
||||
|
||||
await vm.LoadAsync();
|
||||
|
||||
// OnDisplayedDiffChanged offloads the parse via Task.Run and writes PlanningFiles back
|
||||
// fire-and-forget, so it may not have landed yet when LoadAsync returns.
|
||||
var deadline = DateTime.UtcNow.AddSeconds(5);
|
||||
while (DateTime.UtcNow < deadline && vm.PlanningFiles.Count == 0) await Task.Delay(10);
|
||||
|
||||
var file = Assert.Single(vm.PlanningFiles);
|
||||
Assert.Equal("foo.cs", file.Path);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Planning_ToggleCombined_Success_DisplaysUnifiedDiff()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user