some long awaited fixes

Cargo.toml

@@ -53,3 +53,4 @@ tracing-tree = "^0.2.2"
 zbus = { version = "3.14.1", features = ["tokio"] }
 serde_plain = "1.0.1"
 
+xdg = "2.5.2"

common/Cargo.toml

@@ -21,4 +21,4 @@ thiserror = "1.0.37"
 zbus.workspace = true
 serde_plain.workspace = true
 figment = "0.10.15"
-xdg = "2.4.1"
+xdg.workspace=true

odilia/Cargo.toml

@@ -51,7 +51,7 @@ tracing-log.workspace = true
 tracing-subscriber.workspace = true
 tracing-tree.workspace = true
 tracing.workspace = true
-xdg = "2.4.1"
+xdg.workspace=true
 zbus.workspace = true
 odilia-notify = { version = "0.1.0", path = "../odilia-notify" }
 clap = { version = "4.5.1", features = ["derive"] }

odilia/src/events/object.rs

@@ -1,7 +1,7 @@
 use crate::state::ScreenReaderState;
 use atspi_common::events::object::ObjectEvents;
 
-#[tracing::instrument(level = "debug", skip(state), ret, err)]
+#[tracing::instrument(level = "debug", skip(state), err)]
 pub async fn dispatch(state: &ScreenReaderState, event: &ObjectEvents) -> eyre::Result<()> {
 	// Dispatch based on member
 	match event {
@@ -36,7 +36,6 @@ mod text_changed {
 	use ssip_client_async::Priority;
 	use std::collections::HashMap;
 
-	#[inline]
 	#[tracing::instrument(level = "trace")]
 	pub fn update_string_insert(
 		start_pos: usize,
@@ -74,21 +73,18 @@ mod text_changed {
 		}
 	}
 
-	#[inline]
 	pub fn append_to_object(original: &str, to_append: &str) -> String {
 		let mut new_text = original.chars().collect::<Vec<char>>();
 		new_text.extend(to_append.chars());
 		new_text.into_iter().collect()
 	}
 
-	#[inline]
 	pub fn insert_at_index(original: &str, to_splice: &str, index: usize) -> String {
 		let mut new_text = original.chars().collect::<Vec<char>>();
 		new_text.splice(index.., to_splice.chars());
 		new_text.into_iter().collect()
 	}
 
-	#[inline]
 	pub fn insert_at_range(
 		original: &str,
 		to_splice: &str,
@@ -100,18 +96,16 @@ mod text_changed {
 		new_text.into_iter().collect()
 	}
 
-	#[inline]
 	/// Get the live state of a set of attributes.
 	/// Although the function only currently tests one attribute, in the future it may be important to inspect many attributes, compare them, or do additional logic.
-	#[tracing::instrument(level = "trace", ret, err)]
+	#[tracing::instrument(level = "trace", ret)]
 	pub fn get_live_state(attributes: &HashMap<String, String>) -> OdiliaResult<AriaLive> {
 		match attributes.get("live") {
 			None => Err(OdiliaError::NoAttributeError("live".to_string())),
 			Some(live) => Ok(serde_plain::from_str(live)?),
 		}
 	}
 
-	#[inline]
 	/// if the aria-live attribute is set to "polite", then set the priority of the message to speak once all other messages are done
 	/// if the aria-live attribute is set to "assertive", then set the priority of the message to speak immediately, stop all other messages, and do not interrupt that piece of speech
 	/// otherwise, do not continue
@@ -123,8 +117,7 @@ mod text_changed {
 		}
 	}
 
-	#[inline]
-	#[tracing::instrument(level = "trace", ret, err)]
+	#[tracing::instrument(level = "trace", ret)]
 	pub fn get_atomic_state(attributes: &HashMap<String, String>) -> OdiliaResult<AriaAtomic> {
 		match attributes.get("atomic") {
 			None => Err(OdiliaError::NoAttributeError("atomic".to_string())),
@@ -162,7 +155,7 @@ mod text_changed {
 		}
 	}
 
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state), err)]
 	pub async fn dispatch(
 		state: &ScreenReaderState,
 		event: &TextChangedEvent,
@@ -180,7 +173,7 @@ mod text_changed {
 		Ok(())
 	}
 
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state))]
 	pub async fn speak_insertion(
 		state: &ScreenReaderState,
 		event: &TextChangedEvent,
@@ -203,7 +196,7 @@ mod text_changed {
 	/// The `insert` boolean, if set to true, will update the text in the cache.
 	/// If it is set to false, the selection will be removed.
 	/// The [`TextChangedEvent::operation`] value will *NOT* be checked by this function.
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state), err)]
 	pub async fn insert_or_delete(
 		state: &ScreenReaderState,
 		event: &TextChangedEvent,
@@ -258,7 +251,7 @@ mod children_changed {
 	use odilia_common::result::OdiliaResult;
 	use std::sync::Arc;
 
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state), err)]
 	pub async fn dispatch(
 		state: &ScreenReaderState,
 		event: &ChildrenChangedEvent,
@@ -271,7 +264,7 @@ mod children_changed {
 		}
 		Ok(())
 	}
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state), err)]
 	pub async fn add(
 		state: &ScreenReaderState,
 		event: &ChildrenChangedEvent,
@@ -286,7 +279,6 @@ mod children_changed {
 		tracing::debug!("Add a single item to cache.");
 		Ok(())
 	}
-	#[inline]
 	fn get_child_primitive(event: &ChildrenChangedEvent) -> AccessiblePrimitive {
 		event.child.clone().into()
 	}
@@ -395,7 +387,7 @@ mod text_caret_moved {
 	}
 
 	// TODO: left/right vs. up/down, and use generated speech
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state), err)]
 	pub async fn text_cursor_moved(
 		state: &ScreenReaderState,
 		event: &TextCaretMovedEvent,
@@ -431,7 +423,7 @@ mod text_caret_moved {
 		Ok(())
 	}
 
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state), err)]
 	pub async fn dispatch(
 		state: &ScreenReaderState,
 		event: &TextCaretMovedEvent,
@@ -472,7 +464,7 @@ mod state_changed {
 		}
 	}
 
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state), err)]
 	pub async fn dispatch(
 		state: &ScreenReaderState,
 		event: &StateChangedEvent,
@@ -481,7 +473,7 @@ mod state_changed {
 		// update cache with state of item
 		let a11y_prim = AccessiblePrimitive::from_event(event)?;
 		if update_state(state, &a11y_prim, event.state, state_value)? {
-			tracing::debug!("Updating of the state was not succesful! The item with id {:?} was not found in the cache.", a11y_prim.id);
+			tracing::trace!("Updating of the state was not successful! The item with id {:?} was not found in the cache.", a11y_prim.id);
 		} else {
 			tracing::trace!("Updated the state of accessible with ID {:?}, and state {:?} to {state_value}.", a11y_prim.id, event.state);
 		}
@@ -497,7 +489,7 @@ mod state_changed {
 		Ok(())
 	}
 
-	#[tracing::instrument(level = "debug", skip(state), ret, err)]
+	#[tracing::instrument(level = "debug", skip(state), err)]
 	pub async fn focused(
 		state: &ScreenReaderState,
 		event: &StateChangedEvent,

odilia/src/logging.rs

@@ -3,59 +3,46 @@
 //! Not much here yet, but this will get more complex if we decide to add other layers for error
 //! reporting, tokio-console, etc.
 
-use std::env;
+use std::{env, io};
 
 use eyre::Context;
 use odilia_common::settings::{log::LoggingKind, ApplicationConfig};
 use tracing_error::ErrorLayer;
-use tracing_log::LogTracer;
 use tracing_subscriber::{prelude::*, EnvFilter};
 use tracing_tree::HierarchicalLayer;
 
 /// Initialise the logging stack
 /// this requires an application configuration structure, so configuration must be initialized before logging is
 pub fn init(config: &ApplicationConfig) -> eyre::Result<()> {
-	let env_filter =
-		match env::var("APP_LOG").or_else(|_| env::var("RUST_LOG")) {
-			Ok(s) => EnvFilter::from(s),
-			Err(env::VarError::NotPresent) => EnvFilter::from(&config.log.level),
-			Err(e) => {
-				eprintln!("Warning: Failed to read log filter from APP_LOG or RUST_LOG: {e}");
-				EnvFilter::from(&config.log.level)
-			}
-		};
-	//this requires boxing because the types returned by this match block would be incompatible otherwise, since we return different layers depending on what we get from the configuration. It is possible to do it otherwise, hopefully, but for now this and a forced dereference at the end would do
-	let output_layer = match &config.log.logger {
+	let env_filter = match env::var("APP_LOG").or_else(|_| env::var("RUST_LOG")) {
+		Ok(s) => EnvFilter::from(s),
+		_ => EnvFilter::from(&config.log.level),
+	};
+	let tree = HierarchicalLayer::new(4)
+		.with_bracketed_fields(true)
+		.with_targets(true)
+		.with_deferred_spans(true)
+		.with_span_retrace(true)
+		.with_indent_lines(true)
+		.with_ansi(false)
+		.with_wraparound(4);
+	//this requires boxing because the types returned by this match block would be incompatible otherwise, since we return different layers, or modifications to a layer depending on what we get from the configuration. It is possible to do it otherwise, hopefully, but for now this  would do
+	let final_layer = match &config.log.logger {
 		LoggingKind::File(path) => {
-			let file = std::fs::File::options()
-				.create_new(true)
-				.write(true)
-				.open(path)
-				.with_context(|| {
-					format!("creating log file '{}'", path.display())
-				})?;
-			let fmt =
-				tracing_subscriber::fmt::layer().with_ansi(false).with_writer(file);
-			fmt.boxed()
+			let file = std::fs::File::create(path).with_context(|| {
+				format!("creating log file '{}'", path.display())
+			})?;
+			tree.with_writer(file).boxed()
 		}
-		LoggingKind::Tty => tracing_subscriber::fmt::layer()
-			.with_ansi(true)
-			.with_target(true)
+		LoggingKind::Tty => tree.with_writer(io::stdout).with_ansi(true).boxed(),
+		LoggingKind::Syslog => tracing_journald::Layer::new()?
+			.with_syslog_identifier("odilia".to_owned())
 			.boxed(),
-		LoggingKind::Syslog => tracing_journald::layer()?.boxed(),
 	};
-	let subscriber = tracing_subscriber::Registry::default()
+	tracing_subscriber::Registry::default()
 		.with(env_filter)
-		.with(output_layer)
 		.with(ErrorLayer::default())
-		.with(HierarchicalLayer::new(4)
-			.with_bracketed_fields(true)
-			.with_targets(true)
-			.with_deferred_spans(true)
-			.with_span_retrace(true)
-			.with_indent_lines(true));
-	tracing::subscriber::set_global_default(subscriber)
-		.wrap_err("unable to init default logging layer")?;
-	LogTracer::init().wrap_err("unable to init tracing log layer")?;
+		.with(final_layer)
+		.init();
 	Ok(())
 }

odilia/src/main.rs

@@ -165,7 +165,7 @@ async fn main() -> eyre::Result<()> {
 
 fn load_configuration(cli_overide: Option<PathBuf>) -> Result<ApplicationConfig, eyre::Report> {
 	// In order, do  a configuration file specified via cli, XDG_CONFIG_HOME, the usual location for system wide configuration(/etc/odilia/config.toml)
-	// If XDG_CONFIG_HOME based configuration wasn't found, create one with default values for the user to alter, for the next run of odilia
+	// If XDG_CONFIG_HOME based configuration wasn't found, create one by combining default values with the system provided ones, if available, for the user to alter, for the next run of odilia
 	//default configuration first, because that doesn't affect the priority outlined above
 	let figment = Figment::from(Serialized::defaults(ApplicationConfig::default()));
 	//cli override, if applicable
@@ -180,15 +180,16 @@ fn load_configuration(cli_overide: Option<PathBuf>) -> Result<ApplicationConfig,
 		.place_config_file("config.toml")
 		.expect("unable to place configuration file. Maybe your system is readonly?");
 
+	let figment = figment
+		//next, the configuration system wide, in /etc/odilia/config.toml
+		.admerge(Toml::file("/etc/odilia/config.toml"))
+		//finally, the xdg configuration
+		.admerge(Toml::file(&config_path));
+	//realise the configuration and freeze it into place
+	let config: ApplicationConfig = figment.extract()?;
 	if !config_path.exists() {
-		let toml = toml::to_string(&ApplicationConfig::default())?;
+		let toml = toml::to_string(&config)?;
 		fs::write(&config_path, toml).expect("Unable to create default config file.");
 	}
-	//next, the xdg configuration
-	let figment = figment
-		.admerge(Toml::file(&config_path))
-		//last, the configuration system wide, in /etc/odilia/config.toml
-		.admerge(Toml::file("/etc/odilia/config.toml"));
-	//realise the configuration and freeze it into place
-	Ok(figment.extract()?)
+	Ok(config)
 }

odilia/src/state.rs

@@ -114,7 +114,7 @@ impl ScreenReaderState {
 			cache,
 		})
 	}
-	#[tracing::instrument(level="debug", skip(self) ret, err)]
+	#[tracing::instrument(level = "debug", skip(self), err)]
 	pub async fn get_or_create_atspi_cache_item_to_cache(
 		&self,
 		atspi_cache_item: atspi_common::CacheItem,
@@ -130,7 +130,7 @@ impl ScreenReaderState {
 		}
 		self.cache.get(&prim).ok_or(CacheError::NoItem.into())
 	}
-	#[tracing::instrument(level="debug", skip(self) ret, err)]
+	#[tracing::instrument(level = "debug", skip(self), err)]
 	pub async fn get_or_create_atspi_legacy_cache_item_to_cache(
 		&self,
 		atspi_cache_item: atspi_common::LegacyCacheItem,
@@ -146,7 +146,7 @@ impl ScreenReaderState {
 		}
 		self.cache.get(&prim).ok_or(CacheError::NoItem.into())
 	}
-	#[tracing::instrument(skip_all, level = "debug", ret, err)]
+	#[tracing::instrument(skip_all, level = "debug", err)]
 	pub async fn get_or_create_event_object_to_cache<'a, T: GenericEvent<'a>>(
 		&self,
 		event: &T,
@@ -307,7 +307,7 @@ impl ScreenReaderState {
 			.get_or_create(&accessible_proxy, Arc::downgrade(&self.cache))
 			.await
 	}
-	#[tracing::instrument(skip_all, ret, err)]
+	#[tracing::instrument(skip_all, err)]
 	pub async fn new_accessible<'a, T: GenericEvent<'a>>(
 		&self,
 		event: &T,
@@ -321,7 +321,7 @@ impl ScreenReaderState {
 			.build()
 			.await?)
 	}
-	#[tracing::instrument(skip_all, ret, err)]
+	#[tracing::instrument(skip_all, err)]
 	pub async fn add_cache_match_rule(&self) -> OdiliaResult<()> {
 		let cache_rule = MatchRule::builder()
 			.msg_type(MessageType::Signal)