Skip to content

Fix continuation action sorting in Fix Common Errors#11522

Merged
niksedk merged 1 commit into
SubtitleEdit:mainfrom
mjuhasz:fix/fix-common-errors-action-sorting
Jun 9, 2026
Merged

Fix continuation action sorting in Fix Common Errors#11522
niksedk merged 1 commit into
SubtitleEdit:mainfrom
mjuhasz:fix/fix-common-errors-action-sorting

Conversation

@mjuhasz

@mjuhasz mjuhasz commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR changes the Fix Common Errors action list so continuation-style fixes sort under a single visible action label while still keeping the two fix variants distinct internally.

Previously, FixContinuationStyle used a trailing-space hack in the action text to distinguish the current-line suffix fix from the next-line prefix fix. That preserved selection/apply behavior, but it also caused sorting by Action to split the rows into two visually identical groups.

In the screenshot below, the grid is sorted by Action, but the numbering restarts midway without any clear reason. This inconsistency prompted my investigation.

Screenshot 2026-06-09 at 9 43 56 PM

What changed

  • Added an internal FixActionKey helper to separate:
    • the internal action identity
    • the user-visible action label
  • Updated FixContinuationStyle to use explicit internal keys for:
    • suffix/current-paragraph fixes
    • prefix/next-paragraph fixes
  • Updated FixDisplayItem to expose:
    • Action as the internal key
    • ActionDisplay as the normalized visible label
  • Updated the Fix Common Errors grid to bind the Action column to ActionDisplay
  • Added a regression test covering the action-key/display split

Result

  • Sorting by Action no longer creates two visually identical continuation-style groups
  • Continuation-style fix rows still remain distinct for selection persistence and apply behavior
  • No user-facing label changes were introduced

Alternative solution

This would have worked too: give the two continuation rows genuinely different action names, such as:

  • Fix continuation style: Custom (suffix)
  • Fix continuation style: Custom (prefix)

or localized equivalents, the same way src/libse/Forms/FixCommonErrors/FixEmptyLines.cs:20 uses distinct labels for top/bottom/middle.

I did not choose that approach because it changes the UI semantics:

  • it exposes an implementation detail the user may not care about
  • it would sort into separate visible groups by design
  • it would require new user-facing/localized strings if done cleanly

The action-key/display split keeps the UI behavior as I expected it:

  • one visible action group: Fix continuation style: Custom
  • two internally distinct rows, so selection/apply still works correctly

So a solution similar to RemovedEmptyLineAtTop pattern was a valid option, but it would have solved the identity problem by making the labels visibly different, whereas this fix solves it without changing the user-facing label.

Separate Fix Common Errors action identity from the user-visible action label so continuation-style rows can remain distinct for selection/apply while sorting together in the grid.

Replace the trailing-space continuation action hack with explicit suffix/prefix action keys, bind the Action column to the normalized display label, and add a regression test covering the key/display split.
@niksedk niksedk merged commit 40f1e77 into SubtitleEdit:main Jun 9, 2026
2 checks passed
@mjuhasz mjuhasz deleted the fix/fix-common-errors-action-sorting branch June 10, 2026 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants