From e44a8b85c4f0a86ae121d87d9d129880235045cd Mon Sep 17 00:00:00 2001 From: Tal <83217276+talwat@users.noreply.github.com> Date: Sat, 6 Dec 2025 17:28:11 +0100 Subject: [PATCH] fix: error reporting docs: fix typos --- CONTRIBUTING.md | 10 ++++++---- ENVIRONMENT_VARS.md | 2 +- MUSIC.md | 2 +- README.md | 20 +------------------ src/error.rs | 20 +++++++++---------- src/main.rs | 15 ++++++++++---- src/player.rs | 8 ++++++-- src/ui.rs | 48 ++++++++++++++++++++++++++++++++------------- src/ui/mpris.rs | 2 +- 9 files changed, 71 insertions(+), 56 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3dab0ed..4de2754 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,11 +3,13 @@ There are a few guidelines outlined here that will make it more likely for your PR to be accepted. Only ones that are less obvious are going to be listed. If you need to ask, it's probably a no. -## 1. No AI +## 1. AI -You can use AI for searching and so on, and if there's something minor and tedious that you'd like -the AI to write then that's okay, but if it is noticable that you used AI then it is way too much. -If you used AI, you aren't helping any maintainer by submitting your slop, it's just a hassle for them. +You can use AI for searching or if there's something minor and tedious (eg. tests) that you'd like +to avoid having to do manually. + +With that said, if it is noticeable that you used AI then it is way too much. +AI generated PR's do not help maintainers, it's just a hassle and frequently wastes their time. ## 2. Smaller is better diff --git a/ENVIRONMENT_VARS.md b/ENVIRONMENT_VARS.md index 53e1336..0745623 100644 --- a/ENVIRONMENT_VARS.md +++ b/ENVIRONMENT_VARS.md @@ -1,7 +1,7 @@ # Environment Variables Lowfi has some more specific options, usually as a result of minor feature requests, which are only documented here. -If you have some behaviour you'd like to change, which is quite specific, then see if one of these options suits you. +If you have some behavior you'd like to change, which is quite specific, then see if one of these options suits you. * `LOWFI_FIXED_MPRIS_NAME` - Limits the number of lowfi instances to one, but ensures the player name is always `lowfi`. * `LOWFI_DISABLE_UI` - Disables the UI. diff --git a/MUSIC.md b/MUSIC.md index 39a34f8..f5a9c3b 100644 --- a/MUSIC.md +++ b/MUSIC.md @@ -58,7 +58,7 @@ I'm not making money on any of this, and I think degrading the experience for my fellow nerds who just want to listen to some lowfi without all the crap is not worth it. At the end of the day, lowfi has a distinct UserAgent. Should chillhop ever take issue with -it's behaviour, banning it is extremely simple. I don't want that to happen, but I +it's behavior, banning it is extremely simple. I don't want that to happen, but I understand if it does. ## Well, *I* Hate the Chillhop Music diff --git a/README.md b/README.md index 05db5c9..008d03e 100644 --- a/README.md +++ b/README.md @@ -5,24 +5,6 @@ It'll do this as simply as it can: no albums, no ads, just lofi. ![example image](media/example1.png) -## The Rewrite - -This branch serves as a rewrite for lowfi. The main focus is to make the code more -maintainable. This includes such things as: - -- Replacing `Mutex` & `Arc` with channels, massively improving readability and flow. -- More clearly handling tracks in different phases of loading, instead of having -a mess of different structs. -- Making the UI code cleaner and easier to follow. -- Rethinking input & control of the player, especially with MPRIS in mind. -- Making track loading simpler and more consistent. - -This is an *internal rewrite*, and the goal is to retain every single feature. -If there is a feature present in the original version of lowfi that is not present -in the rewrite, then it is a bug and must be implemented. - -Currently, it is in an extremely early and non-functional state. - ## Disclaimer As of the 1.7.0 version of lowfi, **all** of the audio files embedded @@ -207,7 +189,7 @@ If you are dealing with the 1% using another audio format which is in > [!NOTE] > Some nice users, especially [danielwerg](https://github.com/danielwerg), -> have aleady made alternative track lists located in the [data](https://github.com/talwat/lowfi/blob/main/data/) +> have already made alternative track lists located in the [data](https://github.com/talwat/lowfi/blob/main/data/) > directory of this repo. You can use them with lowfi by using the `--track-list` flag. > > Feel free to contribute your own list with a PR. diff --git a/src/error.rs b/src/error.rs index 7f20842..5ba8a4e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -15,39 +15,39 @@ pub type Result = std::result::Result; #[derive(Debug, thiserror::Error)] pub enum Error { /// Errors while loading or saving the persistent volume settings. - #[error("unable to load/save the persistent volume: {0}")] + #[error("unable to load/save the persistent volume")] PersistentVolume(#[from] volume::Error), /// Errors while loading or saving bookmarks. - #[error("unable to load/save bookmarks: {0}")] + #[error("unable to load/save bookmarks")] Bookmarks(#[from] bookmark::Error), /// Network request failures from `reqwest`. - #[error("unable to fetch data: {0}")] + #[error("unable to fetch data")] Request(#[from] reqwest::Error), /// Failure converting to/from a C string (FFI helpers). - #[error("C string null error: {0}")] + #[error("C string null error")] FfiNull(#[from] std::ffi::NulError), /// Errors coming from the audio backend / stream handling. - #[error("audio playing error: {0}")] + #[error("audio playing error")] Rodio(#[from] rodio::StreamError), /// Failure to send an internal `Message` over the mpsc channel. - #[error("couldn't send internal message: {0}")] + #[error("couldn't send internal message")] Send(#[from] mpsc::error::SendError), /// Failure to enqueue a track into the queue channel. - #[error("couldn't add track to the queue: {0}")] + #[error("couldn't add track to the queue")] Queue(#[from] mpsc::error::SendError), /// Failure to broadcast UI updates. - #[error("couldn't update UI state: {0}")] + #[error("couldn't update UI state")] Broadcast(#[from] broadcast::error::SendError), /// Generic IO error. - #[error("io error: {0}")] + #[error("io error")] Io(#[from] std::io::Error), /// Data directory was not found or could not be determined. @@ -59,7 +59,7 @@ pub enum Error { Download, /// Integer parsing errors. - #[error("couldn't parse integer: {0}")] + #[error("couldn't parse integer")] Parse(#[from] std::num::ParseIntError), /// Track subsystem error. diff --git a/src/main.rs b/src/main.rs index c38599f..a4c2f30 100644 --- a/src/main.rs +++ b/src/main.rs @@ -96,6 +96,14 @@ pub fn data_dir() -> crate::Result { Ok(dir) } +async fn player(args: Args, environment: ui::Environment) -> crate::Result<()> { + let stream = audio::stream()?; + let mut player = Player::init(args, environment, stream.mixer()).await?; + player.run().await?; + + Ok(()) +} + /// Program entry point. /// /// Parses CLI arguments, initializes the audio stream and player, then @@ -116,10 +124,9 @@ async fn main() -> eyre::Result<()> { } } - let stream = audio::stream()?; - let mut player = Player::init(args, stream.mixer()).await?; - let result = player.run().await; + let environment = ui::Environment::ready(args.alternate)?; + let result = player(args, environment).await; + environment.cleanup(result.is_ok())?; - player.environment().cleanup(result.is_ok())?; Ok(result?) } diff --git a/src/player.rs b/src/player.rs index c1d8d1c..f9ddf0c 100644 --- a/src/player.rs +++ b/src/player.rs @@ -115,7 +115,11 @@ impl Player { /// This sets up the audio sink, UI, downloader, bookmarks and persistent /// volume state. The function returns a fully constructed `Player` ready /// to be driven via `run`. - pub async fn init(args: crate::Args, mixer: &rodio::mixer::Mixer) -> crate::Result { + pub async fn init( + args: crate::Args, + environment: ui::Environment, + mixer: &rodio::mixer::Mixer, + ) -> crate::Result { let (tx, rx) = mpsc::channel(8); if args.paused { tx.send(Message::Pause).await?; @@ -133,7 +137,7 @@ impl Player { sink.set_volume(volume.float()); Ok(Self { - ui: ui::Handle::init(tx.clone(), urx, state, &args).await?, + ui: ui::Handle::init(tx.clone(), environment, urx, state, &args).await?, downloader: Downloader::init( args.buffer_size as usize, args.timeout, diff --git a/src/ui.rs b/src/ui.rs index d0fb7da..297e307 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -1,4 +1,4 @@ -use std::sync::Arc; +use std::{env, sync::Arc}; use crate::{ player::Current, @@ -39,6 +39,9 @@ pub enum Error { #[error("sharing state between backend and frontend failed: {0}")] Send(#[from] tokio::sync::broadcast::error::SendError), + #[error("you can't disable the UI without MPRIS!")] + RejectedDisable, + #[cfg(feature = "mpris")] #[error("mpris bus error: {0}")] ZBus(#[from] mpris_server::zbus::Error), @@ -115,6 +118,27 @@ struct Tasks { input: JoinHandle>, } +impl Tasks { + pub fn spawn( + tx: Sender, + updater: broadcast::Receiver, + state: State, + params: interface::Params, + ) -> Self { + Self { + render: tokio::spawn(Handle::ui(updater, state, params)), + input: tokio::spawn(input::listen(tx)), + } + } +} + +impl Drop for Tasks { + fn drop(&mut self) { + self.input.abort(); + self.render.abort(); + } +} + /// The UI handle for controlling the state of the UI, as well as /// updating MPRIS information and other small interfacing tasks. pub struct Handle { @@ -126,14 +150,7 @@ pub struct Handle { pub mpris: mpris::Server, /// The UI's running tasks. - tasks: Tasks, -} - -impl Drop for Handle { - fn drop(&mut self) { - self.tasks.input.abort(); - self.tasks.render.abort(); - } + _tasks: Option, } impl Handle { @@ -174,19 +191,22 @@ impl Handle { #[allow(clippy::unused_async)] pub async fn init( tx: Sender, + environment: Environment, updater: broadcast::Receiver, state: State, args: &Args, ) -> Result { - let environment = Environment::ready(args.alternate)?; + let disabled = env::var("LOWFI_DISABLE_UI").is_ok_and(|x| x == "1"); + if disabled && !cfg!(feature = "mpris") { + return Err(Error::RejectedDisable); + } + Ok(Self { #[cfg(feature = "mpris")] mpris: mpris::Server::new(state.clone(), tx.clone(), updater.resubscribe()).await?, environment, - tasks: Tasks { - render: tokio::spawn(Self::ui(updater, state, interface::Params::from(args))), - input: tokio::spawn(input::listen(tx)), - }, + _tasks: (!disabled) + .then(|| Tasks::spawn(tx, updater, state, interface::Params::from(args))), }) } } diff --git a/src/ui/mpris.rs b/src/ui/mpris.rs index 761b461..d8855a0 100644 --- a/src/ui/mpris.rs +++ b/src/ui/mpris.rs @@ -127,7 +127,7 @@ impl PlayerInterface for Player { } async fn stop(&self) -> fdo::Result<()> { - self.pause().await + self.sender.send(Message::Quit).await } async fn play(&self) -> fdo::Result<()> {