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: 1 addition & 1 deletion BACKLOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ 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 two halves of the colour picker that are not built (filed 2026-09-06; the font half closed 2026-09-07).** The font button is in, over a font enumeration that reads the `name` table out of every installed font file, and over a planner that writes several tags as one step, which is what this entry said both halves wanted. What is left is the colour picker's own two: the alpha, `\1a` to `\4a`, written beside the colour and only when it changed, which the new planner can now carry; and the spectrum, where the picker offers sixteen colours and a field that takes `#RRGGBB` and the reference offers a saturation square, a hue slider, the three number notations and a screen eyedropper. What it said when it was filed: 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.
- [ ] **N39 The spectrum the colour picker does not have (filed 2026-09-06; the font half closed 2026-09-07, the alpha half the same day).** The font button is in, over a font enumeration that reads the `name` table out of every installed font file, and over a planner that writes several tags as one step, which is what this entry said both halves wanted. The transparency went in on that same planner: the picker takes a number from 0 to 255 and writes `\1a` to `\4a` beside the colour, as one undo step, and an empty field writes none at all, which is this product's answer to the reference writing it only when it changed. What is left is the spectrum, where the picker offers sixteen colours and a field that takes `#RRGGBB` and the reference offers a saturation square, a hue slider, the three number notations and a screen eyedropper. What it said when it was filed: 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.

- [ ] **N40 (the same defect as N36, filed twice by mistake on 2026-09-06; keep this entry, which carries what was learned) One check in `video-aspect.spec.js` fails on the CI runner about half the time, and its own evidence does not name the cause (filed 2026-09-06).** "says a media with no picture has none, and is as quiet about it as about no audio" opens a media with no video track and asserts that nothing alarming is on the status bar. On the CI runner it twice found "Open a video first." there: on the merge run of #99 at 16:29 and on #101 at 18:39, both on 2026-09-06, and it has never failed on this repository's own runner. What the check already collects rules out two of the three ways that sentence can appear: the app logged no `was refused as` line, so no command was refused through `refused()`, and the run counted zero `video://error` events. That leaves a rejected `invoke` inside `useVideoPlayer`, whose five commands each set `errorCode` from a rejection, and `VideoErrorCode::NotLoaded` is what `from_mpv` answers for `mpv_error::PropertyUnavailable`, which is what a video property is on a media that carries no video. Which of the five it is has not been proven and must not be guessed: the check has to name the node and the code it found before anything is changed, because a fix aimed at the wrong one of them would look like it worked. **A flaky check is a check that says nothing**, so this is a defect in the suite as much as in the app, and the first change is to the check.
- **2026-09-06, found by reading and fixed, not proven to be the cause.** Every one of those five setters wrote its answer unconditionally, so a command sent against the file that was open could still be in flight when the next file opened and then set the error state about a document nobody had asked about. `useVideoPlayer` now stamps each command with the open it belongs to and drops an answer carrying an older one. It is a real race and the fix is right on its own terms; whether it is what this check keeps catching is only knowable from CI, because it has never reproduced here. Two attempts to reproduce it by taking the audio output away, which is the one difference this machine and the runner are known to have, did not: mpv found an output both times and the spec passed.
Expand Down
73 changes: 0 additions & 73 deletions crates/sublore-edit/src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,6 @@ 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 @@ -172,13 +162,6 @@ 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::ClearText { cue, keep_tags } => plan_clear_text(document, *cue, *keep_tags),
Edit::SetOverrideTags { cue, tags, at } => {
plan_set_override_tags(document, *cue, tags, *at)
Expand Down Expand Up @@ -1288,62 +1271,6 @@ fn plan_clear_text(
})
}

fn plan_set_override_tag(
document: &SubtitleDocument,
index: usize,
tag: &str,
value: &str,
from: usize,
to: usize,
) -> Result<Planned, EditError> {
check_tag(tag, value)?;
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
18 changes: 9 additions & 9 deletions crates/sublore-edit/tests/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,13 +986,13 @@ 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 {
/// One tag written at a caret, which is the list of one the pickers send when nothing else goes
/// with it: a colour with no transparency beside it, or a style flag's own value.
fn set_override_tag(cue: usize, tag: &str, value: &str, at: usize) -> Edit {
Edit::SetOverrideTags {
cue,
tag: tag.to_owned(),
value: value.to_owned(),
from,
to,
tags: vec![(tag.to_owned(), value.to_owned())],
at,
}
}

Expand All @@ -1003,7 +1003,7 @@ fn a_tag_with_a_chosen_value_is_written_where_the_caret_is() {
let text = raw_text(&session, 0);
session
.apply(
&set_override_tag(0, "\\c", "&H0000FF&", 0, 0),
&set_override_tag(0, "\\c", "&H0000FF&", 0),
Run::New,
Instant::now(),
)
Expand Down Expand Up @@ -1066,7 +1066,7 @@ fn a_numbered_colour_replaces_the_one_already_in_the_block_rather_than_joining_i
for value in ["&H0000FF&", "&H00FF00&"] {
session
.apply(
&set_override_tag(0, "\\2c", value, 0, 0),
&set_override_tag(0, "\\2c", value, 0),
Run::New,
Instant::now(),
)
Expand Down Expand Up @@ -1167,7 +1167,7 @@ fn a_tag_name_that_is_not_a_name_and_a_value_that_could_close_a_block_are_both_r
for (tag, value) in [("c", "&H0&"), ("\\1c1", "&H0&"), ("\\c", "&H0&}x{\\b1")] {
session
.apply(
&set_override_tag(0, tag, value, 0, 0),
&set_override_tag(0, tag, value, 0),
Run::New,
Instant::now(),
)
Expand Down
25 changes: 20 additions & 5 deletions e2e/specs/current-line-bands.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1373,17 +1373,32 @@ describe("the current line's bands", () => {
expect(await present(".currentline__hex")).toBe(true);

await typeHex(toplevel, "#12AB34");
// A transparency beside it, which ASS counts the other way from opacity: 128 is half see
// through and it is written in hexadecimal. The two go in as one step. See B12.
await clickElement(toplevel, ".currentline__alpha");
pressKey("ctrl+a");
typeText("128");
await waitFor(
async () =>
(await browser.execute(
() => document.querySelector(".currentline__alpha")?.value ?? null,
)) === "128"
? 1
: null,
{ timeout: 15000, message: "the transparency field to hold exactly 128" },
);
pressKey("Return");
// `#12AB34` is red 12, green AB, blue 34, and ASS writes the three the other way round.
await waitFor(async () => ((await lineText()) === `{\\c&H34AB12&}${before}` ? 1 : null), {
timeout: 15000,
message: "the typed colour to be written at the caret",
});
await waitFor(
async () => ((await lineText()) === `{\\c&H34AB12&\\1a&H80&}${before}` ? 1 : null),
{ timeout: 15000, message: "the typed colour and its transparency written at the caret" },
);

// One step, not two: a colour and how see-through it is are one thing a translator chose.
await clickElement(toplevel, ".toolbar__edit-undo");
await waitFor(async () => ((await lineText()) === before ? 1 : null), {
timeout: 15000,
message: "one undo to take the typed colour back off",
message: "one undo to take the colour and its transparency back off together",
});
});

Expand Down
1 change: 0 additions & 1 deletion src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ pub fn run() -> tauri::Result<()> {
subtitle::subtitle_set_field,
subtitle::subtitle_set_comment,
subtitle::subtitle_toggle_style,
subtitle::subtitle_set_override_tag,
subtitle::subtitle_set_override_tags,
subtitle::subtitle_clear_text,
fonts::fonts_installed,
Expand Down
43 changes: 0 additions & 43 deletions src-tauri/src/subtitle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -446,17 +446,6 @@ pub async fn subtitle_toggle_style(
.await
}

/// One override tag as a caller names it: the tag, the value chosen for it, and the stretch of the
/// line it is written over. One value rather than four more arguments, which is the shape H5
/// settled on when `clippy::too_many_arguments` was right about `module_invoke`.
#[derive(Debug, Deserialize)]
pub struct OverrideTagWrite {
pub tag: String,
pub value: String,
pub from: usize,
pub to: usize,
}

/// Empty one cue's text. `keep_tags` is the reference's Clear Text: the braced runs stay where
/// they are and only the words go. See edit-bar-tasks.md B13.
#[tauri::command]
Expand Down Expand Up @@ -503,38 +492,6 @@ pub async fn subtitle_set_override_tags(
.await
}

/// One override tag with a value the caller chose, over the same stretch a style toggle works on.
/// The name and the value are checked by the planner, which refuses anything that could close the
/// block it is written into. See edit-bar-tasks.md B12.
#[tauri::command]
pub async fn subtitle_set_override_tag(
app: AppHandle,
state: State<'_, SubtitleState>,
revision: u64,
cue: usize,
write: OverrideTagWrite,
) -> Result<CuePatchDto, SubtitleError> {
let OverrideTagWrite {
tag,
value,
from,
to,
} = write;
edited(
&app,
state.slot(),
revision,
Edit::SetOverrideTag {
cue,
tag,
value,
from,
to,
},
)
.await
}

/// Whether one cue is a line a player draws. Refused on a format that has no descriptor to
/// rewrite, and the panel draws that control greyed instead of asking. See edit-bar-tasks.md B8.
#[tauri::command]
Expand Down
15 changes: 2 additions & 13 deletions src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1676,23 +1676,12 @@ export default function App() {
fontsLoading={fonts.loading}
onLoadFonts={fonts.load}
onSetOverrideTags={async (tags, at) => {
// The same rule the buttons grey on, read again here: a greyed control must not
// run, and a picker left open on a row the cursor has left must not write to it.
if (writesAtCaret && selection.active !== null) {
await subtitle.setOverrideTags(selection.active, tags, at);
}
}}
onSetOverrideTag={async (tag, value) => {
// The same rule the button greys on, read again here: a greyed command must not
// run, and a picker left open on a row the cursor has left must not write to it.
if (writesAtCaret && caret !== null && selection.active !== null) {
await subtitle.setOverrideTag(
selection.active,
tag,
value,
caret.offset,
caret.to,
);
}
}}
/>
</section>
</div>
Expand Down
Loading