ASSA: new lines keep the neighbouring style instead of the first one in the header - #13686
Merged
Conversation
Fixes #13677: a line inserted into an ASSA file adopted whichever style sat first in the header, so a file whose "Default" style was not listed first got every new line in a foreign style. Two causes: - The waveform and video-position inserts went straight to InsertInCorrectPosition, which has no style logic, so the line was left with an empty style. AdvancedSubStationAlpha.ToText then resolves an empty Extra to styles[0] on save. SE 4 called SetStyleForNewParagraph on these paths; the SE 5 port dropped it. - The insert paths that did set a style copied the neighbour's Extra and then overwrote Style with the first header style. SE 4 ended at "newParagraph.Extra = c.Extra" - the override line is new in SE 5. The TTML branch still inherits correctly, so ASSA was the outlier. GetStyleNameForNewParagraph now picks, in order: the neighbouring line's style, the style flagged as default in the styles storage, a style named "Default", and only then the first style in the header. The storage default flag used to be ignored the moment a file had styles of its own. Also seed the ASSA and SSA style storages with the built-in "Default" style on first start and after a settings reset, so "Styles saved" is not empty and there is something to make default. The seeded style matches libse's built-in default header, so a fresh install converts to ASSA exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
InsertAfter asks for index 1 on an empty subtitle, which threw ArgumentOutOfRangeException, and both InsertBefore and InsertAfter threw on a selection index past the end. Every caller happens to guard against it today, so this was latent - clamp to the collection bounds so the methods append instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #13677.
A line inserted into an ASSA file adopted whichever style sat first in the header, so a file whose
Defaultstyle was not listed first got every new line in a foreign style. In the reported file the style order isJulia Lepetit, Jacob Andrews, Both, Default—Defaultcarries all 2369 lines but sits last, so new lines came out asJulia Lepetit.Two causes
The waveform and video-position inserts never assigned a style. They go straight to
InsertInCorrectPosition, which has no style logic, so the line was left with an empty style — visible as a blank Style cell in the reporter's video.AdvancedSubStationAlpha.ToTextthen resolves an emptyExtratostyles[0]on save, even when the header does contain a style namedDefault. SE 4 calledSetStyleForNewParagraphon these paths; the SE 5 port dropped it.The paths that did set a style threw the neighbour away.
InsertServicecopied the neighbour'sExtraand then overwroteStylewith the first header style. SE 4's ASSA branch ended atnewParagraph.Extra = c.Extra— the override line is new in SE 5. The TTML branch right above still inherits correctly, so ASSA was the outlier.Changes
SetDefaultAssaStyleForNewParagraphtakes the index the line landed at and is now called from the nine insert paths that bypassIInsertService— waveform selection inserts, video-position inserts, waveform paste, split-at-position, and both speech-to-text paths (those two inherit the style of the line they replace). It no-ops when the line already has a style.GetStyleNameForNewParagraphpicks, in order: the neighbouring line's style, the style flagged as default in the styles storage, a style namedDefault, and only then the first style in the header. The storage's "Set style as default" flag used to be ignored the moment a file had styles of its own — the second half of the report.Defaultstyle on first start and after a settings reset, so "Styles saved" is not empty and there is something to make default. The seeded style matches libse's built-in default header, so a fresh install converts to ASSA exactly as before.InsertServiceclamps the insert index.InsertAfterasked for index 1 on an empty subtitle and threw; every caller happens to guard against it today, so it was latent.Tests
tests/UI/Logic/InsertServiceAssaStyleTests.cs(new) drives the realInsertServicewith the reporter's style list: neighbour inheritance both directions, a blank neighbour falling through toDefault, the written file's Dialogue fields, SubRip left untouched, and the clamped insert positions.AssaStyleStorageHelperTestscovers the new resolution order plus two seeding invariants: the seeded style produces the same styles as libse's built-in default header, and a save/load round-trip keeps one style — proving deserialization replaces the seeded list rather than appending, which would otherwise give every existing user a duplicateDefaulton each launch.Full UI suite: 2824 passed, 0 failed.
🤖 Generated with Claude Code