Fix: ColorTextTooLong should not trigger on CPS violations#11325
Open
mjuhasz wants to merge 1 commit into
Open
Fix: ColorTextTooLong should not trigger on CPS violations#11325mjuhasz wants to merge 1 commit into
mjuhasz wants to merge 1 commit into
Conversation
The text cell background was turning red when CPS exceeded the maximum, even though CPS already has its own dedicated column highlight (CpsBackgroundBrush / ColorDurationTooLong). This made it impossible to distinguish line-too-long from CPS-too-high in the text column, and diverged from SE4 behavior where only line length triggers the text background color.
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
1 participant
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.
The text cell background was turning red for CPS violations when "Color text if too long" was enabled. This disrupted my workflow when I migrated from Subtitle Edit 4. In that version, I could easily identify lines that were too long by noticing the text cell turning red, and I could spot lines with excessive CPS by seeing the CPS cell in red. In Subtitle Edit 5, the text cell turned red for both CPS violations and line length violations. I consider this incorrect for two reasons:
Inconsistent with Subtitle Edit 4:
ListViewSyntaxColorLongLinesin Subtitle Edit 4 checks only line length (and line count). CPS violations have always been indicated via the Duration/CPS columns, not the text cell.Redundant: CPS violations already have a dedicated visual indicator — the CPS column is highlighted via
CpsBackgroundBrush(and the Duration column too), both gated byColorDurationTooLong. Lighting up the text cell as well conflates two unrelated conditions under one setting, making it impossible to tell from the text background alone whether a subtitle is too long or just too fast.The fix removes the CPS check from
TextBackgroundBrush. TheColorTextTooLongsetting now correctly reflects only what its name says: lines that exceed the maximum character length.