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
4 changes: 3 additions & 1 deletion crates/sc-cli/src/export_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ fn what_to_do(reason: ExportSkip) -> &'static str {
ExportSkip::UnsupportedChannels { .. } => "export a stereo version first",
ExportSkip::NothingToWrite { .. } => "run it again without --no-xml",
ExportSkip::OtherFileInFolder => "move that file away, or export into another folder",
ExportSkip::CopyOfOtherTrack => "rename one of the two, or export into another folder",
ExportSkip::CopyOfOtherTrack | ExportSkip::SameNameInBatch => {
"rename one of the two, or export into another folder"
}
ExportSkip::Unsupported { .. } => "convert it to AIFF, WAV or FLAC first",
ExportSkip::Silent => "check that it is the right file",
ExportSkip::NoGrid => "set the grid in the app, or export without --grid-only",
Expand Down
15 changes: 11 additions & 4 deletions crates/sc-core/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,13 @@ pub enum ExportSkip {
/// The folder already holds a file of this name that is not a copy SoundCheck wrote, or that
/// changed since: it is left as it is.
OtherFileInFolder,
/// A copy of another track has this name in the folder, or would have it first in this batch:
/// it is left as it is, since it is not this file's to replace.
/// The folder holds a copy of this name that SoundCheck made from another file, or from this
/// one before it was moved and changed: it is left as it is, since it is not this file's to
/// replace.
CopyOfOtherTrack,
/// A file earlier in this batch is written under this name, and nothing of that name need be
/// in the folder yet: the first in the list is written.
SameNameInBatch,
/// An MP3 or AAC file, which only the rekordbox XML carries, without a grid: the XML would
/// list nothing for it.
XmlNoGrid {
Expand Down Expand Up @@ -401,7 +405,8 @@ pub enum SkipCure {
Grid,
/// Other export settings.
Settings,
/// What is in the folder, which can change with no word to the app.
/// What is in the folder or goes along in the batch, which can change with no word to the
/// row.
Disk,
}

Expand All @@ -422,7 +427,9 @@ impl ExportSkip {
Self::SeratoInPlaceCut { .. }
| Self::SeratoUnknownInPlaceCut { .. }
| Self::NothingToWrite { .. } => SkipCure::Settings,
Self::OtherFileInFolder | Self::CopyOfOtherTrack => SkipCure::Disk,
Self::OtherFileInFolder | Self::CopyOfOtherTrack | Self::SameNameInBatch => {
SkipCure::Disk
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/sc-core/src/export/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ fn a_skip_says_what_could_cure_it() {
),
(ExportSkip::OtherFileInFolder, SkipCure::Disk),
(ExportSkip::CopyOfOtherTrack, SkipCure::Disk),
(ExportSkip::SameNameInBatch, SkipCure::Disk),
];
for (skip, cure) in cases {
assert_eq!(skip.cure(), cure, "{skip:?}");
Expand Down
7 changes: 5 additions & 2 deletions crates/sc-core/src/ipc/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ pub struct ConfirmedFile {
pub file_id: u32,
/// Its identity then (length, modification time, inode): opaque, compared whole.
pub identity: String,
/// The plan skipped it because a file earlier in the batch takes its name: it stays skipped,
/// whatever else drops out of the job.
pub same_name: bool,
}

/// What a row's export was made with, so the row can say when the settings have moved on.
Expand Down Expand Up @@ -123,8 +126,8 @@ pub struct ExportPreviewRow {
/// What its row would show: `stage` `processing` for a file that would be written, `skipped`
/// for one that would not, with the plan's notes worded. `None` when it could not be planned.
pub row: Option<Box<ExportRow>>,
/// Why it could not be planned: not analysed yet, changed since it was analysed, or its start
/// could not be read.
/// Why it could not be planned: not analysed yet, changed since it was analysed, its start
/// could not be read, or an export in place would refuse it.
pub error: Option<IpcError>,
/// The file as it was planned, to send back ([`ConfirmedFile`]); `None` without a plan.
pub identity: Option<String>,
Expand Down
2 changes: 2 additions & 0 deletions crates/sc-core/src/ipc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,8 @@ pub enum JobEvent {
plan: Plan,
/// The settings revision `plan` was decided with; a newer replan wins.
revision: u32,
/// The file's export no longer stands for it, as the session now says.
export_stale: bool,
},
/// The file could not be analysed; the job goes on.
Failed {
Expand Down
1 change: 1 addition & 0 deletions crates/sc-core/src/ipc/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ fn a_worst_case_analysed_row_stays_within_budget() {
row: Box::new(row),
plan,
revision: u32::MAX,
export_stale: false,
};
assert!(json_len(&ev) <= MAX_ROW_BYTES, "{} bytes", json_len(&ev));
}
Expand Down
37 changes: 29 additions & 8 deletions crates/sc-engine/src/export/preview.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
//! What exporting would do with one analysed file, for the confirm step: the same
//! [`plan_export_snapped`] the job plans with, on the tags the file holds now, turned into the row
//! the job would show. Nothing is written.
//! the job would show, with the refusals of a write in place said here too. Nothing is written.

use std::path::Path;

use sc_core::Result;
use sc_core::analysis::AnalysisRecord;
use sc_core::export::{ExportOutcome, ExportSettings};
use sc_core::export::{ExportOutcome, ExportSettings, ExportSkip};
use sc_core::ipc::ExportRow;
use sc_core::plan::DecideSettings;

Expand All @@ -16,6 +16,7 @@ use super::row;
use super::snap::plan_export_snapped;
use crate::decide::decide;
use crate::edits::EditState;
use crate::txn::FolderNames;

/// One file to preview: its analysis with the grid as shown, and how it would be exported.
#[derive(Debug, Clone, Copy)]
Expand All @@ -34,17 +35,23 @@ pub struct PreviewInput<'a> {
pub out_dir: Option<&'a Path>,
/// An earlier change of the file is in effect.
pub revertible: bool,
/// An earlier file of the batch takes the name its copy would have.
pub same_name: bool,
/// Where backups go: the transaction refuses a file already inside.
pub backup_root: &'a Path,
}

/// The row exporting the file would show, and whether its write replaces an earlier copy. The
/// source is not hashed, which only the record written into the file needs.
/// source is not hashed, which only the record written into the file needs. `names` holds the names
/// earlier rows of the batch write under, for an export to a folder: only a row that plans a write
/// takes one, so call this in list order.
///
/// # Errors
/// What [`plan_export_snapped`] returns for a file whose start cannot be read up to the cut; what
/// reading a file of that name in the folder returns.
pub fn preview_row(input: &PreviewInput<'_>) -> Result<(Box<ExportRow>, bool)> {
/// reading a file of that name in the folder returns; what a write in place of this file would be
/// refused with ([`sc_io::txn::check_writable_in_place`]).
pub fn preview_row(
input: &PreviewInput<'_>,
names: Option<&mut FolderNames>,
) -> Result<(Box<ExportRow>, bool)> {
let source = ExportSource::read(input.path);
let decided = decide(
input.record,
Expand All @@ -60,7 +67,21 @@ pub fn preview_row(input: &PreviewInput<'_>) -> Result<(Box<ExportRow>, bool)> {
grid_confirmed: input.edit.confirmed,
};
let outcome = plan_export_snapped(input.path, &planning, input.settings)?;
let (outcome, replaces) = gate_folder(outcome, input.path, input.out_dir, input.same_name)?;
let (outcome, replaces) = gate_folder(outcome, input.path, input.out_dir, false)?;
let writes = matches!(outcome, ExportOutcome::Write { .. });
if writes && input.out_dir.is_none() {
sc_io::txn::check_writable_in_place(input.path, input.backup_root)?;
}
let name_taken = writes
&& names
.zip(input.path.file_name())
.is_some_and(|(names, name)| !names.claim(name));
let (outcome, replaces) = if name_taken {
let reason = ExportSkip::SameNameInBatch;
(ExportOutcome::Skip { reason }, false)
} else {
(outcome, replaces)
};
Ok((
match outcome {
ExportOutcome::Write { plan } => row::planned(&plan, input.revertible),
Expand Down
11 changes: 6 additions & 5 deletions crates/sc-engine/src/export/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ pub struct ProcessSettings {
/// Each file as the user confirmed it, by id: a file that is no longer that one fails before
/// anything is written. `None` checks nothing.
pub confirmed: Option<BTreeMap<u32, String>>,
/// Files whose copy would take a name an earlier file of this batch takes: skipped, the first
/// in the list being written. By id.
/// Files skipped because a file earlier in the batch writes under the name theirs would have,
/// by id: what the confirm step decided, when there was one, since only a row that plans a
/// write takes a name and the plans are known there.
pub same_name: BTreeSet<u32>,
}

Expand Down Expand Up @@ -285,8 +286,8 @@ fn check_confirmed(file_id: u32, path: &Path, settings: &ProcessSettings) -> Res
})
}

/// `outcome`, given what is at the path a copy would take in `out_dir`, and whether an earlier file
/// of the batch takes that name (`same_name`): anything of that name but this file's own earlier
/// `outcome`, given what is at the path a copy would take in `out_dir`, and whether a file earlier
/// in the batch writes under that name (`same_name`): anything of that name but this file's own earlier
/// copy keeps the file from being written. Also says whether the write replaces that copy.
///
/// # Errors
Expand All @@ -304,7 +305,7 @@ pub(super) fn gate_folder(
};
let skip = |reason| (ExportOutcome::Skip { reason }, false);
if same_name {
return Ok(skip(ExportSkip::CopyOfOtherTrack));
return Ok(skip(ExportSkip::SameNameInBatch));
}
Ok(match copy_at(&dir.join(name), path)? {
CopyAt::Nothing => (outcome, false),
Expand Down
4 changes: 2 additions & 2 deletions crates/sc-engine/src/export/row.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use sc_io::render::{BlockFate, RenderReport};
use sc_io::txn::{TxnKind, TxnReport};

use super::ProcessDone;
use super::wording::{GRID_WITHHELD, notice_text, skip_why, xml_only_text};
use super::wording::{GRID_WITHHELD, notice_text, skip_note, xml_only_text};

/// What the plan says the user should know, in words, for the row's notes.
fn plan_notes(plan: &ExportPlan) -> Vec<String> {
Expand Down Expand Up @@ -53,7 +53,7 @@ pub(crate) fn skipped(outcome: &ExportOutcome, revertible: bool) -> Box<ExportRo
ExportAction::Skip { reason: *reason },
Vec::new(),
false,
vec![skip_why(*reason)],
vec![skip_note(*reason)],
),
// A batch reports a written file through its other events.
ExportOutcome::Write { plan } => (
Expand Down
25 changes: 23 additions & 2 deletions crates/sc-engine/src/export/wording.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! What a plan did, in words: the grid report's notes, the command line's lines and the row
//! notes the app shows. Only the "why"; what to do about it depends on where it is shown.
//! notes the app shows. [`skip_why`] is only the "why", since what to do about it depends on where
//! it is shown; [`skip_note`] is the app's row.

use sc_core::export::{ExportNotice, ExportSkip, XmlOnlyReason};

Expand Down Expand Up @@ -50,6 +51,18 @@ pub fn xml_only_text(reason: XmlOnlyReason) -> String {
}
}

/// A skipped row's note in the app: why, with what to do when no other part of the app says it.
#[must_use]
pub fn skip_note(reason: ExportSkip) -> String {
let why = skip_why(reason);
match reason {
ExportSkip::CopyOfOtherTrack | ExportSkip::SameNameInBatch => {
format!("{why}; rename one, or choose another folder")
}
_ => why,
}
}

/// Why a file is skipped.
#[must_use]
pub fn skip_why(reason: ExportSkip) -> String {
Expand Down Expand Up @@ -108,7 +121,12 @@ pub fn skip_why(reason: ExportSkip) -> String {
.to_owned()
}
ExportSkip::CopyOfOtherTrack => {
"a copy of another track already has this name in the folder".to_owned()
"the folder has a copy of this name that SoundCheck made from another file, or from \
this one before it was moved and changed"
.to_owned()
}
ExportSkip::SameNameInBatch => {
"a file earlier in this export takes this name in the folder".to_owned()
}
ExportSkip::GridNeedsReview => {
"grid only, and its grid needs review, so it is not written and there is nothing \
Expand All @@ -117,3 +135,6 @@ pub fn skip_why(reason: ExportSkip) -> String {
}
}
}

#[cfg(test)]
mod tests;
48 changes: 48 additions & 0 deletions crates/sc-engine/src/export/wording/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use sc_core::export::ExportSkip;

use super::{skip_note, skip_why};

const WHAT_TO_DO: &str = "rename one, or choose another folder";

#[test]
fn a_copy_in_the_folder_says_it_may_be_this_file_before_it_moved() {
let why = skip_why(ExportSkip::CopyOfOtherTrack);
assert!(why.contains("made from another file"), "{why}");
assert!(why.contains("before it was moved and changed"), "{why}");
}

#[test]
fn a_name_taken_in_the_batch_does_not_say_a_copy_exists() {
let why = skip_why(ExportSkip::SameNameInBatch);
assert!(why.contains("earlier in this export"), "{why}");
assert!(!why.contains("already"), "nothing need be there yet: {why}");
assert!(!why.contains("copy"), "nothing need be there yet: {why}");
}

#[test]
fn the_app_says_what_to_do_about_a_name_and_the_command_line_keeps_its_own() {
for reason in [ExportSkip::CopyOfOtherTrack, ExportSkip::SameNameInBatch] {
let note = skip_note(reason);
assert!(note.starts_with(&skip_why(reason)), "{note}");
assert!(note.ends_with(WHAT_TO_DO), "{note}");
assert!(!skip_why(reason).contains(WHAT_TO_DO));
}
}

#[test]
fn other_skips_keep_their_note() {
let reason = ExportSkip::Silent;
assert_eq!(skip_note(reason), skip_why(reason));
}

#[test]
fn the_words_read_as_the_app_shows_them() {
for reason in [ExportSkip::CopyOfOtherTrack, ExportSkip::SameNameInBatch] {
let note = skip_note(reason);
// The app makes a sentence of it: starting small, ending with no stop of its own.
assert!(note.starts_with(char::is_lowercase), "{note}");
assert!(!note.ends_with('.'), "{note}");
let lower = note.to_lowercase();
assert!(!lower.contains("sorry") && !lower.contains("unfortunately"));
}
}
4 changes: 2 additions & 2 deletions crates/sc-engine/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@ pub use export::{
pub use session::{PlanningRow, RevertedAnalysis, Session, run_job};
pub use track::{Track, TrackProgress};
pub use txn::{
ApplyOptions, ApplyRequest, Place, RecoveryGate, RecoveryGuard, Tag, apply_file, check_inputs,
forget_cached, forget_recovered, recover_at_start, undo_file,
ApplyOptions, ApplyRequest, FolderNames, Place, RecoveryGate, RecoveryGuard, Tag, apply_file,
check_inputs, forget_cached, forget_recovered, recover_at_start, undo_file,
};
Loading
Loading