Repository navigation
Strip Silence: Rename sets the names of the clips it leaves - #1044
Conversation
… without removing any The Strip Silence window had only a Strip button. It now also has Extract and Separate. Extract removes the parts of the selection that hold audio and keeps the silent parts. Separate separates the clips at the same points Strip does and does not remove any clip. Enter on a focused Extract or Separate runs that button, and Enter presses Strip only when neither was clicked. edit.strip_silence takes a mode param: strip (the default), extract or separate. An unknown mode is an error. The Minimum Strip Duration slider now goes up to 4,000 ms.
edit.strip_silence read a null, boolean or number mode as no mode and ran Strip. It now returns an error and leaves the track unchanged, as it does for an unknown mode name.
The Strip Silence window has a Rename… button. It opens Rename Selected Clips, where Name, Number, Zeros and Suffix set the names that Strip, Extract and Separate give the clips they leave in the selection: the name, that many zeros, a number counting up from Number, and the suffix. SFX, 23, 1 and .Reel1 give SFX023.Reel1, SFX024.Reel1 and so on. The app keeps the scheme for later uses of Strip Silence until the app quits, or until Clear and then OK empty it. OK, Cancel and Escape go back to Strip Silence with its slider values. edit.strip_silence takes name, number, zeros and suffix. Numbering runs on from one track to the next, and the result reports how many clips were named. Without a name the clips keep their names. A name or suffix that is not text, a number out of range, or a number, zeros or suffix without a name is an error and leaves the tracks unchanged.
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Engine modes and naming crates/engine/src/io.rs |
Adds mode-specific clip handling and clip naming. Tests cover all modes, naming order, and invalid naming parameters. |
Command options and validation crates/engine/src/cmd/edit.rs |
Adds mode and naming parameters to edit.strip_silence, validates their values, and returns clip and named counts. |
Dialog flow and integration crates/ui-egui/src/dialogs.rs, crates/ui-egui/tests/it/*, crates/ui-egui/tests/it/main.rs, ROADMAP.md |
Adds Extract and Separate actions, a rename dialog, and retained naming settings. The minimum strip-duration slider limit increases to 4,000 ms. UI integration tests and roadmap entries cover the changes. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant User
participant StripSilenceDialog
participant edit.strip_silence
participant strip_silence
participant Session
User->>StripSilenceDialog: Select mode and run
StripSilenceDialog->>edit.strip_silence: Submit mode and naming parameters
edit.strip_silence->>strip_silence: Pass mode and padding
strip_silence->>Session: Split clips and apply mode-specific removal
strip_silence-->>edit.strip_silence: Return affected clip count
edit.strip_silence-->>StripSilenceDialog: Return clip and named counts
Suggested reviewers: echelon
Merge Risk: 
Minimal · up to 69850
-
The checked Strip Silence paths behave as intended; no identified issue prevents merging after normal checks.
Security architecture risk: 
Low · up to 69850
-
The new operations retain current-session targeting and existing undo protection. No introduced security weakness was established in the inspected paths, but automation authorization and compatibility with external callers remain partially verified.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- observed — A command caller can target multiple existing tracks in the current session and provide an explicit range rather than relying on the UI selection. Track resolution rejects unknown targets and deduplicates repeated IDs. This explicit-target capability predates the new mode and naming fields.
Trust Boundaries and Controls
- observed — The existing automation handler forwards command IDs and parameters through SoundApp::run to Engine::execute, so callers reaching that channel can use the new modes and naming inputs. The PR does not change the inspected generic dispatch path. Its deployment authentication and intended caller-authorization policy were not established; session-target validation alone is not proof of caller authorization.
Resilience and Maintainability Implications
- inferred — For command-mediated execution, a later-track failure should not strand earlier tracks with transformed clips or new names: the same document snapshot encloses the entire track loop. This containment does not establish equivalent guarantees for external callers invoking the public mutation helper directly.
-
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passedFull details: Docstring Coverage
-
Explanation
Docstring coverage is 30.19% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 53 functions across 6 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 Clippy (1.99.0)
-
Clippy execution failed
Usage-based review receipt
- Mode: Continue automatically
- Reviewed files: 7
- Waived: $1.75 (charged $0.00)
- View usage details
Note
This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.
Comment @coderabbitai help to get the list of available commands.
Builds on #1040, which adds the Extract and Separate buttons; until #1040 merges, this diff also shows its commits.
On
main, Strip Silence leaves every clip it cuts with the name of the clip it came from, and the Strip Silence window has no Rename button. The reference guide's "Auto-Naming for Strip Silence" section (Pro Tools Reference Guide 2020.12, Chapter 41, page 926) describes the button: it "opens the Rename Selected Clips dialog, which determines how clips are named when stripping silence", with Name, Number, Zeros and Suffix fields and a Clear button, and the dialog "remembers your previous settings". Its example, SFX, 23, 1 zero and .Reel1, gives SFX023.Reel1 to SFX026.Reel1. The guide: https://web.archive.org/web/20210805202452/https://resources.avid.com/SupportFiles/PT/Pro_Tools_Reference_Guide_2020.12.pdfedit.strip_silencetakesname,number(default 1, at most 999,999,999),zeros(at most 9) andsuffix. It names the audio clips that Strip, Extract or Separate leave inside the selection, in time order, and the numbering runs on from one track to the next. The result has anamedcount. Without a name the clips keep their names, as before.nameorsuffixthat is not text, anumberorzerosout of range, ornumber,zerosorsuffixwithout anameis an error and leaves the tracks unchanged.ROADMAP.mdgets a progress log line and a revision table row, and its header names this change.New tests. On #1040's head each of them fails or does not compile:
strip_names_follow_the_guide_example(engine): the guide's example gives SFX023.Reel1 to SFX026.Reel1.strip_names_the_clips_it_keeps(engine): Strip with the guide's scheme names the two tones SFX023.Reel1 and SFX024.Reel1.separate_names_every_piece_in_time_order(engine).strip_numbers_run_on_from_one_track_to_the_next(engine).strip_without_a_name_keeps_the_clip_names(engine): a blank name leaves the names as they were and reports 0 named.bad_naming_params_are_refused(engine): seven bad param sets, each an error that leaves the track unchanged.strip_silence_sends_the_naming_scheme_only_with_a_name(ui-egui).rename_names_the_clips_strip_silence_leaves(ui-egui, through the app): Rename…, OK, then Strip names the Kick clips and keeps the scheme.cancel_in_rename_goes_back_and_keeps_the_names_it_had(ui-egui, through the app).clear_in_rename_empties_the_naming_scheme(ui-egui, through the app).enter_on_a_focused_cancel_goes_back_without_the_new_names,enter_on_a_focused_clear_clears_and_leaves_rename_openandenter_on_a_focused_rename_opens_it_and_does_not_strip(ui-egui, through the app).Checks, with
SOUNDCRAFT_NO_SYSTEM_PLUGINS=1set for the tests:cargo test -p soundcraft-engine --lib strip_tests: 12 passedcargo test -p soundcraft-ui-egui --lib --test it strip_silence: 2 + 10 passedcargo clippy -p soundcraft-engine -p soundcraft-ui-egui --all-targets --keep-going -- -D warnings -A unused-mut: no warnings (the allowedunused_mutatcrates/engine/src/autosave.rs:189is onmainon Windows)cargo fmt --all -- --check: cleancargo xtask assetsandcargo xtask layers: okOn Windows 11 (x86-64) with rustc 1.99.1, on top of
mainatee050dbwith #1040, #1017 and #1018 merged in, and #998 merged without itscrates/engine/src/io.rsandxtask/src/main.rschanges (they conflict withmain), withSOUNDCRAFT_NO_SYSTEM_PLUGINS=1set:cargo xtask cipasses its 7 steps: 1,298 tests passed, 0 failed, 1 ignored.git merge-treereports no conflicts withmainatee050db. With #1036 (Unloop dialog), #1030 (Clip Looping) or #1016 (Vari-Fi) merged first, onlyROADMAP.mdconflicts, as it does for #1040.cargo check -p soundcraft-ui-egui -p soundcraft-engine --testspasses on the merges with #1036 and #1030 (withROADMAP.mdtaken from this branch) and on the merge with #349 (batch track rename).Summary by CodeRabbit