Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions BACKLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ Two items jump the queue by owner decision. Full reasoning in `decisions.md`; th

- [ ] **N38 The current line's box keeps its pixels while its contents scale, so a large interface loses a control off the bottom (filed 2026-09-06).** The top block opens at a pixel count and an interface size change deliberately does not move it: `interface-scale.spec.js`'s "leaves the three panels at the proportions the sashes were left at when the size changes" asserts `bigger.block === left.block`, and its reason is the waveform, whose measurements are in device pixels because a peak bucket is one millisecond. Everything inside the panel is in rem and does scale. So at 150 per cent in the narrowest window the panel is 1.5 times fuller in a box that has not moved, and the text box needs the panel's scroll to be reached. It has been paid for three times by raising the block's opening height, from 13.5rem to 21rem as the panel gained the effect, the drawing order, the three margins, Next line and the style dropdown, and raising it cannot fix the 150 per cent case because the shortfall there grows with the size. The two candidates are scaling the stored heights with the interface size, which contradicts that criterion and needs the owner, and the reference's own answer, which is merging and splitting the panel's rows by width (edit-bar-tasks.md question 1).

- [ ] **N39 The font button of the current line's row three, and the two halves of the colour picker that are not built (filed 2026-09-06).** Row three names nine buttons before Next line: four style flags, a font, and four colours. The flags and the colours are built; the font is not, and it is the one that needs something this repository does not have, a list of the fonts installed on the machine. Beside it two halves of the colour picker are missing. The alpha: a colour is written with its transparency beside it, `\1a` to `\4a`, and only when that transparency changed, which is two tags in one undo step and `Edit::SetOverrideTag` writes one. The spectrum: the picker offers sixteen colours and a field that takes `#RRGGBB`, where a saturation square, a hue slider, the three number notations and a screen eyedropper belong. The alpha and the font want the same thing first, a planner that writes a list of tags as one step.

- [ ] **N37 The scaled surface check misses the doubling by eight pixels, about one run in several (filed 2026-09-06).** `e2e/scripts/scaled-surface-check.js` asserts the video surface doubles when `GDK_SCALE` doubles, within three pixels. On CI job 101433553363 it read 346x166 at ratio 1 and 700x342 at ratio 2, which is eight and ten pixels over twice, and the same script printed `5/5 checks` on the run before it with nothing changed between them. The failure message reads as though the surface had not been resolved to native pixels, which is not what these numbers say: the scale is applied, and the layout under it settles a few pixels away from where it settles at ratio 1. The candidates are the ruler band, whose height is a rounded number of device pixels and therefore not exactly half at ratio 2, and the surface being measured before the last layout pass at a size where that pass takes longer. **Second sighting, 2026-09-06, and it says the difference is fixed rather than random.** Job 101441553639 on a build whose top block is taller read 346x230 at ratio 1 and 700x470 at ratio 2. The first sighting read 346x166 and 700x342. The heights differ between the two runs and **the shortfall does not**: eight pixels of width and ten of height over twice, both times. That is the shape of a fixed inset in device pixels that is applied once at each ratio instead of scaling with it, not of a layout that settles late, and it rules out the ruler band, whose height does change between those runs. The script still prints only the surface. The way to name it is to print the stage rectangle the page sends and the rectangle the backend applies, at both ratios, and see which of the two carries the eight and the ten.

- [ ] **N36 A media with no picture sometimes says "Open a video first." to the translator, on the runner and not here (filed 2026-09-06).** `video-aspect.spec.js`'s "says a media with no picture has none, and is as quiet about it as about no audio" failed on CI job 101422670879 with `["Open a video first."]` where it expects no alert at all, at `video-aspect.spec.js:281`. That string is `video.errors.notLoaded`, which the status bar draws from `useVideoPlayer`'s error code, and the backend answers `NotLoaded` for mpv's `PropertyUnavailable` as well as for a genuinely closed player (`src-tauri/src/video/error.rs:81`). So a property read that lands in the window around an open is shown to a person as an instruction to do the thing they just did. This is the twin of the audio fix of 2026-09-05: a media with no video track is not an error and must be as quiet as a machine with no sound. The fix is to find which call answers it and stop that answer reaching the status bar, and the way to prove it is to force the answer and watch the check go red.
Expand Down
95 changes: 95 additions & 0 deletions crates/sublore-edit/src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ pub enum Edit {
from: usize,
to: usize,
},
/// Write one override tag with a value the caller chose, over the same stretch a style toggle
/// works on. The pickers use this where the four flags use `ToggleStyle`: a colour is picked
/// rather than flipped, so there is no state to read first. See edit-bar-tasks.md B12.
SetOverrideTag {
cue: usize,
tag: String,
value: String,
from: usize,
to: usize,
},
/// Turn an ASS event into a `Comment:` or back into a `Dialogue:`. The descriptor is not one
/// of the fields `AssField` can name, and this changes how many cues a player would draw, so it
/// is its own edit. See edit-bar-tasks.md B8.
Expand Down Expand Up @@ -148,6 +158,13 @@ pub fn plan(document: &SubtitleDocument, edit: &Edit) -> Result<Planned, EditErr
from,
to,
} => plan_toggle_style(document, *cue, *flag, *from, *to),
Edit::SetOverrideTag {
cue,
tag,
value,
from,
to,
} => plan_set_override_tag(document, *cue, tag, value, *from, *to),
Edit::Insert {
before,
start_ms,
Expand Down Expand Up @@ -1105,6 +1122,84 @@ fn validate_field_value(field: AssField, value: &str) -> Result<(), EditError> {
/// The flag's state at the caret, then the opposite of it written there, and the state it had put
/// back at the far end of the selection shifted by whatever the first write inserted. That is the
/// whole of it, and it is why the writer returns a shift.
/// The same write a style toggle makes, with the value given rather than worked out. A tag name
/// that is not a backslash and letters is refused: everything downstream reads a name that way, and
/// a value carrying a brace would close the block it was written into.
fn plan_set_override_tag(
document: &SubtitleDocument,
index: usize,
tag: &str,
value: &str,
from: usize,
to: usize,
) -> Result<Planned, EditError> {
let named = tag.strip_prefix('\\').unwrap_or("");
// One digit may lead, because the numbered colours and alphas are spelt `\\2c` and `\\1a`, and
// after it the name is letters to the end: whatever follows those is the value.
let letters = named
.strip_prefix(|first: char| first.is_ascii_digit())
.unwrap_or(named);
if letters.is_empty() || !letters.bytes().all(|byte| byte.is_ascii_alphabetic()) {
return Err(EditError::new(
EditErrorKind::NotApplicable,
format!(
"{tag} is not a tag name: a name is a backslash, one digit at most, then letters"
),
));
}
if value.contains(['{', '}', '\\']) {
return Err(EditError::new(
EditErrorKind::NotApplicable,
"a tag value may not carry a brace or a backslash",
));
}
let located = locate(document, index)?;
if !matches!(&located.cue.detail, CueDetail::Ass(_)) {
return Err(EditError::new(
EditErrorKind::NotApplicable,
"only an ASS event carries override tags",
));
}
let text = document.slice(located.cue.text);
if from > text.len()
|| to > text.len()
|| !text.is_char_boundary(from)
|| !text.is_char_boundary(to)
{
return Err(EditError::new(
EditErrorKind::NotApplicable,
format!("the range {from}..{to} is outside the cue's text or cuts a character"),
));
}
let (start, _) = if from <= to { (from, to) } else { (to, from) };
let (written, _) = override_tags::set_tag(text, start, tag, value);

let write = plan_text_write(document, &located, &written)?;
Ok(Planned {
splice: Splice::new(
write.range.start,
document.slice(write.range).to_owned(),
write.inserted,
),
label: EditLabel {
kind: EditKind::SetOverrideTag,
cue: index,
},
expect: Expectation {
from: index,
removed: 1,
cues: vec![ExpectedCue {
text_raw: write.written,
start_ms: located.cue.start.millis(),
end_ms: located.cue.end.millis(),
}],
segments_from: located.segment_index,
segments_removed: 1,
segments_inserted: 1,
},
})
}

fn plan_toggle_style(
document: &SubtitleDocument,
index: usize,
Expand Down
3 changes: 3 additions & 0 deletions crates/sublore-edit/src/splice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ pub enum EditKind {
/// Which flag is on the label, for the reason `SetField` carries its field: bold and italic on
/// one line must never merge into one undo step.
ToggleStyle(sublore_formats::override_tags::StyleFlag),
/// A tag written with a value the caller chose. One kind rather than one per tag, unlike the
/// flag above: the picker commits on the pick, so every pick opens its own step.
SetOverrideTag,
Insert,
Delete,
Split,
Expand Down
61 changes: 61 additions & 0 deletions crates/sublore-edit/tests/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,67 @@ fn a_field_committed_as_whitespace_writes_nothing_however_often_it_is_committed(
);
}

fn set_override_tag(cue: usize, tag: &str, value: &str, from: usize, to: usize) -> Edit {
Edit::SetOverrideTag {
cue,
tag: tag.to_owned(),
value: value.to_owned(),
from,
to,
}
}

#[test]
fn a_tag_with_a_chosen_value_is_written_where_the_caret_is() {
// B12: a colour is picked rather than flipped, so the value comes from the caller.
let mut session = session("ass/clean/basic.ass");
let text = raw_text(&session, 0);
session
.apply(
&set_override_tag(0, "\\c", "&H0000FF&", 0, 0),
Run::New,
Instant::now(),
)
.expect("an ASS event takes an override tag");
assert_eq!(raw_text(&session, 0), format!("{{\\c&H0000FF&}}{text}"));

session.undo().expect("a step to undo").expect("a patch");
assert_eq!(raw_text(&session, 0), text);
}

#[test]
fn a_numbered_colour_replaces_the_one_already_in_the_block_rather_than_joining_it() {
// B12: `\\2c` is one name, so a second pick of the same colour is not a second tag.
let mut session = session("ass/clean/basic.ass");
let text = raw_text(&session, 0);
for value in ["&H0000FF&", "&H00FF00&"] {
session
.apply(
&set_override_tag(0, "\\2c", value, 0, 0),
Run::New,
Instant::now(),
)
.expect("an ASS event takes a numbered colour");
}
assert_eq!(raw_text(&session, 0), format!("{{\\2c&H00FF00&}}{text}"));
}

#[test]
fn a_tag_name_that_is_not_a_name_and_a_value_that_could_close_a_block_are_both_refused() {
let mut session = session("ass/clean/basic.ass");
let before = session.to_bytes();
for (tag, value) in [("c", "&H0&"), ("\\1c1", "&H0&"), ("\\c", "&H0&}x{\\b1")] {
session
.apply(
&set_override_tag(0, tag, value, 0, 0),
Run::New,
Instant::now(),
)
.expect_err("neither a bare name nor a value carrying a brace is written");
}
assert_eq!(session.to_bytes(), before, "a refusal writes nothing");
}

fn toggle_style(cue: usize, flag: StyleFlag, from: usize, to: usize) -> Edit {
Edit::ToggleStyle {
cue,
Expand Down
37 changes: 32 additions & 5 deletions crates/sublore-formats/src/override_tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,21 @@ pub struct Block {
pub span: Span,
}

/// Whether a braced run holds a tag. A tag is a backslash followed by a letter, and a run with none
/// is a comment: `{note}` is a note and `{\b1}` is styling.
/// Whether a braced run holds a tag. A tag is a backslash and a name, and a run with none is a
/// comment: `{note}` is a note and `{\b1}` is styling.
fn holds_a_tag(inside: &str) -> bool {
let bytes = inside.as_bytes();
bytes
.iter()
.enumerate()
.any(|(at, byte)| *byte == b'\\' && bytes.get(at + 1).is_some_and(u8::is_ascii_alphabetic))
.any(|(at, byte)| *byte == b'\\' && names_a_tag(bytes, at))
}

/// Whether the backslash at `at` opens a name: one digit at most, then at least one letter. The
/// digit is there because the numbered colours and alphas are spelt `\2c` and `\1a`.
fn names_a_tag(bytes: &[u8], at: usize) -> bool {
let letters = at + 1 + usize::from(bytes.get(at + 1).is_some_and(u8::is_ascii_digit));
bytes.get(letters).is_some_and(u8::is_ascii_alphabetic)
}

/// The drawing scale a braced run leaves behind it: the last `\p<digits>` in it, or `None` when it
Expand Down Expand Up @@ -185,13 +192,14 @@ pub fn tags_in(text: &str, block: Block) -> Vec<Tag> {
at += 1;
continue;
}
let mut after = at + 1;
let letters = at + 1 + usize::from(bytes.get(at + 1).is_some_and(u8::is_ascii_digit));
let mut after = letters;
while after < end && bytes[after].is_ascii_alphabetic() {
after += 1;
}
// A backslash with no letter after it is not a tag: `\\N` is a line break and its letter is
// taken by the name, which is right, and a trailing backslash names nothing.
if after == at + 1 {
if after == letters {
at += 1;
continue;
}
Expand Down Expand Up @@ -550,4 +558,23 @@ mod tests {
]
);
}

#[test]
fn a_numbered_colour_is_one_name_and_not_a_digit_before_a_value() {
let text = "{\\2c&H0000FF&}word";
let parsed = blocks(text);
assert_eq!(parsed[0].kind, BlockKind::Override);
let found = tags_in(text, parsed[0]);
assert_eq!(found.len(), 1);
assert_eq!(&text[found[0].name.range()], "\\2c");
assert_eq!(&text[found[0].value.range()], "&H0000FF&");
}

#[test]
fn a_backslash_and_a_digit_with_no_letter_after_it_names_nothing() {
let text = "{\\3}word";
let parsed = blocks(text);
assert_eq!(parsed[0].kind, BlockKind::Comment);
assert!(tags_in(text, parsed[0]).is_empty());
}
}
Loading
Loading