From ce12c77fd25ac21094079055d772c1ae9477ac09 Mon Sep 17 00:00:00 2001 From: botandbang <337765928+botandbang@users.noreply.github.com> Date: Fri, 30 Oct 2026 10:56:43 -0400 Subject: [PATCH] fix(locallibrary): plugins on folder drives, plugin fields in rules and edits, panel leftovers Planned as these commits, folded into one because they share files: - fix(locallibrary): let a plugin hook read the folder drive it runs on, and only while it runs - feat(locallibrary): take analyzers' fields in smart rules, edit selectors and drive sorts - refactor(locallibrary): read the player's analysis and matches through the api - fix(locallibrary): title plugin panels as track show does and ask again when settings change --- apps/locallibrary/CHANGELOG.md | 13 ++ apps/locallibrary/docs/editing.md | 8 +- apps/locallibrary/docs/player.md | 8 +- apps/locallibrary/docs/plugins.md | 6 +- apps/locallibrary/src/edit_cmd.rs | 2 +- apps/locallibrary/src/main.rs | 2 +- apps/locallibrary/src/plugin_cmd/view.rs | 8 +- apps/locallibrary/src/plugins.rs | 29 +---- apps/locallibrary/src/rpc/methods.rs | 2 +- apps/locallibrary/src/tui/app.rs | 8 +- apps/locallibrary/src/tui/edits.rs | 16 +-- apps/locallibrary/src/tui/mix.rs | 4 +- apps/locallibrary/src/tui/mytags/tests.rs | 2 +- apps/locallibrary/src/tui/plugins.rs | 54 +++----- .../locallibrary/src/tui/plugins/analyzers.rs | 4 +- apps/locallibrary/src/tui/plugins/events.rs | 2 +- .../locallibrary/src/tui/plugins/exporters.rs | 2 +- .../locallibrary/src/tui/plugins/importers.rs | 2 +- apps/locallibrary/src/tui/plugins/panels.rs | 36 +++--- .../src/tui/plugins/panels/tests.rs | 28 ++++- .../locallibrary/src/tui/plugins/providers.rs | 10 +- apps/locallibrary/src/tui/plugins/sources.rs | 2 +- apps/locallibrary/src/tui/plugins/tests.rs | 28 +++-- .../src/tui/plugins/tests/events.rs | 2 +- apps/locallibrary/src/tui/props.rs | 9 +- crates/locallibrary/src/api/editing.rs | 17 ++- crates/locallibrary/src/api/reads.rs | 8 +- crates/locallibrary/src/edit.rs | 42 +++++-- crates/locallibrary/src/fields.rs | 31 +++++ crates/locallibrary/src/plugin/analyzers.rs | 2 +- crates/locallibrary/src/plugin/callbacks.rs | 32 +++-- .../src/plugin/callbacks/tests.rs | 6 +- crates/locallibrary/src/plugin/checks.rs | 5 +- crates/locallibrary/src/plugin/commands.rs | 2 +- .../src/plugin/destinations/publish.rs | 2 +- crates/locallibrary/src/plugin/events.rs | 9 +- crates/locallibrary/src/plugin/exports.rs | 9 +- crates/locallibrary/src/plugin/imports.rs | 3 +- crates/locallibrary/src/plugin/operations.rs | 2 +- crates/locallibrary/src/plugin/panels.rs | 2 +- crates/locallibrary/src/plugin/process.rs | 55 ++++++++- crates/locallibrary/src/plugin/providers.rs | 2 +- crates/locallibrary/src/plugin/services.rs | 2 +- crates/locallibrary/src/plugin/sources.rs | 2 +- crates/locallibrary/src/query.rs | 116 ++++++++++++++---- crates/locallibrary/src/smart.rs | 17 ++- crates/locallibrary/src/smart/tests.rs | 19 +-- crates/locallibrary/tests/it/plugins.rs | 34 +++++ .../tests/it/plugins/callbacks.rs | 29 +++++ .../locallibrary/tests/it/plugins/fields.rs | 100 +++++++++++++++ crates/locallibrary/tests/it/plugins/jobs.rs | 32 ----- .../tests/it/props_core2_smart.rs | 2 +- crates/locallibrary/tests/it/props_smart.rs | 21 ++-- .../tests/it/props_store_records.rs | 17 +-- 54 files changed, 629 insertions(+), 278 deletions(-) create mode 100644 crates/locallibrary/tests/it/plugins/fields.rs diff --git a/apps/locallibrary/CHANGELOG.md b/apps/locallibrary/CHANGELOG.md index a0490254b..fdcf66da6 100644 --- a/apps/locallibrary/CHANGELOG.md +++ b/apps/locallibrary/CHANGELOG.md @@ -793,6 +793,19 @@ their old names. - When the track queued after the playing one is short enough to play out before the player next looks, the player goes on past it at once. It showed it as playing until then, and `n` pressed in that moment skipped the track after it. +- The player heads a plugin's panel with its view's own title after the panel's, as `track show` + does, and asks for the selected track's panels again once that plugin's settings are saved, + where it kept showing what they answered before until the selection moved. +- Smart playlist rules, edits' track selectors (`--filter`) and sorting a playlist on the drive + (`playlist sort`, the player's `s`) take analyzers' fields (`sound.bpm:>120`, `-sound.bpm`), as + `tracks` does, where they were refused. A rule naming a field no plugin installed declares, or + a value its field can't hold, is refused when it's added and when it's applied, saying so, and + an edit whose filter names a field no track has a value of yet warns that they need analyzing. +- Plugins' commands, operations, panels, analyzers, exporters, importers, library sources and + providers run on a folder the player (or any command) opened by path, where the player said + "only on a plugged-in drive". The plugin may name that folder as its hook's context does, only + while the hook runs, and is told its name rather than where it is; a path or another folder is + still refused. ### Security diff --git a/apps/locallibrary/docs/editing.md b/apps/locallibrary/docs/editing.md index 90ce90d56..b499d02cf 100644 --- a/apps/locallibrary/docs/editing.md +++ b/apps/locallibrary/docs/editing.md @@ -211,7 +211,7 @@ track. `--filter` takes the same filter `tracks --filter` does: | `rating:>=4`, `plays:0`, `year:1999`, `time:>6:00`, `bitrate:<320` | numbers, ranges, comparisons | | `color:aqua`, `color:none`, `added:2026-09`, `added:>=2026-01-01` | colour, date added | | `playlist:Sets/Friday`, `tag:Genre/Deep`, `is:unfiled`, `is:unplayed`, `has:artwork` | membership, My Tags, state | -| `sound.bpm:>120`, `sound.key:8A` | what an analyzer [plugin](plugins.md) worked out (`tracks` and the player's search only) | +| `sound.bpm:>120`, `sound.key:8A` | what an analyzer [plugin](plugins.md) worked out | `playlist:` takes a path or a unique name, as `--playlist` does, and a folder means everything in it; one that names no playlist is refused. So is a word before a colon that names no field @@ -219,8 +219,10 @@ it; one that names no playlist is refused. So is a word before a colon that name (`"re:zero"`) it's text. `tag:` reads the drive's My Tags wherever a filter goes, and `--history` finds lists in both databases. -Sort orders are lists of fields, highest first with a `-`: `key,bpm`, `-rating,title`. `tracks ---sort` also takes a plugin's field (`-sound.bpm`); tracks without a value for it go last. +Sort orders are lists of fields, highest first with a `-`: `key,bpm`, `-rating,title`, and a +plugin's field (`-sound.bpm`); tracks without a value for it go last. A plugin's field is one an +analyzer installed declares, and a rule or an edit naming another is refused, saying so; an edit +whose filter names one no track has a value of yet warns that the tracks need analyzing first. ### When rekordbox exports again diff --git a/apps/locallibrary/docs/player.md b/apps/locallibrary/docs/player.md index 76ef3e22d..c1ed99233 100644 --- a/apps/locallibrary/docs/player.md +++ b/apps/locallibrary/docs/player.md @@ -41,14 +41,16 @@ listening to your library the way the players see it: plan, with what the plugin says about it first, is confirmed and written like any edit, and `u` takes it back. A question one asks first (which track an entry is) is a list to pick from (`space` marks items when it takes several, `enter` takes them, `esc` stops the run). An - operation, or a command of a plugin that reads the drive, needs one plugged in, not a folder - opened by path. What plugins log goes to the messages (`!`). See + operation, or a command of a plugin that reads the drive, needs a drive open (a folder opened + by path too, which the plugin can read only while it runs on it). What plugins log goes to the + messages (`!`). See [Plugins](plugins.md). - **Analyzers**: "analyze the tracks: …" in the palette (or a drive's `e` menu) runs a plugin's analyzer over the drive's tracks in the background, those not analysed already. "show the sound.bpm column" adds a column of what it worked out (remembered for the next run; `-` where it found nothing to show; shown as its analyzer says, in bytes or as a meter, say), sorted by a click on its header like any column, and the search box - filters by it: `sound.bpm:>120`. "write keys and BPMs to the tracks: …" writes what an + filters by it: `sound.bpm:>120`. Sorted by it, a playlist is sorted on the drive by it too + (`s` in the actions menu). "write keys and BPMs to the tracks: …" writes what an analyzer heard to the tracks (a form asks which, and whether over what they have), confirmed like any edit. - **Checks**: a drive's check (in its `e` menu) shows what plugins' checks found too, and a fix diff --git a/apps/locallibrary/docs/plugins.md b/apps/locallibrary/docs/plugins.md index 0a7b57477..3d0fb87af 100644 --- a/apps/locallibrary/docs/plugins.md +++ b/apps/locallibrary/docs/plugins.md @@ -234,7 +234,8 @@ track's values once (again only for a changed audio file, or a new version of th keeps them in the store. A value worked out with nothing to show (silence's key) shows as `-`, matches no filter and sorts last. `locallibrary track LIBRARY --track ID` shows them under the track's details, each as `ID.FIELD` (`sound.bpm`), and `locallibrary tracks` filters and sorts by -them (`--filter "sound.bpm:>120" --sort -sound.bpm`), as the sidecar's `tracks.query` does. In +them (`--filter "sound.bpm:>120" --sort -sound.bpm`), as the sidecar's `tracks.query`, smart +playlists' rules, edits' track selectors and playlist sorts do. In the player, analyzers are in the palette and a drive's menu, and their fields are columns to show, sort and search by. An analyzer may say how its fields are shown: a unit (`LUFS`), how many places, as a key, a duration (`3:07`), decibels, a percentage or a meter, and how wide its column @@ -503,7 +504,8 @@ locallibrary adds kinds of hooks without breaking older plugins. A plugin is refused if it says it's something other than what locallibrary knows it as. It may call only the reads its capabilities grant, naming drives by name or id (never a path), in pages of -bounded size, and what it gets back, errors too, doesn't say where things are on this computer: +bounded size. Those are the drives known here, and the one a hook under way runs on, a folder +opened by path too, named as the hook's context names it, until the hook is done. What it gets back, errors too, doesn't say where things are on this computer: a drive is said by its name, where the store's copies, a set's or a mix's recording and rekordbox's collection are is left out, and any other path is said from its drive, the store (``) or your home folder (`~`). An exporter alone is told where the drive it exports is, as diff --git a/apps/locallibrary/src/edit_cmd.rs b/apps/locallibrary/src/edit_cmd.rs index 5eb8f024e..a00ee931f 100644 --- a/apps/locallibrary/src/edit_cmd.rs +++ b/apps/locallibrary/src/edit_cmd.rs @@ -897,7 +897,7 @@ pub fn smart(args: SmartArgs, store_path: &Option, out: Output, ctx: &C limit, drives: Vec::new(), }; - rule.validate()?; + rule.validate(&crate::plugins::host().map(|p| p.fields()).unwrap_or_default())?; let change = match crate::diff_cmd::read_or_empty(store_path, api::smart_list)?.get(&path) { Some(old) => format!("replace the rule for {path:?} ({}) with {filter:?}", old.filter), None => format!("add a rule for {path:?}: {filter:?}"), diff --git a/apps/locallibrary/src/main.rs b/apps/locallibrary/src/main.rs index bdb3e60c2..454405dee 100644 --- a/apps/locallibrary/src/main.rs +++ b/apps/locallibrary/src/main.rs @@ -1198,7 +1198,7 @@ fn run(cli: Cli, out: Output, contributed: Option) -> R // --search is free text in the filter. let filter: Vec = a.search.as_deref().map(search_terms).into_iter().chain(a.filter).collect(); let filter = (!filter.is_empty()).then(|| filter.join(" ")); - let fields = plugins::Installed::with(diff_cmd::existing_store(&cli.store)?); + let fields = plugins::installed(diff_cmd::existing_store(&cli.store)?); let reads = each_drive::read(a.drive, |drive| { let (filter, playlist, sort) = (filter.clone(), a.playlist.clone(), a.sort.clone()); let req = api::TracksQuery { library: drive.into(), filter, playlist, sort, offset: 0, limit: a.limit }; diff --git a/apps/locallibrary/src/plugin_cmd/view.rs b/apps/locallibrary/src/plugin_cmd/view.rs index 28126bfe7..1d40cb733 100644 --- a/apps/locallibrary/src/plugin_cmd/view.rs +++ b/apps/locallibrary/src/plugin_cmd/view.rs @@ -232,13 +232,19 @@ pub(crate) fn view(v: &View, names: Names<'_>) { /// A view under `title`, a section of what the CLI shows (a panel under a track's details): the /// title (and the view's, after it), then its lines, saying so when it shows nothing. pub(crate) fn section(title: &str, v: &View, names: Names<'_>) { - heading(&v.title().map_or_else(|| title.to_string(), |t| format!("{title} · {t}"))); + heading(&titled(title, v.title())); match says_nothing(v) { true => draw(&[Drawn::Missing]), false => draw(&lines(v, names)), } } +/// A panel's `title` with its view's (`view`), after it, when it has one: how the CLI and the +/// player head a panel. +pub(crate) fn titled(title: &str, view: Option<&str>) -> String { + view.map_or_else(|| title.to_string(), |v| format!("{title} · {v}")) +} + /// Lines as the CLI draws them: section titles in bold; a table's headings, details and what's /// missing dim; an item indented two spaces a level, its detail beside it. fn draw(lines: &[Drawn]) { diff --git a/apps/locallibrary/src/plugins.rs b/apps/locallibrary/src/plugins.rs index 2d0948b7e..21f760c87 100644 --- a/apps/locallibrary/src/plugins.rs +++ b/apps/locallibrary/src/plugins.rs @@ -338,31 +338,10 @@ impl Reads for SidecarReads { } } -/// The analyzers installed and what they worked out, for filters and sorts naming their fields -/// (`sound.bpm:>120`): kept in `store`. Plugins that were refused have none. -pub struct Installed { - store: Option, - analyzers: Vec, - declared: locallibrary::fields::Declared, -} - -impl Installed { - pub fn with(store: Option) -> Installed { - let host = host().ok(); - Installed { - store, - analyzers: host.map(Plugins::analyzers).unwrap_or_default(), - declared: host.map(Plugins::fields).unwrap_or_default(), - } - } - - pub fn source(&self) -> locallibrary::fields::Source<'_> { - locallibrary::fields::Source { - store: self.store.as_ref(), - analyzers: &self.analyzers, - declared: &self.declared, - } - } +/// The analyzers installed and what they worked out, kept in `store`, for filters and sorts +/// naming their fields (`sound.bpm:>120`). +pub fn installed(store: Option) -> locallibrary::fields::Installed { + locallibrary::fields::Installed::new(store, host().ok()) } /// What analyzers worked out for a track on a drive, kept in `store`, that a page shows: each diff --git a/apps/locallibrary/src/rpc/methods.rs b/apps/locallibrary/src/rpc/methods.rs index 27d22a5a6..03766639f 100644 --- a/apps/locallibrary/src/rpc/methods.rs +++ b/apps/locallibrary/src/rpc/methods.rs @@ -505,7 +505,7 @@ fn build() -> Registry { r.read("playlists.tree", |_, req| Ok(api::playlists_tree(&LIBRARIES, &req)?)).quick(); r.read("tracks.get", |_, req| Ok(api::tracks_get(&LIBRARIES, &req)?)).quick(); r.read("tracks.query", |c, req| { - let fields = crate::plugins::Installed::with(c.existing()?); + let fields = crate::plugins::installed(c.existing()?); Ok(api::tracks_query_with(&LIBRARIES, &req, Some(fields.source()))?) }) .quick(); diff --git a/apps/locallibrary/src/tui/app.rs b/apps/locallibrary/src/tui/app.rs index 5a1020854..b442cdf39 100644 --- a/apps/locallibrary/src/tui/app.rs +++ b/apps/locallibrary/src/tui/app.rs @@ -34,7 +34,7 @@ use super::crate_list::{Crate, CrateTrack}; use super::drives::{DriveKey, DriveRow}; use super::edits::{Action, Job, Menu, Modal, Pick}; use super::keymap::{self, Act}; -use super::keys::{self, Camelot, Relation}; +use super::keys::{Camelot, Relation}; use super::state::{self, Saved, SavedCrateTrack, SavedQueue}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -411,7 +411,7 @@ pub struct Mix { impl Matches { pub(super) fn find(library: Arc, root: PathBuf, track: &Track, limit: usize) -> Matches { - let items = keys::suggestions(&library, track, limit) + let items = api::matches_in(&library, track, limit) .into_iter() .filter_map(|s| { let index = library.tracks.iter().position(|t| t.id == s.track.id)?; @@ -462,7 +462,7 @@ pub struct TrackInfo { fn track_info(root: &Path, track: &Track, library: Option>, store_root: &Path) -> TrackInfo { let device = Device::open(root).ok(); - let mut analysis = device.as_ref().map(|d| anlz::load(d, track)).unwrap_or_default(); + let mut analysis = device.as_ref().map(|d| api::analysis_of(d, track)).unwrap_or_default(); let analyzers = crate::plugins::host().map(|p| p.analyzers()).unwrap_or_default(); let audio = device.as_ref().and_then(|d| d.checked_path(&paths::key(&track.file_path)).ok()); let from_plugins = audio.filter(|_| !analyzers.is_empty()).and_then(|audio| { @@ -2285,7 +2285,7 @@ impl App { let store_root = self.store_root.clone(); let job = move || { let read = Device::open(&root).map(|device| { - let installed = crate::plugins::Installed::with(Store::open(&store_root).ok()); + let installed = crate::plugins::installed(Store::open(&store_root).ok()); Box::new(TrackValues::read(installed.source(), &device, &library)) }); Msg::Fields(generation, read.map_err(|e| e.to_string())) diff --git a/apps/locallibrary/src/tui/edits.rs b/apps/locallibrary/src/tui/edits.rs index 8f9ab10b0..871f347e0 100644 --- a/apps/locallibrary/src/tui/edits.rs +++ b/apps/locallibrary/src/tui/edits.rs @@ -20,7 +20,7 @@ use locallibrary::model::TrackColor; use locallibrary::op::{AnyPlanned, Planned}; use locallibrary::plugin::PluginExport; use locallibrary::progress::{Event, Phase, Progress}; -use locallibrary::query::{SortField, SortKey, TrackSelector}; +use locallibrary::query::{SortKey, TrackSelector}; use locallibrary::{CancelToken, Ctx, Error}; use loclib_plugin::Where; use ratatui::crossterm::event::{KeyCode, KeyEvent}; @@ -151,8 +151,6 @@ pub struct Target { pub crate_total: usize, /// The drive was unplugged. pub stale: bool, - /// The drive is a folder opened by path, not one plugged in: plugins can't name it. - pub folder_drive: bool, } pub struct MenuItem { @@ -242,12 +240,7 @@ pub fn menu_items(t: &Target) -> Vec { _ => "sort this playlist on the drive".into(), }, Action::SortList, - stale.clone().or(not_list).or(because(t.sort.is_none(), "sort the list first (b, or click a column)")).or( - because( - t.sort.is_some_and(|k| matches!(k.field, SortField::Field(_))), - "a plugin's column can't sort the drive's playlists", - ), - ), + stale.clone().or(not_list).or(because(t.sort.is_none(), "sort the list first (b, or click a column)")), ), item("N", format!("new playlist{into}…"), Action::NewPlaylist, stale.clone()), item("F", format!("new folder{into}…"), Action::NewFolder, stale.clone()), @@ -279,7 +272,7 @@ pub fn menu_items(t: &Target) -> Vec { item("y", keymap::name(Act::Replay).into(), Action::Replay, stale.clone()), ] .into_iter() - .chain(super::plugins::offered(Some(!t.folder_drive), &[Where::Track, Where::Playlist]).into_iter().map(|o| { + .chain(super::plugins::offered(true, &[Where::Track, Where::Playlist]).into_iter().map(|o| { // What it works on here: the tracks, or the playlist (not a folder). let on_tracks = o.offered.contains(&Where::Track) && !t.tracks.is_empty(); let on_list = o.offered.contains(&Where::Playlist) && t.node.as_ref().is_some_and(|(_, folder)| !folder); @@ -290,7 +283,7 @@ pub fn menu_items(t: &Target) -> Vec { let unavailable = stale.clone().or(o.unavailable).or(because(!on_tracks && !on_list, nothing)); item("", o.label, Action::Plugin(o.plugin, o.name), unavailable) })) - .chain(super::plugins::providers::fetches(Some(!t.folder_drive)).into_iter().map(|(kind, label, _, why)| { + .chain(super::plugins::providers::fetches(true).into_iter().map(|(kind, label, _, why)| { let unavailable = stale.clone().or(why).or(because(t.tracks.is_empty(), "no track selected")); item("", label, Action::Fetch(kind), unavailable) })) @@ -619,7 +612,6 @@ impl App { crated, crate_total: self.crate_list.len(), stale: l.stale, - folder_drive: self.pluggable(Some(l.root())) == Some(false), }) } diff --git a/apps/locallibrary/src/tui/mix.rs b/apps/locallibrary/src/tui/mix.rs index 0e43d8eb1..bc3b99ac0 100644 --- a/apps/locallibrary/src/tui/mix.rs +++ b/apps/locallibrary/src/tui/mix.rs @@ -10,7 +10,7 @@ use std::sync::mpsc::Sender; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; -use locallibrary::anlz::{self, TrackAnalysis, Waveform}; +use locallibrary::anlz::{TrackAnalysis, Waveform}; use locallibrary::api; use locallibrary::mix::export; use locallibrary::mix::index::Cache; @@ -507,7 +507,7 @@ fn identify( let store = Store::open_or_init(store_root)?; let id = Identifier::with_library(&device, library.clone(), &Cache::in_store(&store), &ctx)?; let report = id.identify(path, &mix::Options::default(), &ctx)?; - let analyses = report.plays.iter().map(|p| anlz::load(&device, &p.track)).collect(); + let analyses = report.plays.iter().map(|p| api::analysis_of(&device, &p.track)).collect(); let cover = report.mix.cover.as_ref().and_then(|c| c.read().ok()).and_then(|b| Art::decode(&b)); // The player plays the recording itself; for a project, the audio exported from it. let audio = match mix::aup::is_project(&report.mix.path) { diff --git a/apps/locallibrary/src/tui/mytags/tests.rs b/apps/locallibrary/src/tui/mytags/tests.rs index a50777c20..a4a80b923 100644 --- a/apps/locallibrary/src/tui/mytags/tests.rs +++ b/apps/locallibrary/src/tui/mytags/tests.rs @@ -90,7 +90,7 @@ fn lists_the_rules_in_the_store() { limit: Some(20), drives: vec![uuid::Uuid::new_v4()], }; - all.set(rule).unwrap(); + all.set(rule, &Default::default()).unwrap(); store.save_smart_playlists(&all).unwrap(); let lines = rules(&mut app); assert_eq!(lines[..2], ["Smart/Peak", "bpm:>=128 · sorted by -bpm · at most 20 · some libraries only"]); diff --git a/apps/locallibrary/src/tui/plugins.rs b/apps/locallibrary/src/tui/plugins.rs index 6ad7fb01d..b1167be64 100644 --- a/apps/locallibrary/src/tui/plugins.rs +++ b/apps/locallibrary/src/tui/plugins.rs @@ -104,16 +104,12 @@ pub struct Offered { pub unavailable: Option, } -/// Plugins' commands and operations offered in a menu of one of `places`, as it offers them on a -/// drive: `drive` is `None` without one, `Some(false)` for a folder opened by path, which a plugin -/// can't name. An operation always needs a drive; a command, when its plugin reads one. -pub fn offered(drive: Option, places: &[Where]) -> Vec { +/// Plugins' commands and operations offered in a menu of one of `places`, as it offers them with +/// a `drive` open or without one. An operation always needs a drive; a command, when its plugin +/// reads one. +pub fn offered(drive: bool, places: &[Where]) -> Vec { let Ok(plugins) = crate::plugins::host() else { return Vec::new() }; - let why = match drive { - None => Some("open a drive first"), - Some(false) => Some("only on a plugged-in drive"), - Some(true) => None, - }; + let why = (!drive).then_some("open a drive first"); let mut all = Vec::new(); for m in plugins.manifests() { let c = &m.contributes; @@ -249,17 +245,11 @@ impl App { self.loaded.as_ref()?.library.track(id).map(|t| t.display_name()) } - /// Whether plugins can run on the drive at `root`: one plugged in, rather than a folder - /// opened by path. `None` without a drive. - pub(super) fn pluggable(&self, root: Option<&Path>) -> Option { - root.map(|root| !self.extra_roots.iter().any(|r| r == root)) - } - /// Every plugin's commands and operations offered in a menu of one of `places`, analyzers, /// export and import formats, as the palette and the drive menu list them for the drive at /// `root`: by title, with the plugin's name beside each. pub(super) fn plugin_items(&self, root: Option<&Path>, places: &[Where]) -> Vec<(Act, Item)> { - let drive = self.pluggable(root); + let drive = root.is_some(); let all = offered(drive, places).into_iter(); let mut items: Vec<(Act, Item)> = all .map(|o| (Act::Plugin(o.plugin, o.name), Item { unavailable: o.unavailable, ..Item::new(o.label, o.by) })) @@ -274,12 +264,8 @@ impl App { } let Ok(plugins) = crate::plugins::host() else { return items }; // An analyzer reads the tracks' audio files, an exporter the library, an importer's file is - // about its tracks and a library source writes one: on a drive plugged in, as a command. - let why = match drive { - None => Some("open a drive first"), - Some(false) => Some("only on a plugged-in drive"), - Some(true) => None, - }; + // about its tracks and a library source writes one: on a drive, as a command. + let why = (!drive).then_some("open a drive first"); // Each family's, in the order the palette lists them (a family's placed as loclib-plugin's // `hooks` says). // A set's formats run on the set open: the palette names them anyway, saying so, and a @@ -294,19 +280,12 @@ impl App { items } - /// The drive at `root`, when plugins can run on it; else `None`, saying why. - fn plugged(&mut self, root: Option) -> Option { - match (self.pluggable(root.as_deref()), root) { - (Some(true), Some(root)) => Some(root), - (None, _) => { - self.toast("open a drive first", Level::Info); - None - } - _ => { - self.toast("only on a plugged-in drive", Level::Info); - None - } + /// The drive at `root`, when there's one; else `None`, saying so. + fn drive_open(&mut self, root: Option) -> Option { + if root.is_none() { + self.toast("open a drive first", Level::Info); } + root } /// The settings of each plugin that has some (or a service) and signing in to and out of its @@ -372,7 +351,12 @@ impl App { 0 => "nothing changed".to_string(), _ => format!("{} settings saved", m.name), }; - Ok(Box::new(move |app: &mut App| app.toast(said, Level::Info))) + Ok(Box::new(move |app: &mut App| { + if changed > 0 { + app.panels.stale(plugin); + } + app.toast(said, Level::Info) + })) }); } diff --git a/apps/locallibrary/src/tui/plugins/analyzers.rs b/apps/locallibrary/src/tui/plugins/analyzers.rs index 7a4a55712..5fe183c3a 100644 --- a/apps/locallibrary/src/tui/plugins/analyzers.rs +++ b/apps/locallibrary/src/tui/plugins/analyzers.rs @@ -79,7 +79,7 @@ impl App { tracks: Option>, ) { let Ok(plugins) = crate::plugins::host() else { return }; - let Some(root) = self.plugged(root) else { return }; + let Some(root) = self.drive_open(root) else { return }; let Some(declared) = plugins.manifest(plugin).ok().and_then(|m| m.contributes.analyzers.iter().find(|a| a.name == analyzer)) else { @@ -117,7 +117,7 @@ impl App { /// it must be; then the plan comes back to confirm, as any edit's does. pub(in crate::tui) fn apply_fields(&mut self, plugin: &'static str, analyzer: &'static str, root: Option) { let Ok(plugins) = crate::plugins::host() else { return }; - let Some(root) = self.plugged(root) else { return }; + let Some(root) = self.drive_open(root) else { return }; let declared = plugins.manifest(plugin).ok().and_then(|m| m.contributes.analyzers.iter().find(|a| a.name == analyzer)); let Some(a) = declared else { return }; diff --git a/apps/locallibrary/src/tui/plugins/events.rs b/apps/locallibrary/src/tui/plugins/events.rs index 2d5d8f8f2..96619cb2c 100644 --- a/apps/locallibrary/src/tui/plugins/events.rs +++ b/apps/locallibrary/src/tui/plugins/events.rs @@ -146,7 +146,7 @@ impl App { Some(Err(why)) => return self.toast(why, Level::Info), None => self.action_drive(), }; - let Some(root) = self.plugged(root) else { return }; + let Some(root) = self.drive_open(root) else { return }; let label = self.drive_label(&root); if let (_, Some(tracks)) = action.scope_mut() && !tracks.is_empty() diff --git a/apps/locallibrary/src/tui/plugins/exporters.rs b/apps/locallibrary/src/tui/plugins/exporters.rs index 0cf83d3cc..2e974b2e8 100644 --- a/apps/locallibrary/src/tui/plugins/exporters.rs +++ b/apps/locallibrary/src/tui/plugins/exporters.rs @@ -90,7 +90,7 @@ impl App { suggested: Option<&ExportAction>, ) { let Ok(plugins) = crate::plugins::host() else { return }; - let Some(root) = self.plugged(root) else { return }; + let Some(root) = self.drive_open(root) else { return }; let exporter = plugins.manifest(plugin).ok().and_then(|m| m.contributes.exporters.iter().find(|e| e.format == format)); let Some(e) = exporter else { return }; diff --git a/apps/locallibrary/src/tui/plugins/importers.rs b/apps/locallibrary/src/tui/plugins/importers.rs index 2bc56aa29..fce486af0 100644 --- a/apps/locallibrary/src/tui/plugins/importers.rs +++ b/apps/locallibrary/src/tui/plugins/importers.rs @@ -47,7 +47,7 @@ impl App { /// (on this computer) and the format's options first. pub(in crate::tui) fn plugin_import(&mut self, plugin: &'static str, format: &'static str, root: Option) { let Ok(plugins) = crate::plugins::host() else { return }; - let Some(root) = self.plugged(root) else { return }; + let Some(root) = self.drive_open(root) else { return }; let importer = plugins.manifest(plugin).ok().and_then(|m| m.contributes.importers.iter().find(|i| i.format == format)); let Some(i) = importer else { return }; diff --git a/apps/locallibrary/src/tui/plugins/panels.rs b/apps/locallibrary/src/tui/plugins/panels.rs index 8b9f29c85..70f559eb6 100644 --- a/apps/locallibrary/src/tui/plugins/panels.rs +++ b/apps/locallibrary/src/tui/plugins/panels.rs @@ -67,14 +67,14 @@ pub struct Shown { pub enum State { Loading, - /// It can't be asked here, and why (a folder opened by path, which a plugin can't name). - Unavailable(&'static str), Failed, Ready(Box), } /// A panel's view, as the player draws it: worked out in the background. pub struct Ready { + /// The view's own title, if it has one: shown after the panel's. + pub title: Option, /// It as lines: what's drawn, but for a picture or a chart. pub lines: Vec<(Tone, String)>, pub art: Option, @@ -105,7 +105,7 @@ impl Ready { View::Chart { style, series, .. } if !view.is_empty() => Some(Plot::of(*style, series)), _ => None, }; - Ready { lines, art, chart } + Ready { title: view.title().map(String::from), lines, art, chart } } /// How many rows its body takes: all there are for a picture or a chart it draws, else a row @@ -171,6 +171,15 @@ impl Panels { pub fn of_track(&self, selected: (u64, usize)) -> bool { self.track == Some(selected) } + + /// The panels of `plugin` may show otherwise now (its settings changed): the track's panels + /// are asked for again, each in the room its last answer took. + pub fn stale(&mut self, plugin: &str) { + if self.shown.iter().any(|s| s.plugin == plugin) { + self.cancel.cancel(); + self.track = None; + } + } } impl App { @@ -205,7 +214,6 @@ impl App { } } let (library, track) = (l.library.clone(), l.library.tracks[i].id); - let why = (self.pluggable(Some(&root)) == Some(false)).then_some("only on a plugged-in drive"); self.panels.cancel.cancel(); self.panels.cancel = CancelToken::new(); let last = |plugin: &str, panel: &str| { @@ -217,14 +225,11 @@ impl App { panel: &p.name, title: &p.title, by: plugins.manifest(plugin).map_or("", |m| m.name.as_str()), - state: why.map_or(State::Loading, State::Unavailable), + state: State::Loading, rows: last(plugin, &p.name), }); self.panels.shown = shown.collect(); self.panels.track = Some((generation, i)); - if why.is_some() { - return; - } for (at, (plugin, p)) in all.into_iter().enumerate() { let (cancel, store_root, root, library) = (self.panels.cancel.clone(), self.store_root.clone(), root.clone(), library.clone()); @@ -276,9 +281,9 @@ pub fn height(app: &App, room: u16) -> u16 { }) } -/// The selected track's panels in `area`, one after another, each under its title and plugin's -/// name: what it shows (a picture, a chart, else its lines, cut at the edge), or that it's -/// coming, failed or can't be shown here. Another track's are drawn as coming, in their room. +/// The selected track's panels in `area`, one after another, each under its title (and its view's, +/// once it's shown) and plugin's name: what it shows (a picture, a chart, else its lines, cut at the edge), or that it's +/// coming or failed. Another track's are drawn as coming, in their room. pub fn draw(f: &mut Frame, app: &mut App, area: Rect) { let current = app.selected_key().is_some_and(|k| app.panels.of_track(k)); // Before any track's were asked for: each coming, in a row. @@ -305,8 +310,12 @@ pub fn draw(f: &mut Frame, app: &mut App, area: Rect) { } let r = Rect { y, height: (1 + shown.rows).min(area.bottom() - y), ..area }; y += r.height; - let heading = - vec![Span::styled(format!(" {} ", shown.title), dim()), Span::styled(format!("{} ", shown.by), faint())]; + let view = match &shown.state { + State::Ready(ready) if current => ready.title.as_deref(), + _ => None, + }; + let title = crate::plugin_cmd::view::titled(shown.title, view); + let heading = vec![Span::styled(format!(" {title} "), dim()), Span::styled(format!("{} ", shown.by), faint())]; let body = section_with(f, r, Line::from(heading)); let said = |f: &mut Frame, why: &str| { f.render_widget(Paragraph::new(Line::styled(clip(why, body.width as usize), dim())), body) @@ -314,7 +323,6 @@ pub fn draw(f: &mut Frame, app: &mut App, area: Rect) { match &mut shown.state { _ if !current => said(f, "loading…"), State::Loading => said(f, "loading…"), - State::Unavailable(why) => said(f, why), State::Failed => said(f, "couldn't show it (! says why)"), State::Ready(ready) => draw_ready(f, ready, body), } diff --git a/apps/locallibrary/src/tui/plugins/panels/tests.rs b/apps/locallibrary/src/tui/plugins/panels/tests.rs index 37c489791..65fca5607 100644 --- a/apps/locallibrary/src/tui/plugins/panels/tests.rs +++ b/apps/locallibrary/src/tui/plugins/panels/tests.rs @@ -108,7 +108,27 @@ fn coming_back_to_a_track_asks_for_its_panels_again() { assert!(s.contains("back again"), "{s}"); } -/// A panel that fails says so, the messages why; one on a folder opened by path can't be asked. +/// A panel is headed with its view's own title after its own, as `track show` heads it; once its +/// plugin's settings change, the track's panels are asked for again, without moving off it. +#[test] +fn a_panel_shows_its_views_title_and_follows_its_settings() { + let (dir, root) = demo_drive(); + said_plugin(dir.path()); + let mut app = showing(&root); + let view = |text: &str| json!({ "view": { "kind": "text", "title": "Loudness", "text": text } }); + saying(&app, view("first")); + settled(&mut app); + wait(&mut app, answered); + let s = screen(&mut app, 160, 45); + assert!(s.contains("As it says · Loudness") && s.contains("first"), "{s}"); + app.save_settings("echo", json!({ "panel": view("second").to_string() })); + wait(&mut app, |a| a.job.is_none() && answered(a) && a.selected_key().is_some_and(|k| a.panels.of_track(k))); + let s = screen(&mut app, 160, 45); + assert!(s.contains("second") && !s.contains("first"), "{s}"); +} + +/// A panel that fails says so, the messages why; one on a folder opened by path is asked as on a +/// drive. #[test] fn a_panel_that_cant_show_says_why() { let (dir, root) = demo_drive(); @@ -122,9 +142,11 @@ fn a_panel_that_cant_show_says_why() { assert!(app.log.iter().any(|(m, ..)| m.starts_with("As it says panel: "))); let mut app = showing(&root); app.extra_roots.push(app.loaded.as_ref().unwrap().root().to_path_buf()); + saying(&app, json!({ "view": { "kind": "text", "text": "from the folder" } })); settled(&mut app); - assert!(matches!(app.panels.shown[0].state, State::Unavailable(_))); - assert!(screen(&mut app, 160, 45).contains("only on a plugged-in drive")); + wait(&mut app, answered); + let s = screen(&mut app, 160, 45); + assert!(s.contains("from the folder"), "{s}"); } /// A panel's picture is drawn in colour (without colours, what it shows is said), and its chart diff --git a/apps/locallibrary/src/tui/plugins/providers.rs b/apps/locallibrary/src/tui/plugins/providers.rs index 6a1c3f92c..b9dea8b43 100644 --- a/apps/locallibrary/src/tui/plugins/providers.rs +++ b/apps/locallibrary/src/tui/plugins/providers.rs @@ -33,13 +33,9 @@ pub struct Fetching { /// What providers fetch for the tracks selected, as a menu offers it on a drive (see /// [`super::offered`]): each kind some plugin provides, with its providers' plugins' names, and /// why it can't run here, if it can't. -pub fn fetches(drive: Option) -> Vec<(ProviderKind, String, String, Option)> { +pub fn fetches(drive: bool) -> Vec<(ProviderKind, String, String, Option)> { let Ok(plugins) = crate::plugins::host() else { return Vec::new() }; - let why = match drive { - None => Some("open a drive first"), - Some(false) => Some("only on a plugged-in drive"), - Some(true) => None, - }; + let why = (!drive).then_some("open a drive first"); let kinds = [ProviderKind::Artwork, ProviderKind::Metadata]; let found = kinds.into_iter().filter_map(|kind| { let by = providing(plugins, Some(kind))?; @@ -98,7 +94,7 @@ impl App { /// What providers of `kind` find for the tracks `target` selects on the drive at `root`: a /// form asks which provider (any, to start with) and whether over what tracks have first. pub(in crate::tui) fn fetch_form(&mut self, kind: ProviderKind, root: Option, target: Option<&Target>) { - let Some(root) = self.plugged(root) else { return }; + let Some(root) = self.drive_open(root) else { return }; let tracks = target.map(|t| self.context(t).tracks).unwrap_or_default(); if tracks.is_empty() { return self.toast("no track selected", Level::Info); diff --git a/apps/locallibrary/src/tui/plugins/sources.rs b/apps/locallibrary/src/tui/plugins/sources.rs index c7bd31336..756151413 100644 --- a/apps/locallibrary/src/tui/plugins/sources.rs +++ b/apps/locallibrary/src/tui/plugins/sources.rs @@ -35,7 +35,7 @@ impl App { /// may read files in (`~/Music` to begin with) and for the source's options first. pub(in crate::tui) fn plugin_source(&mut self, plugin: &'static str, source: &'static str, root: Option) { let Ok(plugins) = crate::plugins::host() else { return }; - let Some(root) = self.plugged(root) else { return }; + let Some(root) = self.drive_open(root) else { return }; let declared = plugins.manifest(plugin).ok().and_then(|m| m.contributes.sources.iter().find(|s| s.name == source)); let Some(s) = declared else { return }; diff --git a/apps/locallibrary/src/tui/plugins/tests.rs b/apps/locallibrary/src/tui/plugins/tests.rs index b1c7a5b9c..4591484fa 100644 --- a/apps/locallibrary/src/tui/plugins/tests.rs +++ b/apps/locallibrary/src/tui/plugins/tests.rs @@ -36,8 +36,8 @@ fn panel(app: &App) -> String { } /// Every plugin's commands are in the palette, by title with the plugin's name, after the -/// player's own; those of a plugin reading a drive say why they can't run without one, or on a -/// drive opened as a folder (which a plugin can't name). +/// player's own; those of a plugin reading a drive say why they can't run without one, and run on +/// a folder opened by path as on a drive. #[test] fn plugins_commands_are_in_the_palette() { let (dir, root) = demo_drive(); @@ -57,7 +57,7 @@ fn plugins_commands_are_in_the_palette() { press(&mut app, "\u{1b}"); app.extra_roots.push(root.clone()); press(&mut app, ":"); - assert_eq!(entry(&app, "answer the params…").1, Some("only on a plugged-in drive")); + assert_eq!(entry(&app, "answer the params…").1, None); } /// The drive menu ends with plugins' commands, operations, analyzers, export and import formats, @@ -406,7 +406,7 @@ fn operations_are_offered_where_they_can_run() { app.loaded.as_mut().unwrap().stale = false; app.extra_roots.push(root.clone()); press(&mut app, "e"); - assert_eq!(menu(&app)[echo].1.as_deref(), Some("only on a plugged-in drive")); + assert_eq!(menu(&app)[echo].1, None, "a folder opened by path is a drive here"); } /// An operation from the actions menu: its form starts with the marked tracks, it's planned in @@ -621,7 +621,7 @@ fn the_search_box_filters_by_plugins_fields() { } /// A plugin's column sorts by its values when its header's clicked, tracks without one last -/// either way; a playlist can't be sorted on the drive by it. +/// either way; a playlist is sorted on the drive by it as by any column. #[test] fn plugins_columns_sort_by_their_values() { let (_dir, mut app) = sized(); @@ -642,12 +642,17 @@ fn plugins_columns_sort_by_their_values() { assert!(screen(&mut app, 120, 30).contains("echo.bytes↑")); super::super::app::testing::mouse(&mut app, super::super::app::testing::LEFT, header); assert_eq!(shown(&app), [second, first], "the one without a value stays last"); - // Shown a playlist, it can't be sorted on the drive by it. + // Shown a playlist, it's sorted on the drive by it. let mut t = app.target().unwrap(); t.list = Some("Sets/Friday".into()); let items = super::super::edits::menu_items(&t); let sort = items.iter().find(|i| i.action == super::super::edits::Action::SortList).unwrap(); - assert_eq!(sort.unavailable.as_deref(), Some("a plugin's column can't sort the drive's playlists")); + assert_eq!((sort.label.as_str(), sort.unavailable.as_deref()), ("sort Friday by echo.bytes ↓ on the drive", None)); + let by = match super::super::edits::sort("Sets/Friday", t.sort.unwrap()) { + locallibrary::edit::Edit::SortPlaylist { by, .. } => by, + other => panic!("{other:?}"), + }; + assert_eq!(by, "-echo.bytes"); } /// The settings of the plugin open in a form, from the palette. @@ -1022,7 +1027,7 @@ use super::super::app::testing::{Plugged, plugged, plugins_once, settle}; use super::super::palette::Then; /// Where a case runs plugins: with no drive open, on one plugged in, or on a folder opened by -/// path (which a plugin can't name). +/// path (which a plugin reads as it reads a drive). #[derive(Debug, Clone, Copy, PartialEq)] enum Opened { Nowhere, @@ -1121,12 +1126,11 @@ fn form_open(app: &App) -> (String, Vec) { #[hegel::test(test_cases = 20, suppress_health_check = [hegel::HealthCheck::TooSlow])] fn the_palettes_plugin_entries_do_what_they_say(tc: TestCase) { let (at, mut app, drive, _store) = somewhere(&tc); - // Every plugin here reads the drive: its hooks need one plugged in, its settings and columns - // don't. + // Every plugin here reads the drive: its hooks need one open (a folder opened by path too), + // its settings and columns don't. let why = match at { Opened::Nowhere => Some("open a drive first"), - Opened::Folder => Some("only on a plugged-in drive"), - Opened::Plugged => None, + Opened::Folder | Opened::Plugged => None, }; let in_menu = drive.is_some() && tc.draw(gs::booleans()); if let Some(d) = drive.as_ref().filter(|_| in_menu) { diff --git a/apps/locallibrary/src/tui/plugins/tests/events.rs b/apps/locallibrary/src/tui/plugins/tests/events.rs index 236729e4c..5ce3c122b 100644 --- a/apps/locallibrary/src/tui/plugins/tests/events.rs +++ b/apps/locallibrary/src/tui/plugins/tests/events.rs @@ -390,7 +390,7 @@ fn reviewing_a_suggested_action_starts_nothing_by_itself(tc: hegel::TestCase) { } (None, _) => { let said = last_message(&app); - let why = ["ELSEWHERE isn't plugged in", "open a drive first", "only on a plugged-in drive"]; + let why = ["ELSEWHERE isn't plugged in", "open a drive first"]; let none = !tracks.is_empty() && on.is_empty() && said.starts_with("none of the tracks it suggests are on "); assert!(why.contains(&said) || none, "{said}"); diff --git a/apps/locallibrary/src/tui/props.rs b/apps/locallibrary/src/tui/props.rs index 1ce2230dc..fb20d75c2 100644 --- a/apps/locallibrary/src/tui/props.rs +++ b/apps/locallibrary/src/tui/props.rs @@ -1604,9 +1604,10 @@ fn a_smart_playlist_holds_what_the_search_showed(tc: TestCase) { assert!(app.modal.is_none(), "x didn't close the panel"); } -/// The details panel of whichever track is selected lists what mixes into it, as the library -/// suggests (the other recordings in a key that mixes, near its tempo, best first), names it and -/// the best of them on screen, and enter there queues those tracks from the one picked. +/// The details panel of whichever track is selected lists what mixes into it, as the api's +/// matches (`track.matches`) are (the other recordings in a key that mixes, near its tempo, best +/// first), names it and the best of them on screen, and enter there queues those tracks from the +/// one picked. #[hegel::test(test_cases = 20, suppress_health_check = [HealthCheck::TooSlow])] fn the_details_list_what_mixes_into_the_track(tc: TestCase) { use super::app::testing::{app_with, track}; @@ -1629,7 +1630,7 @@ fn the_details_list_what_mixes_into_the_track(tc: TestCase) { app.on_key(key_event("i")); wait(&mut app, |a| a.details_current()); let d = app.details.as_ref().unwrap(); - let want: Vec = (super::keys::suggestions(&lib, &lib.tracks[at], 60).iter()) + let want: Vec = (locallibrary::api::matches_in(&lib, &lib.tracks[at], 60).iter()) .map(|s| lib.tracks.iter().position(|t| t.id == s.track.id).unwrap()) .collect(); assert_eq!(d.matches.items.iter().map(|m| m.index).collect::>(), want, "for {:?}", lib.tracks[at]); diff --git a/crates/locallibrary/src/api/editing.rs b/crates/locallibrary/src/api/editing.rs index ba2232e97..1c6c10814 100644 --- a/crates/locallibrary/src/api/editing.rs +++ b/crates/locallibrary/src/api/editing.rs @@ -615,7 +615,7 @@ pub fn track_analysis(req: &TrackRequest) -> Result { let d = resolve_drive(&req.drive)?; let lib = d.library()?; let t = lib.track(req.track).ok_or_else(|| Error::no_track(req.track))?; - Ok(anlz::load(&d, t)) + Ok(super::analysis_of(&d, t)) } #[derive(Debug, Clone, Deserialize, JsonSchema)] @@ -742,7 +742,7 @@ pub fn track_details_cached(cache: &LibraryCache, req: &TrackDetailsRequest) -> copies: fingerprint(t) .and_then(|k| recordings.get(&k)) .map_or_else(Vec::new, |v| v.iter().filter(|o| o.id != t.id).map(|o| TrackRef::from(*o)).collect()), - analysis: AnalysisSummary::of(&anlz::load(&d, t), analysed), + analysis: AnalysisSummary::of(&super::analysis_of(&d, t), analysed), track: t.clone(), } }) @@ -769,6 +769,12 @@ pub struct HarmonicMatch { pub tempo_gap: f64, } +/// The tracks of `library` that mix into `track` ([`harmony::suggestions`]), at most `limit`, best +/// first: what `track.matches` and the player list. +pub fn matches_in(library: &Library, track: &Track, limit: usize) -> Vec { + harmony::suggestions(library, track, limit) +} + /// Tracks that mix into a track: a compatible key and a close tempo, best first. pub fn harmonic_matches(req: &MatchesRequest) -> Result> { harmonic_matches_cached(LibraryCache::none(), req) @@ -778,7 +784,7 @@ pub fn harmonic_matches(req: &MatchesRequest) -> Result> { pub fn harmonic_matches_cached(cache: &LibraryCache, req: &MatchesRequest) -> Result> { let (_, _, lib) = cache.read(&req.library)?; let t = lib.track(req.track).ok_or_else(|| Error::no_track(req.track))?; - Ok(harmony::suggestions(&lib, t, req.limit.unwrap_or(40)) + Ok(matches_in(&lib, t, req.limit.unwrap_or(40)) .into_iter() .map(|s| HarmonicMatch { track: s.track, @@ -912,13 +918,14 @@ pub fn smart_set(store: &Store, req: &SetSmartPlaylist) -> Result, } +/// A track's beat grid, cues, waveforms and phrases on `device`: what `track.analysis`, +/// `tracks.analysis`, `track.show` and the player read. +pub fn analysis_of(device: &Device, track: &Track) -> TrackAnalysis { + anlz::load(device, track) +} + impl AnalysisView { /// The track's analysis on `device`, with the waveforms chosen and encoded as asked. fn of(device: &Device, track: &Track, waveform: WaveformChoice, encoding: WaveformEncoding) -> AnalysisView { - let a: TrackAnalysis = anlz::load(device, track); + let a = analysis_of(device, track); let encode = |w: Waveform| match encoding { WaveformEncoding::Json => WaveformData::Json(w), WaveformEncoding::Base64 => WaveformData::Packed(PackedWaveform::pack(&w)), diff --git a/crates/locallibrary/src/edit.rs b/crates/locallibrary/src/edit.rs index 238a00caa..1c8fdedbb 100644 --- a/crates/locallibrary/src/edit.rs +++ b/crates/locallibrary/src/edit.rs @@ -30,6 +30,7 @@ use crate::commit::{Changes, NewDatabases, NewFile, Options}; use crate::cue_edit::CueChange; use crate::device::{Device, LineageOp}; use crate::error::{Action, Reason}; +use crate::fields::TrackValues; use crate::fsutil; use crate::harmony; use crate::hash::Hash; @@ -38,7 +39,7 @@ use crate::onelibrary::{OneLibrary, Tree}; use crate::paths; use crate::pdb; use crate::progress::{Ctx, Phase}; -use crate::query::{self, DriveContext, TrackSelector}; +use crate::query::{self, DriveContext, Filter, TrackSelector}; use crate::tags::TagChange; use crate::{Error, Result}; @@ -1425,6 +1426,18 @@ impl<'a> Resolver<'a> { kept } + /// Says so when `sel`'s filter names a plugin's field no track here has a value of (none was + /// analyzed for it): the filter selects none of the tracks by it. + fn note_unanalyzed(&self, sel: &TrackSelector) { + let Some(values) = self.extra.fields() else { return }; + let Some(filter) = sel.filter.as_deref().and_then(|f| Filter::parse(f).ok()) else { return }; + for name in filter.fields().into_iter().filter(|name| values.declared.contains_key(*name)) { + if !values.tracks.values().any(|fields| fields.contains_key(name)) { + self.note(format!("no track here has a value for {name}: analyze the tracks first")); + } + } + } + fn keys(lib: &Library, ids: &[u32]) -> Vec { lib.tracks_by_id(ids).into_iter().map(|t| paths::file_key(&t.file_path)).collect() } @@ -1434,6 +1447,7 @@ impl<'a> Resolver<'a> { } fn select(&self, lib: &Library, sel: &TrackSelector, allow_empty: bool) -> Result> { + self.note_unanalyzed(sel); let ctx = self.extra.query(lib); if self.lenient && !sel.paths.is_empty() { let paths = self.present(lib, &sel.paths, "the edit selects"); @@ -1607,7 +1621,7 @@ impl<'a> Resolver<'a> { let p = list(spec)?; let mut add = self.select(lib, tracks, false)?; if let Some(s) = sort { - add = sorted(lib, &add, s)?; + add = sorted(self.extra.fields(), lib, &add, s)?; } let old = entries(p); if !allow_repeats { @@ -1718,7 +1732,7 @@ impl<'a> Resolver<'a> { Edit::SetTracks { playlist: spec, tracks, sort, limit, create } => { let mut ids = self.select(lib, tracks, true)?; if let Some(s) = sort { - ids = sorted(lib, &ids, s)?; + ids = sorted(self.extra.fields(), lib, &ids, s)?; } if let Some(n) = limit { ids.truncate(*n); @@ -1775,7 +1789,7 @@ impl<'a> Resolver<'a> { } Edit::SortPlaylist { playlist: spec, by } => { let p = list(spec)?; - let new = sorted(lib, &entries(p), by)?; + let new = sorted(self.extra.fields(), lib, &entries(p), by)?; let canonical = Edit::SortPlaylist { playlist: named(p), by: by.clone() }; (vec![set(p, &new)], format!("sort {} by {by}", lib.playlist_path(p.id)), canonical) } @@ -2105,10 +2119,12 @@ impl<'a> Resolver<'a> { } } -fn sorted(lib: &Library, ids: &[u32], by: &str) -> Result> { - let keys = query::parse_sort(by)?; +/// `ids` sorted by `by` (see [`query::parse_sort`]), a plugin's field by what the analyzers +/// installed worked out (`fields`, read for a sort naming one). +fn sorted(fields: Option<&TrackValues>, lib: &Library, ids: &[u32], by: &str) -> Result> { + let keys = query::parse_sort_with(by, fields.map(|f| &f.declared))?; let mut tracks = lib.tracks_by_id(ids); - query::sort_tracks(&mut tracks, &keys); + query::sort_tracks_with(&mut tracks, &keys, fields); Ok(tracks.iter().map(|t| t.id).collect()) } @@ -2282,7 +2298,8 @@ pub(crate) fn plan_writing( if !opts.lenient { edits = edits.into_iter().flat_map(with_tempo).collect(); } - let extra = DriveContext::read(device, edits.iter().flat_map(Edit::selectors))?; + let extra = + DriveContext::read(device, edits.iter().flat_map(Edit::selectors), edits.iter().filter_map(Edit::sort))?; plan_with(device, &edits, opts, &extra, incoming, ctx) } @@ -2302,6 +2319,15 @@ impl Edit { matches!(self, Edit::MyTag(_) | Edit::Cues { .. } | Edit::SetArtwork { .. } | Edit::SetAnalysis { .. }) } + /// The order it sorts tracks in, if it sorts them (see [`query::parse_sort`]). + fn sort(&self) -> Option<&str> { + match self { + Edit::AddTracks { sort, .. } | Edit::SetTracks { sort, .. } => sort.as_deref(), + Edit::SortPlaylist { by, .. } => Some(by), + _ => None, + } + } + /// The track selections the edit makes. fn selectors(&self) -> Vec<&TrackSelector> { match self { diff --git a/crates/locallibrary/src/fields.rs b/crates/locallibrary/src/fields.rs index 2ca65743f..a2de3d830 100644 --- a/crates/locallibrary/src/fields.rs +++ b/crates/locallibrary/src/fields.rs @@ -146,6 +146,37 @@ pub struct Source<'a> { pub declared: &'a Declared, } +/// The analyzers installed and the fields they declare, and the store what they worked out is +/// kept in: what filters and sorts naming their fields (`sound.bpm:>120`) read. Plugins that were +/// refused have none. +pub struct Installed { + store: Option, + analyzers: Vec, + declared: Declared, +} + +impl Installed { + /// The analyzers of `plugins` (none without), what they worked out kept in `store`. + pub fn new(store: Option, plugins: Option<&crate::plugin::Plugins>) -> Installed { + Installed { + store, + analyzers: plugins.map(crate::plugin::Plugins::analyzers).unwrap_or_default(), + declared: plugins.map(crate::plugin::Plugins::fields).unwrap_or_default(), + } + } + + /// [`Installed::new`] for the plugins the app hosts ([`crate::plugin::set_hosted`]), in the + /// store drive lookups consult ([`crate::api::store_root`]): what edits' selectors and sorts + /// and smart playlists' rules read. + pub(crate) fn hosted() -> Installed { + Installed::new(crate::api::registry(), crate::plugin::hosted()) + } + + pub fn source(&self) -> Source<'_> { + Source { store: self.store.as_ref(), analyzers: &self.analyzers, declared: &self.declared } + } +} + /// What the analyzers installed worked out for a library's tracks, as filters and sorts read it. #[derive(Debug, Clone, Default, PartialEq)] pub struct TrackValues { diff --git a/crates/locallibrary/src/plugin/analyzers.rs b/crates/locallibrary/src/plugin/analyzers.rs index 5972090ae..63eab7324 100644 --- a/crates/locallibrary/src/plugin/analyzers.rs +++ b/crates/locallibrary/src/plugin/analyzers.rs @@ -87,7 +87,7 @@ impl Plugins { ) -> Result { let slot = self.slot(plugin)?; let declared = slot.declared("analyzer", analyzer, &slot.manifest.contributes.analyzers, |a| &a.name)?; - let (context, device) = + let (context, device, _given) = slot.context(Context { drive: Some(drive.into()), ..Context::default() }, Some(store))?; let device = device.expect("a drive was named"); let library = device.library()?; diff --git a/crates/locallibrary/src/plugin/callbacks.rs b/crates/locallibrary/src/plugin/callbacks.rs index 784a9913a..9679c7bfb 100644 --- a/crates/locallibrary/src/plugin/callbacks.rs +++ b/crates/locallibrary/src/plugin/callbacks.rs @@ -1,7 +1,7 @@ //! What plugins read: requests named after the sidecar's read methods, answered by the app's //! (see [`Reads`]) once the host has checked them. A plugin may call only the reads its //! capabilities grant, with only the params listed for each, naming drives the host finds itself -//! (never by path), in pages of a bounded size, a few calls at a time; what it gets back, errors +//! or gave its hooks under way (never by path), in pages of a bounded size, a few calls at a time; what it gets back, errors //! too, says nothing of where things are on this computer (see [`answer_for_plugin`]). A plugin //! with the `net` feature calls `net/fetch` here too (see [`super::net`]). @@ -28,7 +28,8 @@ pub trait Reads: Send + Sync { /// The root of the drive a plugin names: one known here (by identity, adopted name or /// volume name) or a library's lead clone, never a path or a folder - /// ([`api::resolve_known_drive`]). The host gives the app that root. + /// ([`api::resolve_known_drive`]). The host gives the app that root. A drive given to one of + /// the plugin's hooks under way is found before this is asked (see `Plugin::drive`). fn drive(&self, spec: &str) -> Result { Ok(api::resolve_known_drive(spec)?.root().to_path_buf()) } @@ -187,10 +188,10 @@ pub(crate) fn answer( let run = || match req.method.as_str() { method::NET_FETCH if live.sends(method::NET_FETCH) => plugin.fetch(&live, req.params, &req.cancel), _ => { - let params = check(&plugin.manifest, reads, &req.method, req.params); + let params = check(&plugin.manifest, &|spec| plugin.drive(spec), &req.method, req.params); let read = params.and_then(|p| reads.read(&req.method, p, &Ctx::new(&NoProgress, req.cancel.clone()))); - let roots = known_roots(live.store.as_ref()); + let roots = known_roots(live.store.as_ref(), &plugin.given()); read.map(|answer| answer_for_plugin(&req.method, answer, &roots)).map_err(|e| for_plugin(e, &roots)) } }; @@ -204,8 +205,9 @@ pub(crate) fn answer( } /// `params` for the read `method`, once the plugin may call it with them: only params listed -/// for it, each as its [`Param`] says (drives found, lists and pages bounded), or refused. -pub(crate) fn check(manifest: &Manifest, reads: &dyn Reads, method: &str, params: Value) -> Result { +/// for it, each as its [`Param`] says (drives found by `drive`, lists and pages bounded), or +/// refused. +pub(crate) fn check(manifest: &Manifest, drive: Drive<'_>, method: &str, params: Value) -> Result { let Some((capability, library, others)) = allowed(method) else { let e = format!("{method:?} isn't a read a plugin can call"); return Err(RpcFailure::new(code::METHOD_NOT_FOUND, "method_not_found", e)); @@ -221,13 +223,13 @@ pub(crate) fn check(manifest: &Manifest, reads: &dyn Reads, method: &str, params } for (name, param) in listed { match (param, params.get_mut(name)) { - (Param::Drive, Some(v)) => *v = drive(reads, name, v)?, + (Param::Drive, Some(v)) => *v = root(drive, name, v)?, (Param::Drives, Some(Value::Array(list))) => { for v in list { - *v = drive(reads, name, v)?; + *v = root(drive, name, v)?; } } - (Param::Drives, Some(v)) => *v = drive(reads, name, v)?, + (Param::Drives, Some(v)) => *v = root(drive, name, v)?, (Param::AtMost(most), Some(Value::Array(list))) if list.len() > most => { return Err(invalid_params(format!("`{name}` has {} items: at most {most} at a time", list.len()))); } @@ -280,20 +282,23 @@ fn without(v: &mut Value, parent: Option<&str>, places: &[(Option<&str>, &str)]) } } +/// How a callback finds the root of the drive a plugin names ([`super::process::Plugin::drive`]). +pub(crate) type Drive<'a> = &'a dyn Fn(&str) -> Result; + /// The root of the drive the param `name` names, as the app is given it. -fn drive(reads: &dyn Reads, name: &str, v: &Value) -> Result { +fn root(drive: Drive<'_>, name: &str, v: &Value) -> Result { match v { Value::Null => Ok(Value::Null), - Value::String(spec) => Ok(Value::from(reads.drive(spec)?.to_string_lossy().into_owned())), + Value::String(spec) => Ok(Value::from(drive(spec)?.to_string_lossy().into_owned())), other => Err(invalid_params(format!("`{name}` names a drive, which {other} doesn't"))), } } /// What absolute paths here start with, longest first, each with what it's said as instead: -/// the mounted drives' roots (their names), the store's and `store`'s (``), the temporary +/// the mounted drives' roots and those `given` to the plugin's hooks (their names), the store's and `store`'s (``), the temporary /// folder's (``) and the home folder's (`~`), each as given and resolved (a drive in the /// temporary folder through either of its spellings too), with either separator. -pub(crate) fn known_roots(store: Option<&crate::Store>) -> Vec<(String, String)> { +pub(crate) fn known_roots(store: Option<&crate::Store>, given: &[(String, PathBuf)]) -> Vec<(String, String)> { let (temp, home) = (std::env::temp_dir(), directories::BaseDirs::new().map(|b| b.home_dir().to_path_buf())); // Folders spelt otherwise once resolved (macOS's `/var` is `/private/var`). let aliases: Vec<(PathBuf, PathBuf)> = std::iter::once(temp.clone()) @@ -303,6 +308,7 @@ pub(crate) fn known_roots(store: Option<&crate::Store>) -> Vec<(String, String)> .collect(); let mut roots: Vec<(PathBuf, String)> = api::known_drives(true).into_iter().map(|d| (d.root().to_path_buf(), d.name())).collect(); + roots.extend(given.iter().map(|(name, root)| (root.clone(), name.clone()))); let stores = api::registry().map(|s| s.root().to_path_buf()).into_iter().chain(store.map(|s| s.root().into())); roots.extend(stores.map(|root| (root, "".into()))); roots.push((temp, "".into())); diff --git a/crates/locallibrary/src/plugin/callbacks/tests.rs b/crates/locallibrary/src/plugin/callbacks/tests.rs index 16ad7fb81..1569d3959 100644 --- a/crates/locallibrary/src/plugin/callbacks/tests.rs +++ b/crates/locallibrary/src/plugin/callbacks/tests.rs @@ -39,7 +39,7 @@ impl Reads for Drives { } fn check(m: &Manifest, method: &str, params: Value) -> Result { - super::check(m, &Drives, method, params) + super::check(m, &|spec| Drives.drive(spec), method, params) } /// One read for each capability: refused without it, let through with it. @@ -185,7 +185,7 @@ fn errors_reach_plugins_without_paths_or_actions() { /// The roots here are said otherwise: the temporary folder and the home folder at least. #[test] fn the_roots_here_are_known() { - let roots = known_roots(None); + let roots = known_roots(None, &[]); let temp = std::env::temp_dir().to_string_lossy().trim_end_matches(['/', '\\']).to_string(); assert!(roots.contains(&(temp, "".into())), "{roots:?}"); let home = directories::BaseDirs::new().unwrap().home_dir().to_string_lossy().into_owned(); @@ -618,7 +618,7 @@ fn a_plugin_has_at_most_its_quota_of_calls_under_way(tc: TestCase) { let name = tc.draw(gs::sampled_from(vec!["export.pdb", "a b.txt", "x"])); let file = std::env::temp_dir().join("PIONEER").join(name); // As the plugin is told it, from wherever the temporary folder is (a drive root on Windows CI). - let said = super::scrub(&file.display().to_string(), &super::known_roots(None)); + let said = super::scrub(&file.display().to_string(), &super::known_roots(None, &[])); let ((entered_tx, entered), (release, release_rx)) = (mpsc::channel(), mpsc::channel()); let reads = Held { entered: Mutex::new(entered_tx), release: Mutex::new(release_rx), file }; let options = Options { callbacks: quota, ..Options::default() }; diff --git a/crates/locallibrary/src/plugin/checks.rs b/crates/locallibrary/src/plugin/checks.rs index cea960471..45600ce77 100644 --- a/crates/locallibrary/src/plugin/checks.rs +++ b/crates/locallibrary/src/plugin/checks.rs @@ -241,7 +241,7 @@ impl Plugins { for (slot, check, _) in self.running(When::Check, store) { let ran = within(timeout, ctx, |ctx| { let context = Context { drive: Some(device.root().display().to_string()), ..Context::default() }; - let (context, _) = slot.context(context, store)?; + let (context, _, _given) = slot.context(context, store)?; let req = RunCheck { check: check.name.clone(), context }; slot.call_with(method::RUN_CHECK, &req, store, ctx, &|_| {}, |answer, _, _| slot.found(check, answer)) }); @@ -281,7 +281,8 @@ impl Plugins { let timeout = check_timeout(store); for (slot, check, settings) in guards { let ran = within(timeout, ctx, |ctx| { - let (context, _) = slot.context(Context { drive: drive.clone(), ..Context::default() }, store)?; + let (context, _, _given) = + slot.context(Context { drive: drive.clone(), ..Context::default() }, store)?; let operation = planned.operation.to_string(); let req = PlanCheck { check: check.name.clone(), operation, edits: edits.clone(), context }; slot.call_with(method::PLAN_CHECK, &req, store, ctx, &|_| {}, |answer, _, _| slot.found(check, answer)) diff --git a/crates/locallibrary/src/plugin/commands.rs b/crates/locallibrary/src/plugin/commands.rs index 50507078c..f2782d8f2 100644 --- a/crates/locallibrary/src/plugin/commands.rs +++ b/crates/locallibrary/src/plugin/commands.rs @@ -44,7 +44,7 @@ impl Plugins { ) -> Result { let slot = self.slot(plugin)?; let declared = slot.declared("command", command, &slot.manifest.contributes.commands, |c| &c.name)?; - let (context, _) = slot.context(context, store)?; + let (context, _, _given) = slot.context(context, store)?; let params = super::fitting(declared.params.as_ref(), params, &context)?; let run = RunCommand { command: command.into(), params, context }; let shown = |partial| { diff --git a/crates/locallibrary/src/plugin/destinations/publish.rs b/crates/locallibrary/src/plugin/destinations/publish.rs index d2de25530..fc9b63a3b 100644 --- a/crates/locallibrary/src/plugin/destinations/publish.rs +++ b/crates/locallibrary/src/plugin/destinations/publish.rs @@ -127,7 +127,7 @@ impl Plugins { } let unnamed = set.unnamed(hide, &ProfilePrefs::load(store)?)?; let (details, tracklist) = published(set, &unnamed); - let (context, _) = slot.context(loclib_plugin::Context::default(), Some(store))?; + let (context, ..) = slot.context(loclib_plugin::Context::default(), Some(store))?; let mut publish = PublishSet { destination: destination.clone(), set: details, recording, tracklist, dry_run: true, context }; let Published { url, warnings, .. } = slot.call(method::SET_PUBLISH, &publish, Some(store), ctx)?; diff --git a/crates/locallibrary/src/plugin/events.rs b/crates/locallibrary/src/plugin/events.rs index dc0dded19..fb5ae77ff 100644 --- a/crates/locallibrary/src/plugin/events.rs +++ b/crates/locallibrary/src/plugin/events.rs @@ -174,7 +174,7 @@ impl Slot { /// `event/notify`, its payload saying nothing of where things are on this computer (as a /// read's answer doesn't, see [`answer_for_plugin`]). One that can't be sent is logged. fn send_event(self: &Arc, event: Event, store: Option<&Store>) { - let payload = answer_for_plugin("", event.payload, &known_roots(store)); + let payload = answer_for_plugin("", event.payload, &known_roots(store, &self.given())); let notify = EventNotify { event: event.name, drive: event.drive, payload }; if let Err(e) = self.notify("events", method::EVENT_NOTIFY, ¬ify, store) { let said = format!("the {} event wasn't sent: {e}", event.name.name()); @@ -244,13 +244,14 @@ impl Plugin { /// `action` on the drive it names as this computer knows it, by id (else by name), so a front /// end never takes it for a folder; its tracks those the drive has, each once (the others left - /// out, and logged). Why not, when the drive isn't one known here, or has none of its tracks. - /// Found as its reads find drives: in the store the plugin was started in. + /// out, and logged). Why not, when the drive isn't one known here or given to a hook under + /// way, or has none of its tracks. Found as its reads find drives: in the store the plugin was + /// started in. fn on_known_drive(&self, live: &Live, mut action: HostAction) -> std::result::Result { let (Some(drive), tracks) = action.scope_mut() else { return Ok(action) }; let store = live.store.as_ref().map(|s| s.root().to_path_buf()); crate::api::THREAD_STORE_ROOT.with(|r| *r.borrow_mut() = store); - let device = self.host.reads.drive(drive).and_then(|root| crate::Device::open(&root)); + let device = self.drive(drive).and_then(|root| crate::Device::open(&root)); let device = device.map_err(|e| format!("it suggests an action on {drive:?}, which isn't a drive known here: {e}"))?; let name = device.name(); diff --git a/crates/locallibrary/src/plugin/exports.rs b/crates/locallibrary/src/plugin/exports.rs index 43661f3cc..f0d952fb5 100644 --- a/crates/locallibrary/src/plugin/exports.rs +++ b/crates/locallibrary/src/plugin/exports.rs @@ -43,6 +43,10 @@ pub struct PluginExport { #[serde(skip)] #[schemars(skip)] slot: Weak, + /// The drive it's given again while it writes the export (see [`Slot::give`]). + #[serde(skip)] + #[schemars(skip)] + given: Option<(String, PathBuf)>, } /// A file an export writes, as the plugin says, and whether it's there already (and so @@ -87,10 +91,11 @@ impl Plugins { outside_drives(output, store)?; let source = of(declared, source, store)?; let drive = Some(source.drive.clone()).filter(|d| !d.is_empty()); - let (context, device) = + let (context, device, _given) = slot.context(Context { drive, set: source.set.clone(), ..Context::default() }, store)?; let options = super::fitting(declared.options.as_ref(), options, &context)?; let root = device.map(|d| d.root().to_path_buf()).unwrap_or_default(); + let given = context.drive.clone().map(|name| (name, root.clone())); let source = ExportSource { drive: context.drive.clone().unwrap_or_default(), root, ..source }; let mut write = WriteExport { exporter: exporter.into(), source, output: output.into(), options, dry_run: true, context }; @@ -129,6 +134,7 @@ impl Plugins { notes, write, slot: Arc::downgrade(slot), + given, }; Ok(Planned::new(format!("{plugin}.export.{exporter}"), export, ()) .summary(summary) @@ -223,6 +229,7 @@ impl Operation for PluginExport { let dir = e.output.parent().unwrap_or(&e.output); std::fs::create_dir_all(dir).at(dir)?; let slot = e.slot.upgrade().ok_or_else(|| Error::stale(format!("the {} plugin", e.plugin), "stopped"))?; + let _given = slot.give(e.given.clone()); let written: Exported = slot.call(method::WRITE_EXPORT, &e.write, store, ctx)?; // By path: a file's size is known exactly only once it's written. let said: Vec<&Path> = e.files.iter().map(|f| f.file.path.as_path()).collect(); diff --git a/crates/locallibrary/src/plugin/imports.rs b/crates/locallibrary/src/plugin/imports.rs index 949dc7348..ad99fbfeb 100644 --- a/crates/locallibrary/src/plugin/imports.rs +++ b/crates/locallibrary/src/plugin/imports.rs @@ -104,7 +104,8 @@ impl Plugins { if !std::fs::metadata(file).is_ok_and(|m| m.is_file()) { return Err(Error::invalid(format!("{} isn't a file", file.display()))); } - let (context, device) = slot.context(Context { drive: Some(drive.into()), ..Context::default() }, store)?; + let (context, device, _given) = + slot.context(Context { drive: Some(drive.into()), ..Context::default() }, store)?; let options = super::fitting(declared.options.as_ref(), options, &context)?; let device = device.expect("a drive was named"); let req = ReadImport { importer: importer.into(), file: file.into(), options, context }; diff --git a/crates/locallibrary/src/plugin/operations.rs b/crates/locallibrary/src/plugin/operations.rs index c24aab2b4..dbd57bd6b 100644 --- a/crates/locallibrary/src/plugin/operations.rs +++ b/crates/locallibrary/src/plugin/operations.rs @@ -74,7 +74,7 @@ impl Plugins { ) -> Result { let slot = self.slot(plugin)?; let declared = slot.declared("operation", operation, &slot.manifest.contributes.operations, |o| &o.name)?; - let (context, device) = slot.context(context, store)?; + let (context, device, _given) = slot.context(context, store)?; let params = super::fitting(declared.params.as_ref(), params, &context)?; let device = device.ok_or_else(|| Error::invalid(format!("{operation} edits a drive: say which")))?; let op = PlanOp { operation: operation.into(), params, context }; diff --git a/crates/locallibrary/src/plugin/panels.rs b/crates/locallibrary/src/plugin/panels.rs index 0d6af1a60..e1d606db3 100644 --- a/crates/locallibrary/src/plugin/panels.rs +++ b/crates/locallibrary/src/plugin/panels.rs @@ -37,7 +37,7 @@ impl Plugins { let slot = self.slot(plugin)?; slot.declared("panel", panel, &slot.manifest.contributes.panels, |p| &p.name)?; let context = Context { drive: Some(drive.into()), tracks: vec![track], ..Context::default() }; - let (context, _) = slot.context(context, store)?; + let (context, _, _given) = slot.context(context, store)?; let render = RenderPanel { panel: panel.into(), context }; let read = |answer: Value, scratch: &Scratch, live: &Live| { let view = slot.view(panel, answer, scratch, live)?; diff --git a/crates/locallibrary/src/plugin/process.rs b/crates/locallibrary/src/plugin/process.rs index edbea940b..3beccce7c 100644 --- a/crates/locallibrary/src/plugin/process.rs +++ b/crates/locallibrary/src/plugin/process.rs @@ -63,6 +63,26 @@ pub(crate) struct Plugin { pub(crate) host: Arc, /// Its `net/fetch` requests' client and secrets, whichever process makes them. pub(crate) net: Arc, + /// The drives given to its hooks under way (see [`Slot::context`]), each by the name its + /// context gives it and with its root, once for each hook. + given: Mutex>, +} + +/// A drive given to a hook under way: its plugin's callbacks may name it (as the context does) +/// until it's dropped, whether or not it's a drive known here (a folder opened by path, say). +pub(crate) struct Given { + plugin: Arc, + drive: Option<(String, PathBuf)>, +} + +impl Drop for Given { + fn drop(&mut self) { + let Some(drive) = self.drive.take() else { return }; + let mut given = lock(&self.plugin.given); + if let Some(i) = given.iter().position(|d| *d == drive) { + given.swap_remove(i); + } + } } /// One plugin, and its process while it runs. Only the host and the hooks under way (an event @@ -276,11 +296,24 @@ impl Plugin { pub(crate) fn misbehaved(&self, message: impl Into) -> Error { Error::External { command: self.label(), message: message.into() } } + + /// The root of the drive a callback names: one given to a hook under way, by the name its + /// context gave it, else one the host finds ([`Reads::drive`]). Never a path or a folder it + /// wasn't given. + pub(crate) fn drive(&self, spec: &str) -> Result { + let given = lock(&self.given).iter().find(|(name, _)| name == spec).map(|(_, root)| root.clone()); + given.map_or_else(|| self.host.reads.drive(spec), Ok) + } + + /// The drives given to its hooks under way, by name, with their roots. + pub(crate) fn given(&self) -> Vec<(String, PathBuf)> { + lock(&self.given).clone() + } } impl Slot { pub(crate) fn new(manifest: Manifest, launch: Launch, host: Arc) -> Slot { - let plugin = Arc::new(Plugin { manifest, host, net: Arc::default() }); + let plugin = Arc::new(Plugin { manifest, host, net: Arc::default(), given: Mutex::default() }); Slot { plugin, launch, hosting: Arc::default(), events: Arc::default(), services: Mutex::default() } } @@ -604,12 +637,26 @@ impl Slot { } /// `context` as the plugin gets it: its drive (by id, path or name) named by id, else name, - /// never a path, and the plugin's settings. With the drive. - pub(crate) fn context(&self, mut context: Context, store: Option<&Store>) -> Result<(Context, Option)> { + /// never a path, and the plugin's settings. With the drive, and that drive given to the plugin + /// until the hook is done with it: its callbacks find it by that name, a folder opened by + /// path too, which the drives known here don't have. + pub(crate) fn context( + &self, + mut context: Context, + store: Option<&Store>, + ) -> Result<(Context, Option, Given)> { let device = context.drive.as_deref().map(api::resolve_drive).transpose()?; context.drive = device.as_ref().map(|d| d.id().map_or_else(|| d.name(), |id| id.to_string())); context.settings = self.settings(store); - Ok((context, device)) + let given = self.give(context.drive.clone().zip(device.as_ref().map(|d| d.root().to_path_buf()))); + Ok((context, device, given)) + } + + /// Gives the plugin `drive` (its name, as a context names it, and its root) until what this + /// answers is dropped: see [`Given`]. + pub(crate) fn give(&self, drive: Option<(String, PathBuf)>) -> Given { + lock(&self.given).extend(drive.clone()); + Given { plugin: self.plugin.clone(), drive } } } diff --git a/crates/locallibrary/src/plugin/providers.rs b/crates/locallibrary/src/plugin/providers.rs index ff9421511..32df37836 100644 --- a/crates/locallibrary/src/plugin/providers.rs +++ b/crates/locallibrary/src/plugin/providers.rs @@ -239,7 +239,7 @@ impl Plugins { tracks: tags.iter().map(|t| t.id).collect(), ..Context::default() }; - let (context, _) = slot.context(context, store)?; + let (context, _, _given) = slot.context(context, store)?; let ask = ProvideCandidates { provider: provider.name.clone(), kind: req.kind, tracks: tags.clone(), context }; let read = diff --git a/crates/locallibrary/src/plugin/services.rs b/crates/locallibrary/src/plugin/services.rs index 6177e7ecd..429ed1ceb 100644 --- a/crates/locallibrary/src/plugin/services.rs +++ b/crates/locallibrary/src/plugin/services.rs @@ -98,7 +98,7 @@ fn supervise(slot: &Weak, service: &str, store: Option<&Store>, cancel: &C let Some(s) = slot.upgrade() else { return }; let started = Instant::now(); let ctx = Ctx::new(&NoProgress, cancel.clone()); - let ran = s.context(Context::default(), store).and_then(|(context, _)| { + let ran = s.context(Context::default(), store).and_then(|(context, ..)| { let start = StartService { service: service.into(), context }; s.call::(method::SERVICE_START, &start, store, &ctx) }); diff --git a/crates/locallibrary/src/plugin/sources.rs b/crates/locallibrary/src/plugin/sources.rs index 04dccea8d..bc3794f31 100644 --- a/crates/locallibrary/src/plugin/sources.rs +++ b/crates/locallibrary/src/plugin/sources.rs @@ -117,7 +117,7 @@ impl Plugins { let slot = self.slot(&req.plugin)?; let declared = slot.declared("library source", &req.source, &slot.manifest.contributes.sources, |s| &s.name)?; let roots = granted(&req.grants)?; - let (context, _) = slot.context(Context::default(), store)?; + let (context, ..) = slot.context(Context::default(), store)?; let options = super::fitting(declared.options.as_ref(), req.options.clone(), &context)?; let source = format!("{}:{}", req.plugin, req.source); let mut read = ReadLibrary { source: req.source.clone(), options, roots: roots.clone(), cursor: None, context }; diff --git a/crates/locallibrary/src/query.rs b/crates/locallibrary/src/query.rs index 1095c5cb5..2dcd18f84 100644 --- a/crates/locallibrary/src/query.rs +++ b/crates/locallibrary/src/query.rs @@ -144,6 +144,14 @@ fn unknown_field<'a>(tok: &str, word: &str, names: impl IntoIterator Error { + let near = nearest(name, declared.keys().map(String::as_str)) + .map_or_else(|| " (no plugin installed has it)".to_string(), |near| format!(" (did you mean {near}?)")); + Error::invalid(format!("can't read {tok:?}: there's no field {name:?}{near}")) +} + /// A numeric condition: `128`, `120-128`, `>=124`, `<5`. #[derive(Debug, Clone, Copy, PartialEq)] enum Num { @@ -495,24 +503,7 @@ impl Filter { "a {name}: term needs what analyzers worked out, and none was read" ))); } - Cond::Plugin { name, num, text } => { - let declared = &ctx.fields.expect("given").declared; - let Some(&kind) = declared.get(name) else { - return Err(unknown_field(name, name, declared.keys().map(String::as_str))); - }; - let bad = |what: &str| Err(Error::invalid(format!("can't read the {name} term: {what}"))); - let switch = |t: &String| ["true", "false", "yes", "no"].contains(&t.as_str()); - match kind { - FieldType::Number | FieldType::Integer if num.is_none() => { - return bad("it's a number: give one, a range or a comparison (>120, 120-128)"); - } - FieldType::Boolean if text.is_empty() || !text.iter().all(switch) => { - return bad("it's true or false"); - } - _ if text.is_empty() && num.is_none() => return bad("nothing to look for"), - _ => {} - } - } + Cond::Plugin { .. } => term.check_field(&ctx.fields.expect("given").declared)?, Cond::Playlist(specs) => { if let Some(s) = specs.iter().find(|s| ctx.library.find_playlist(s).is_none()) { return Err(Error::NotFound(format!("playlist {s:?}"))); @@ -527,6 +518,26 @@ impl Filter { Ok(()) } + /// Refuse a plugin's field none of the analyzers installed declares (`declared`), or a value + /// its field can't hold, as [`Filter::check`] does: what a rule is checked against before + /// any track is read. + pub fn check_fields(&self, declared: &Declared) -> Result<()> { + self.terms.iter().try_for_each(|t| t.check_field(declared)) + } + + /// The plugins' fields its terms name (`sound.bpm`), each once. + pub fn fields(&self) -> Vec<&str> { + let mut names: Vec<&str> = Vec::new(); + for t in &self.terms { + if let Cond::Plugin { name, .. } = &t.cond + && !names.contains(&name.as_str()) + { + names.push(name); + } + } + names + } + pub fn matches(&self, t: &Track, ctx: &QueryContext<'_>) -> bool { self.terms.iter().all(|term| term.matches(t, ctx) != term.negate) } @@ -538,6 +549,23 @@ impl Filter { } impl Term { + /// Refuse it when it names a plugin's field none of `declared` is, or a value its field can't + /// hold. + fn check_field(&self, declared: &Declared) -> Result<()> { + let Cond::Plugin { name, num, text } = &self.cond else { return Ok(()) }; + let Some(&kind) = declared.get(name) else { return Err(undeclared(name, name, declared)) }; + let bad = |what: &str| Err(Error::invalid(format!("can't read the {name} term: {what}"))); + let switch = |t: &String| ["true", "false", "yes", "no"].contains(&t.as_str()); + match kind { + FieldType::Number | FieldType::Integer if num.is_none() => { + bad("it's a number: give one, a range or a comparison (>120, 120-128)") + } + FieldType::Boolean if text.is_empty() || !text.iter().all(switch) => bad("it's true or false"), + _ if text.is_empty() && num.is_none() => bad("nothing to look for"), + _ => Ok(()), + } + } + fn matches(&self, t: &Track, ctx: &QueryContext<'_>) -> bool { let has = |hay: &str, needles: &[String]| { let hay = normalize(hay); @@ -844,7 +872,7 @@ pub fn parse_sort_with(s: &str, declared: Option<&Declared>) -> Result SortField::Path, other if plugin_field(other).is_some() => match declared { Some(d) if d.contains_key(other) => SortField::Field(FieldName::new(other)), - Some(d) => return Err(unknown_field(other, other, d.keys().map(String::as_str))), + Some(d) => return Err(undeclared(other, other, d)), None => { return Err(Error::invalid(format!( "can't sort by {other:?} here: a plugin's field sorts track listings only" @@ -1001,7 +1029,12 @@ impl TrackSelector { /// The selected track ids on `device`, whose library is `lib`, sorted, without repeats, with /// what the selector needs from the drive besides its library (see [`DriveContext`]). pub fn select_on(&self, device: &Device, lib: &Library) -> Result> { - self.select_with(&DriveContext::read(device, [self])?.query(lib)) + self.select_with(&DriveContext::read(device, [self], [])?.query(lib)) + } + + /// Whether the filter names a plugin's field (`sound.bpm:>120`). + pub fn uses_fields(&self) -> bool { + self.filter.as_deref().and_then(|f| Filter::parse(f).ok()).is_some_and(|f| f.uses_fields()) } pub fn select_with(&self, ctx: &QueryContext<'_>) -> Result> { @@ -1115,35 +1148,64 @@ impl TrackSelector { } } -/// What selecting tracks on a drive reads besides its library, each only when a selector needs -/// it: the drive's My Tags (for `tag:`), and the history lists of both its databases (players that -/// read only OneLibrary record theirs there, not in `export.pdb`). +/// What selecting tracks on a drive reads besides its library, each only when a selector (or a +/// sort) needs it: the drive's My Tags (for `tag:`), the history lists of both its databases +/// (players that read only OneLibrary record theirs there, not in `export.pdb`), and what the +/// analyzers installed worked out (for a plugin's field, `sound.bpm`; see +/// [`crate::fields::Installed`]). #[derive(Debug, Default)] pub struct DriveContext { tags: Option>>, history: Option>, + fields: Option, } impl DriveContext { - /// Read what `selectors` need from `device`. - pub fn read<'s>(device: &Device, selectors: impl IntoIterator) -> Result { - let (mut tags, mut history) = (false, false); + /// Read what `selectors` and `sorts` (see [`parse_sort`]) need from `device`. + pub fn read<'s>( + device: &Device, + selectors: impl IntoIterator, + sorts: impl IntoIterator, + ) -> Result { + let (mut tags, mut history, mut fields) = (false, false, false); for s in selectors { tags |= s.uses_tags(); history |= s.history.is_some(); + fields |= s.uses_fields(); } + fields |= sorts.into_iter().any(sorts_by_field); + let values = || { + let installed = crate::fields::Installed::hosted(); + Ok::<_, Error>(TrackValues::read(installed.source(), device, &device.library()?)) + }; Ok(DriveContext { tags: tags.then(|| crate::tags::by_track(device)).transpose()?, history: history.then(|| crate::plays::drive_history(device).map(|h| h.lists)).transpose()?, + fields: fields.then(values).transpose()?, }) } /// A context for selecting on `library` (the drive's) with what was read. pub fn query<'a>(&'a self, library: &'a Library) -> QueryContext<'a> { - QueryContext { tags: self.tags.as_ref(), history: self.history.as_deref(), ..QueryContext::new(library) } + QueryContext { + tags: self.tags.as_ref(), + history: self.history.as_deref(), + fields: self.fields.as_ref(), + ..QueryContext::new(library) + } + } + + /// What the analyzers installed worked out, when a selector or a sort needed it. + pub fn fields(&self) -> Option<&TrackValues> { + self.fields.as_ref() } } +/// Whether the sort `s` names a plugin's field (`-sound.bpm`), whether or not one declares it. +fn sorts_by_field(s: &str) -> bool { + s.split(',').map(|p| p.trim().trim_start_matches('-')).any(|p| plugin_field(p).is_some()) +} + /// Refuse a blank text field of a selector: an empty search or album name would take every track. fn given(what: &str, value: &str) -> Result<()> { if value.trim().is_empty() { diff --git a/crates/locallibrary/src/smart.rs b/crates/locallibrary/src/smart.rs index d57b52328..625bd6858 100644 --- a/crates/locallibrary/src/smart.rs +++ b/crates/locallibrary/src/smart.rs @@ -7,6 +7,7 @@ use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::edit::Edit; +use crate::fields::Declared; use crate::query::{self, Filter, TrackSelector}; use crate::{Error, Result}; @@ -27,7 +28,10 @@ pub struct SmartPlaylist { } impl SmartPlaylist { - pub fn validate(&self) -> Result<()> { + /// Refuse a rule that can't be applied: one without a path or a filter, or whose filter or + /// sort can't be read, or names a plugin's field none of the analyzers installed declares + /// (`declared` are those they do) or a value its field can't hold. + pub fn validate(&self, declared: &Declared) -> Result<()> { if self.path.split('/').all(|p| p.trim().is_empty()) { return Err(Error::invalid("give the playlist a path, e.g. Smart/Peak time")); } @@ -35,9 +39,9 @@ impl SmartPlaylist { if self.filter.trim().is_empty() { return Err(Error::invalid("give the rule a filter, e.g. \"bpm:126-132 rating:>=4\"")); } - Filter::parse(&self.filter)?; + Filter::parse(&self.filter)?.check_fields(declared)?; if let Some(s) = &self.sort { - query::parse_sort(s)?; + query::parse_sort_with(s, Some(declared))?; } Ok(()) } @@ -83,9 +87,10 @@ impl SmartPlaylists { self.playlists.iter().find(|p| p.key() == key(path)) } - /// Add a rule, or replace the one for exactly its path. - pub fn set(&mut self, rule: SmartPlaylist) -> Result<()> { - rule.validate()?; + /// Add a rule, or replace the one for exactly its path, once it's valid (see + /// [`SmartPlaylist::validate`]). + pub fn set(&mut self, rule: SmartPlaylist, declared: &Declared) -> Result<()> { + rule.validate(declared)?; match self.playlists.iter_mut().find(|p| p.key() == rule.key()) { Some(p) => *p = rule, None => self.playlists.push(rule), diff --git a/crates/locallibrary/src/smart/tests.rs b/crates/locallibrary/src/smart/tests.rs index 336c4b5d3..e4bd4c88f 100644 --- a/crates/locallibrary/src/smart/tests.rs +++ b/crates/locallibrary/src/smart/tests.rs @@ -10,15 +10,18 @@ fn rules_are_validated_and_replaced_by_path() { limit: None, drives: vec![], }; - s.set(rule("Smart/Peak", "bpm:>=128")).unwrap(); - s.set(rule("Smart / Peak ", "bpm:>=130")).unwrap(); + s.set(rule("Smart/Peak", "bpm:>=128"), &Default::default()).unwrap(); + s.set(rule("Smart / Peak ", "bpm:>=130"), &Default::default()).unwrap(); assert_eq!(s.playlists.len(), 1); assert_eq!(s.playlists[0].filter, "bpm:>=130"); - assert!(s.set(rule("Smart/Bad", "key:H")).is_err()); - assert!(s.set(rule("", "bpm:1")).is_err()); - assert!(s.set(rule("Smart/All", "")).is_err() && s.set(rule("Smart/All", " ")).is_err()); + assert!(s.set(rule("Smart/Bad", "key:H"), &Default::default()).is_err()); + assert!(s.set(rule("", "bpm:1"), &Default::default()).is_err()); + assert!( + s.set(rule("Smart/All", ""), &Default::default()).is_err() + && s.set(rule("Smart/All", " "), &Default::default()).is_err() + ); let only = Uuid::new_v4(); - s.set(SmartPlaylist { drives: vec![only], ..rule("Tokyo", "genre:techno") }).unwrap(); + s.set(SmartPlaylist { drives: vec![only], ..rule("Tokyo", "genre:techno") }, &Default::default()).unwrap(); assert_eq!(s.edits_for(&[]).len(), 1); assert_eq!(s.edits_for(&[only]).len(), 2); s.remove("SMART/PEAK").unwrap(); @@ -37,8 +40,8 @@ fn rules_whose_paths_differ_only_in_case_are_two_rules() { limit: None, drives: vec![], }; - s.set(rule("Smart/Foo")).unwrap(); - s.set(rule("Smart/FoO")).unwrap(); + s.set(rule("Smart/Foo"), &Default::default()).unwrap(); + s.set(rule("Smart/FoO"), &Default::default()).unwrap(); assert_eq!(s.playlists.len(), 2); assert!(s.remove("smart/foo").is_err(), "ambiguous"); assert_eq!(s.remove("Smart/FoO").unwrap().path, "Smart/FoO"); diff --git a/crates/locallibrary/tests/it/plugins.rs b/crates/locallibrary/tests/it/plugins.rs index 89098b5e2..ca3cc8c0e 100644 --- a/crates/locallibrary/tests/it/plugins.rs +++ b/crates/locallibrary/tests/it/plugins.rs @@ -10,6 +10,7 @@ mod choose; mod destinations; mod events; mod exports; +mod fields; mod hooks; mod imports; mod jobs; @@ -21,6 +22,7 @@ mod roles; mod services; mod sources; +use std::cell::Cell; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::mpsc::{self, Receiver, Sender}; @@ -156,6 +158,38 @@ impl Reads for TestReads { } } +thread_local! { + /// The plugins the library takes for the app's on this thread (see `locallibrary::plugin::set_hosted`). + static HOSTED: Cell> = const { Cell::new(None) }; +} + +/// A fixture whose plugins are the app's host on this thread, as a refresh and an edit plan find them. +struct Hosted { + _dir: tempfile::TempDir, + store: Store, + drive: PathBuf, + plugins: &'static Plugins, +} + +impl Hosted { + /// `f`'s plugins, hosted. The host lives as long as the test process does (a plan holds it), + /// its plugins stopped when the test ends. + fn new(f: Fixture) -> Hosted { + let Fixture { _dir, store, drive, plugins, .. } = f; + let plugins: &'static Plugins = Box::leak(Box::new(plugins)); + locallibrary::plugin::set_hosted(|| HOSTED.with(Cell::get)); + HOSTED.with(|h| h.set(Some(plugins))); + Hosted { _dir, store, drive, plugins } + } +} + +impl Drop for Hosted { + fn drop(&mut self) { + HOSTED.with(|h| h.set(None)); + self.plugins.shutdown(); + } +} + /// A store, a fixture drive (`USB`, adopted) and the plugins `specs` makes. struct Fixture { _dir: tempfile::TempDir, diff --git a/crates/locallibrary/tests/it/plugins/callbacks.rs b/crates/locallibrary/tests/it/plugins/callbacks.rs index 11e4b9df7..7a93d9b38 100644 --- a/crates/locallibrary/tests/it/plugins/callbacks.rs +++ b/crates/locallibrary/tests/it/plugins/callbacks.rs @@ -82,6 +82,35 @@ fn a_plugin_names_no_folder_as_a_drive() { assert_eq!(folder["error"]["data"]["kind"], "not_found", "{folder}"); } +/// A hook run on a folder opened by path, which isn't a drive known here, may name it as its +/// context does while it runs, and is told its name rather than where it is. Nothing else +/// opens up: not the folder by path, not another folder, not the folder once the hook is done. +#[test] +fn a_hook_on_a_folder_reads_only_that_folder() { + let f = Fixture::echo(); + let elsewhere = f._dir.path().join("elsewhere"); + let folder = common::demo_drive(&elsewhere, "Export"); + common::demo_drive(&elsewhere, "Other"); + let on_folder = Context { drive: Some(folder.display().to_string()), ..Context::default() }; + let run = |context: &Context, params: Value| { + let ran = f.plugins.run_command("echo", "echo", params, context.clone(), None, &Ctx::silent(), &|_| {}); + match ran.unwrap() { + CommandOutput::Headless(v) => v, + view => panic!("{view:?}"), + } + }; + let call = + |context: &Context, drive: &str| run(context, json!({ "call": "tags.list", "params": { "drive": drive } })); + assert_eq!(run(&on_folder, json!({ "context": true }))["drive"], "Export"); + // The app is given the folder's root, which the plugin is told as the folder's name. + assert_eq!(call(&on_folder, "Export")["result"]["params"]["drive"], "Export"); + assert_eq!(call(&on_folder, "USB")["result"]["params"]["drive"], "USB"); + let path = call(&on_folder, &folder.display().to_string())["error"].clone(); + assert_eq!(path["data"]["kind"], "invalid", "{path}"); + assert_eq!(call(&on_folder, "Other")["error"]["data"]["kind"], "not_found"); + assert_eq!(call(&Context::default(), "Export")["error"]["data"]["kind"], "not_found"); +} + /// Params a read takes that a plugin may not give are refused, as every one not listed is. #[test] fn a_plugin_gives_only_the_params_listed() { diff --git a/crates/locallibrary/tests/it/plugins/fields.rs b/crates/locallibrary/tests/it/plugins/fields.rs new file mode 100644 index 000000000..b2e1fa8b2 --- /dev/null +++ b/crates/locallibrary/tests/it/plugins/fields.rs @@ -0,0 +1,100 @@ +//! Analyzers' fields (`echo.bytes`) beyond track listings: smart playlist rules, edit selectors +//! and sorting a playlist on the drive take those the plugins installed declare, as they're +//! declared, and read what the analyzers worked out. + +use locallibrary::api::{DriveRef, EditRequest, SetSmartPlaylist}; +use locallibrary::edit::{Edit, EditOptions}; +use locallibrary::query::TrackSelector; + +use super::*; + +/// The fixture's two tracks with audio files of 2000 and 500 bytes, analyzed for their size when +/// `analyzed`. +fn sized(f: &Hosted, analyzed: bool) -> Vec { + let tracks = common::library(&f.drive).tracks; + assert_eq!(tracks.len(), 2); + for (t, size) in tracks.iter().zip([2000, 500]) { + std::fs::write(locallibrary::paths::to_fs(&f.drive, &locallibrary::paths::key(&t.file_path)), "x".repeat(size)) + .unwrap(); + } + if analyzed { + f.plugins.analyze("echo", "size", "USB", None, &f.store, &Ctx::silent()).unwrap(); + } + tracks +} + +fn rule(filter: &str, sort: Option<&str>) -> SetSmartPlaylist { + let sort = sort.map(String::from); + SetSmartPlaylist { path: "Smart/Big".into(), filter: filter.into(), sort, limit: None, drives: Vec::new() } +} + +/// The playlist's tracks on the drive, by id. +fn entries(f: &Hosted, path: &str) -> Vec { + common::library(&f.drive).find_playlist(path).unwrap().track_ids.clone() +} + +/// A rule naming a plugin's field is kept when an analyzer installed declares it, and the value +/// it's given fits; applied, it reads what the analyzer worked out, sorted by it too. +#[test] +fn a_smart_rule_takes_a_plugins_field() { + let f = Hosted::new(Fixture::echo()); + let tracks = sized(&f, true); + api::smart_set(&f.store, &rule("echo.bytes:>100", Some("echo.bytes"))).unwrap(); + let e = api::smart_set(&f.store, &rule("echo.byte:>1000", None)).unwrap_err().to_string(); + assert!(e.contains("there's no field \"echo.byte\" (did you mean echo.bytes?)"), "{e}"); + let e = api::smart_set(&f.store, &rule("echo.bytes:big", None)).unwrap_err().to_string(); + assert!(e.contains("it's a number"), "{e}"); + let e = api::smart_set(&f.store, &rule("bpm:>1", Some("-echo.byte"))).unwrap_err().to_string(); + assert!(e.contains("did you mean echo.bytes?"), "{e}"); + + let plan = api::smart_apply_plan(&f.store, &DriveRef { drive: "USB".into() }, &Ctx::silent()).unwrap(); + assert!(plan.warnings.is_empty(), "{:?}", plan.warnings); + plan.apply(Some(&f.store), &Ctx::silent()).unwrap(); + assert_eq!(entries(&f, "Smart/Big"), [tracks[1].id, tracks[0].id]); +} + +/// Without the analyzer's values, a rule naming its field says so: when no plugin installed +/// declares it, applying it is refused; when nothing was analyzed yet, the plan warns. +#[test] +fn a_smart_rule_on_a_field_says_when_it_cant_be_read() { + let f = Hosted::new(Fixture::echo()); + sized(&f, false); + api::smart_set(&f.store, &rule("echo.bytes:>1000", None)).unwrap(); + let plan = api::smart_apply_plan(&f.store, &DriveRef { drive: "USB".into() }, &Ctx::silent()).unwrap(); + assert_eq!(plan.warnings, ["no track here has a value for echo.bytes: analyze the tracks first"]); + + // A rule kept while a plugin was installed that isn't any more. + let mut rules = f.store.smart_playlists().unwrap(); + rules.playlists[0].filter = "other.bytes:>1000".into(); + f.store.save_smart_playlists(&rules).unwrap(); + let e = api::smart_apply_plan(&f.store, &DriveRef { drive: "USB".into() }, &Ctx::silent()).unwrap_err(); + assert!(e.to_string().contains("there's no field \"other.bytes\" (no plugin installed has it)"), "{e}"); + let e = api::smart_set(&f.store, &rule("other.bytes:>1000", None)).unwrap_err(); + assert!(e.to_string().contains("(no plugin installed has it)"), "{e}"); +} + +/// An edit's selector and a playlist sorted on the drive take a plugin's field. +#[test] +fn edits_select_and_sort_by_a_plugins_field() { + let f = Hosted::new(Fixture::echo()); + let tracks = sized(&f, true); + let edit = |edits: Vec| { + let req = EditRequest { drive: "USB".into(), edits, options: EditOptions::default() }; + api::edit_plan(&req, &Ctx::silent()).and_then(|p| p.apply(Some(&f.store), &Ctx::silent())) + }; + let add = |filter: &str| Edit::AddTracks { + playlist: "Sized".into(), + tracks: TrackSelector { filter: Some(filter.into()), ..TrackSelector::default() }, + position: None, + sort: None, + allow_repeats: false, + }; + edit(vec![Edit::CreatePlaylist { path: "Sized".into(), folder: false }, add("echo.bytes:<1000")]).unwrap(); + assert_eq!(entries(&f, "Sized"), [tracks[1].id]); + edit(vec![add("echo.bytes:>1000")]).unwrap(); + assert_eq!(entries(&f, "Sized"), [tracks[1].id, tracks[0].id]); + edit(vec![Edit::SortPlaylist { playlist: "Sized".into(), by: "-echo.bytes".into() }]).unwrap(); + assert_eq!(entries(&f, "Sized"), [tracks[0].id, tracks[1].id]); + let e = edit(vec![Edit::SortPlaylist { playlist: "Sized".into(), by: "other.bytes".into() }]).unwrap_err(); + assert!(e.to_string().contains("(no plugin installed has it)"), "{e}"); +} diff --git a/crates/locallibrary/tests/it/plugins/jobs.rs b/crates/locallibrary/tests/it/plugins/jobs.rs index eddf4bcd9..fdb800dc9 100644 --- a/crates/locallibrary/tests/it/plugins/jobs.rs +++ b/crates/locallibrary/tests/it/plugins/jobs.rs @@ -1,8 +1,6 @@ //! A refresh running plugins' operations, a step of its own: planned with the job, applied on the //! drive as the steps before left it, and refused up front when one can't be planned. -use std::cell::Cell; - use hegel::TestCase; use hegel::generators as gs; use locallibrary::api::{DriveRef, JobPlan, OperationStep, RefreshRequest, StepKind}; @@ -11,30 +9,7 @@ use loclib_plugin::{Choice, ChoiceItem}; use super::*; -thread_local! { - /// The plugins a refresh on this thread finds (see `locallibrary::plugin::set_hosted`). - static HOSTED: Cell> = const { Cell::new(None) }; -} - -/// A fixture whose plugins are the app's host on this thread, as `refresh` finds them. -struct Hosted { - _dir: tempfile::TempDir, - store: Store, - drive: PathBuf, - plugins: &'static Plugins, -} - impl Hosted { - /// `f`'s plugins, hosted. The host lives as long as the test process does (a plan holds it), - /// its plugins stopped when the test ends. - fn new(f: Fixture) -> Hosted { - let Fixture { _dir, store, drive, plugins, .. } = f; - let plugins: &'static Plugins = Box::leak(Box::new(plugins)); - locallibrary::plugin::set_hosted(|| HOSTED.with(Cell::get)); - HOSTED.with(|h| h.set(Some(plugins))); - Hosted { _dir, store, drive, plugins } - } - fn refresh(&self, operations: Vec, skip: Vec) -> Result> { let req = RefreshRequest { drive: Some(common::path(&self.drive)), skip, sync: false, operations }; locallibrary::api::refresh_plan(&self.store, &req, &Ctx::silent()) @@ -45,13 +20,6 @@ impl Hosted { } } -impl Drop for Hosted { - fn drop(&mut self) { - HOSTED.with(|h| h.set(None)); - self.plugins.shutdown(); - } -} - fn op(operation: &str, params: Value) -> OperationStep { OperationStep { operation: operation.into(), params } } diff --git a/crates/locallibrary/tests/it/props_core2_smart.rs b/crates/locallibrary/tests/it/props_core2_smart.rs index c9936758d..e32f6af7d 100644 --- a/crates/locallibrary/tests/it/props_core2_smart.rs +++ b/crates/locallibrary/tests/it/props_core2_smart.rs @@ -22,7 +22,7 @@ fn a_path_matching_several_rules_only_ignoring_case_removes_none(tc: TestCase) { limit: None, drives: vec![], }; - rules.set(rule).unwrap(); + rules.set(rule, &Default::default()).unwrap(); } let asked = tc.draw(gs::sampled_from(CASES)); let before: BTreeSet = rules.playlists.iter().map(|p| p.path.clone()).collect(); diff --git a/crates/locallibrary/tests/it/props_smart.rs b/crates/locallibrary/tests/it/props_smart.rs index 4365bb3c8..d6bb46791 100644 --- a/crates/locallibrary/tests/it/props_smart.rs +++ b/crates/locallibrary/tests/it/props_smart.rs @@ -47,8 +47,8 @@ fn rules_match_the_model(tc: TestCase) { drives: if tc.draw(gs::booleans()) { vec![drive] } else { vec![] }, }; let valid = !key(path).is_empty() && filter_ok && sort != Some("nonsense:"); - assert_eq!(rule.validate().is_ok(), valid, "{rule:?}"); - assert_eq!(rules.set(rule).is_ok(), valid); + assert_eq!(rule.validate(&Default::default()).is_ok(), valid, "{rule:?}"); + assert_eq!(rules.set(rule, &Default::default()).is_ok(), valid); if valid { match model.iter_mut().find(|(k, _)| *k == key(path)) { Some(slot) => slot.1 = filter.into(), @@ -93,13 +93,16 @@ fn a_rules_playlist_holds_what_it_selects(tc: TestCase) { let sort = tc.draw(gs::sampled_from(&SORTS[..3])); let limit = tc.draw(gs::optional(gs::integers::().min_value(1).max_value(5))); rules - .set(SmartPlaylist { - path: path.into(), - filter: filter.into(), - sort: sort.map(Into::into), - limit, - drives: vec![], - }) + .set( + SmartPlaylist { + path: path.into(), + filter: filter.into(), + sort: sort.map(Into::into), + limit, + drives: vec![], + }, + &Default::default(), + ) .unwrap(); } let edits = rules.edits_for(&[]); diff --git a/crates/locallibrary/tests/it/props_store_records.rs b/crates/locallibrary/tests/it/props_store_records.rs index e6408306e..485a07277 100644 --- a/crates/locallibrary/tests/it/props_store_records.rs +++ b/crates/locallibrary/tests/it/props_store_records.rs @@ -148,13 +148,16 @@ fn the_stores_records_match_the_model(tc: TestCase) { // What's kept beside the snapshots reads back as saved. let mut smart = SmartPlaylists::default(); smart - .set(SmartPlaylist { - path: "Smart/Peak".into(), - filter: "bpm:>=128".into(), - sort: None, - limit: Some(3), - drives: vec![], - }) + .set( + SmartPlaylist { + path: "Smart/Peak".into(), + filter: "bpm:>=128".into(), + sort: None, + limit: Some(3), + drives: vec![], + }, + &Default::default(), + ) .unwrap(); store.save_smart_playlists(&smart).unwrap(); assert_eq!(store.smart_playlists().unwrap(), smart);