Skip to content

Commit b4ed5d8

Browse files
NainetenNaineten
Naineten
authored and
Naineten
committed
fix(ofpa): eliminate filename popping in history and stash views
- Refactor OFPA decoding in CommitDetail and StashesPage to be fully async and atomic. - Prevent UI updates until decoding is complete to avoid visual artifacts. - Ensure thread safety and handle race conditions during rapid navigation.
1 parent f78a6c9 commit b4ed5d8

2 files changed

Lines changed: 114 additions & 103 deletions

File tree

‎src/ViewModels/CommitDetail.cs‎

Lines changed: 57 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -454,19 +454,26 @@ private void Refresh()
454454

455455
if (!token.IsCancellationRequested)
456456
{
457-
Dispatcher.UIThread.Post(() =>
457+
Dictionary<string, string> decodedPaths = null;
458+
if (_repo.Settings.EnableUnrealEngineSupport && _repo.Settings.EnableOFPADecoding)
459+
decodedPaths = await CalculateDecodedPathsAsync(changes).ConfigureAwait(false);
460+
461+
if (!token.IsCancellationRequested)
458462
{
459-
Changes = changes;
460-
VisibleChanges = visible;
463+
Dispatcher.UIThread.Post(() =>
464+
{
465+
_decodedPaths = decodedPaths;
466+
OnPropertyChanged(nameof(DecodedPaths));
461467

462-
if (visible.Count == 0)
463-
SelectedChanges = null;
464-
else
465-
SelectedChanges = [VisibleChanges[0]];
468+
Changes = changes;
469+
VisibleChanges = visible;
466470

467-
if (_repo.Settings.EnableUnrealEngineSupport && _repo.Settings.EnableOFPADecoding)
468-
_currentDecodeTask = DecodeOFPAPathsAsync(changes);
469-
});
471+
if (visible.Count == 0)
472+
SelectedChanges = null;
473+
else
474+
SelectedChanges = [VisibleChanges[0]];
475+
});
476+
}
470477
}
471478
}, token);
472479
}
@@ -689,60 +696,60 @@ private async Task DecodeOFPAPathsAsync(List<Models.Change> changes)
689696
changes == null || changes.Count == 0)
690697
return;
691698

692-
var repositoryPath = _repo.FullPath;
693-
var parent = _commit.Parents.Count == 0 ? Models.Commit.EmptyTreeSHA1 : $"{_commit.SHA}^";
694-
var results = new Dictionary<string, string>(StringComparer.Ordinal);
695-
696-
await Task.Run(async () =>
699+
_currentDecodeTask = Task.Run(async () =>
697700
{
698-
var filesToDecode = new List<(string RelativePath, string Spec)>();
699-
foreach (var change in changes)
701+
var results = await CalculateDecodedPathsAsync(changes).ConfigureAwait(false);
702+
if (results != null)
700703
{
701-
if (Utilities.OFPAParser.IsOFPAFile(change.Path))
704+
await Dispatcher.UIThread.InvokeAsync(() =>
702705
{
703-
// Use heuristic to determine revision:
704-
// - Deleted files (WorkTree or Index) -> fetch from Parent commit.
705-
// - Added/Modified files -> fetch from Current commit.
706-
var spec = (change.WorkTree == Models.ChangeState.Deleted || change.Index == Models.ChangeState.Deleted)
707-
? $"{parent}:{change.Path}"
708-
: $"{_commit.SHA}:{change.Path}";
709-
filesToDecode.Add((change.Path, spec));
710-
}
706+
_decodedPaths = results;
707+
OnPropertyChanged(nameof(DecodedPaths));
708+
});
711709
}
710+
});
711+
}
712712

713-
if (filesToDecode.Count == 0)
714-
return;
713+
private async Task<Dictionary<string, string>> CalculateDecodedPathsAsync(List<Models.Change> changes)
714+
{
715+
if (_repo == null || _commit == null || changes == null || changes.Count == 0)
716+
return null;
715717

716-
var batchRequests = new List<string>();
717-
foreach (var entry in filesToDecode)
718-
batchRequests.Add(entry.Spec);
718+
var repositoryPath = _repo.FullPath;
719+
var parent = _commit.Parents.Count == 0 ? Models.Commit.EmptyTreeSHA1 : $"{_commit.SHA}^";
720+
var filesToDecode = new List<(string RelativePath, string Spec)>();
719721

720-
var batchResults = await Commands.QueryFileContent.RunBatchAsync(repositoryPath, batchRequests, MaxOFPASampleSize).ConfigureAwait(false);
721-
foreach (var entry in filesToDecode)
722+
foreach (var change in changes)
723+
{
724+
if (Utilities.OFPAParser.IsOFPAFile(change.Path))
722725
{
723-
if (batchResults.TryGetValue(entry.Spec, out var data))
724-
{
725-
var decoded = Utilities.OFPAParser.DecodeFromData(data);
726-
results[entry.RelativePath] = decoded?.LabelValue;
727-
}
726+
var spec = (change.WorkTree == Models.ChangeState.Deleted || change.Index == Models.ChangeState.Deleted)
727+
? $"{parent}:{change.Path}"
728+
: $"{_commit.SHA}:{change.Path}";
729+
filesToDecode.Add((change.Path, spec));
728730
}
731+
}
729732

730-
var updated = new Dictionary<string, string>(StringComparer.Ordinal);
731-
foreach (var kvp in results)
732-
updated[kvp.Key] = kvp.Value;
733-
_decodedPaths = updated;
734-
});
733+
if (filesToDecode.Count == 0)
734+
return null;
735+
736+
var batchRequests = new List<string>();
737+
foreach (var entry in filesToDecode)
738+
batchRequests.Add(entry.Spec);
735739

736-
if (results.Count > 0)
740+
var batchResults = await Commands.QueryFileContent.RunBatchAsync(repositoryPath, batchRequests, MaxOFPASampleSize).ConfigureAwait(false);
741+
var results = new Dictionary<string, string>(StringComparer.Ordinal);
742+
foreach (var entry in filesToDecode)
737743
{
738-
await Dispatcher.UIThread.InvokeAsync(() =>
744+
if (batchResults.TryGetValue(entry.Spec, out var data))
739745
{
740-
if (_repo == null || !_repo.Settings.EnableUnrealEngineSupport || !_repo.Settings.EnableOFPADecoding)
741-
return;
742-
743-
OnPropertyChanged(nameof(DecodedPaths));
744-
});
746+
var decoded = Utilities.OFPAParser.DecodeFromData(data);
747+
if (decoded.HasValue)
748+
results[entry.RelativePath] = decoded.Value.LabelValue;
749+
}
745750
}
751+
752+
return results;
746753
}
747754

748755
private Repository _repo = null;

‎src/ViewModels/StashesPage.cs‎

Lines changed: 57 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,19 @@ public Models.Stash SelectedStash
8484
changes.Sort((l, r) => Models.NumericSort.Compare(l.Path, r.Path));
8585
}
8686

87+
Dictionary<string, string> decodedPaths = null;
88+
if (_repo.Settings.EnableUnrealEngineSupport && _repo.Settings.EnableOFPADecoding)
89+
decodedPaths = await CalculateDecodedPathsAsync(value, changes, untracked).ConfigureAwait(false);
90+
8791
Dispatcher.UIThread.Post(() =>
8892
{
8993
if (value.SHA.Equals(_selectedStash?.SHA ?? string.Empty, StringComparison.Ordinal))
9094
{
95+
_decodedPaths = decodedPaths;
96+
OnPropertyChanged(nameof(DecodedPaths));
97+
9198
_untracked = untracked;
9299
Changes = changes;
93-
94-
if (_repo.Settings.EnableUnrealEngineSupport && _repo.Settings.EnableOFPADecoding)
95-
_currentDecodeTask = DecodeOFPAPathsAsync(value, changes, untracked);
96100
}
97101
});
98102
});
@@ -306,70 +310,70 @@ private async Task DecodeOFPAPathsAsync(Models.Stash stash, List<Models.Change>
306310
changes == null || changes.Count == 0)
307311
return;
308312

309-
var repositoryPath = _repo.FullPath;
310-
var untrackedSet = new HashSet<Models.Change>(untracked);
311-
var results = new Dictionary<string, string>(StringComparer.Ordinal);
312-
313-
await Task.Run(async () =>
313+
_currentDecodeTask = Task.Run(async () =>
314314
{
315-
var filesToDecode = new List<(string RelativePath, string Spec)>();
316-
foreach (var change in changes)
315+
var results = await CalculateDecodedPathsAsync(stash, changes, untracked).ConfigureAwait(false);
316+
if (results != null)
317317
{
318-
if (!Utilities.OFPAParser.IsOFPAFile(change.Path))
319-
continue;
320-
321-
string spec;
322-
if (untrackedSet.Contains(change) && stash.Parents.Count == 3)
318+
await Dispatcher.UIThread.InvokeAsync(() =>
323319
{
324-
// Untracked files are in the 3rd parent commit.
325-
spec = $"{stash.Parents[2]}:{change.Path}";
326-
}
327-
else
328-
{
329-
// Standard stash changes (index + worktree).
330-
// Deleted files need to be looked up in the parent.
331-
if (change.WorkTree == Models.ChangeState.Deleted || change.Index == Models.ChangeState.Deleted)
332-
spec = $"{stash.Parents[0]}:{change.Path}";
333-
else
334-
spec = $"{stash.SHA}:{change.Path}";
335-
}
336-
337-
filesToDecode.Add((change.Path, spec));
320+
_decodedPaths = results;
321+
OnPropertyChanged(nameof(DecodedPaths));
322+
});
338323
}
324+
});
325+
}
326+
327+
private async Task<Dictionary<string, string>> CalculateDecodedPathsAsync(Models.Stash stash, List<Models.Change> changes, List<Models.Change> untracked)
328+
{
329+
if (_repo == null || stash == null || changes == null || changes.Count == 0)
330+
return null;
339331

340-
if (filesToDecode.Count == 0)
341-
return;
332+
var repositoryPath = _repo.FullPath;
333+
var untrackedSet = new HashSet<Models.Change>(untracked);
334+
var filesToDecode = new List<(string RelativePath, string Spec)>();
342335

343-
var batchRequests = new List<string>();
344-
foreach (var entry in filesToDecode)
345-
batchRequests.Add(entry.Spec);
336+
foreach (var change in changes)
337+
{
338+
if (!Utilities.OFPAParser.IsOFPAFile(change.Path))
339+
continue;
346340

347-
var batchResults = await Commands.QueryFileContent.RunBatchAsync(repositoryPath, batchRequests, MaxOFPASampleSize).ConfigureAwait(false);
348-
foreach (var entry in filesToDecode)
341+
string spec;
342+
if (untrackedSet.Contains(change) && stash.Parents.Count == 3)
349343
{
350-
if (batchResults.TryGetValue(entry.Spec, out var data))
351-
{
352-
var decoded = Utilities.OFPAParser.DecodeFromData(data);
353-
results[entry.RelativePath] = decoded?.LabelValue;
354-
}
344+
spec = $"{stash.Parents[2]}:{change.Path}";
345+
}
346+
else
347+
{
348+
if (change.WorkTree == Models.ChangeState.Deleted || change.Index == Models.ChangeState.Deleted)
349+
spec = $"{stash.Parents[0]}:{change.Path}";
350+
else
351+
spec = $"{stash.SHA}:{change.Path}";
355352
}
356353

357-
var updated = new Dictionary<string, string>(StringComparer.Ordinal);
358-
foreach (var kvp in results)
359-
updated[kvp.Key] = kvp.Value;
360-
_decodedPaths = updated;
361-
});
354+
filesToDecode.Add((change.Path, spec));
355+
}
356+
357+
if (filesToDecode.Count == 0)
358+
return null;
359+
360+
var batchRequests = new List<string>();
361+
foreach (var entry in filesToDecode)
362+
batchRequests.Add(entry.Spec);
362363

363-
if (results.Count > 0)
364+
var batchResults = await Commands.QueryFileContent.RunBatchAsync(repositoryPath, batchRequests, MaxOFPASampleSize).ConfigureAwait(false);
365+
var results = new Dictionary<string, string>(StringComparer.Ordinal);
366+
foreach (var entry in filesToDecode)
364367
{
365-
await Dispatcher.UIThread.InvokeAsync(() =>
368+
if (batchResults.TryGetValue(entry.Spec, out var data))
366369
{
367-
if (_repo == null || !_repo.Settings.EnableUnrealEngineSupport || !_repo.Settings.EnableOFPADecoding)
368-
return;
369-
370-
OnPropertyChanged(nameof(DecodedPaths));
371-
});
370+
var decoded = Utilities.OFPAParser.DecodeFromData(data);
371+
if (decoded.HasValue)
372+
results[entry.RelativePath] = decoded.Value.LabelValue;
373+
}
372374
}
375+
376+
return results;
373377
}
374378

375379
private Repository _repo = null;

0 commit comments

Comments
 (0)