phughk/ottyl
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Trim comments across Rust and webapp source to direct one-liners (ticket 030)

Browse Source 
Pass over src/*.rs, webapp/src/**, and the in-app "?" docs prose:
condensed multi-paragraph doc comments to single lines that state the
non-obvious why, deleted comments that only restated adjacent code, and
tightened the Docs.tsx help-panel wording. No logic, structure, or code
formatting changed — verified via cargo test (143 passed) and
tsc -b && vite build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XFUNZwCvz3kcod3352ZR2o
This commit is contained in:
Hugh Kaznowski
2026-09-05 17:02:33 +01:00
co-authored by Claude Sonnet 5
parent 3cef6dc8ae
commit 00c21c8032
32 changed files with 347 additions and 1550 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/settings.rs
+23 -102
View File
@@ -1,9 +1,4 @@
//! Process-wide settings model: colour scheme, CSS entries, and profile
//! pointers. There is exactly one `Settings` instance at runtime, loaded once
//! on startup and persisted to disk whenever it changes (see `main.rs`).
//!
//! Persistence format is JSON (via `serde_json`, already a dependency) rather
//! than TOML, to keep the dependency list minimal.
//! Process-wide settings model: colour scheme, CSS entries, and profile pointers, persisted as JSON.
use std::collections::hash_map::DefaultHasher;
use std::fmt;
@@ -19,8 +14,7 @@ use serde::{Deserialize, Serialize};
use crate::alerts::AlertRule;
/// Shared, process-wide settings handle. Cheap to clone; lock is only held
/// briefly since settings changes are rare (unlike the engine's hot path).
/// Shared, process-wide settings handle. Cheap to clone.
pub type SettingsHandle = Arc<Mutex<Settings>>;
/// User's preferred colour scheme for the webapp.
@@ -40,17 +34,7 @@ pub enum ColorScheme {
System,
}
/// A single CSS entry configured for the webapp: either content that ottyl
/// stores itself ("imported") or a reference to an external path/URL that
/// the webapp links to directly ("linked", and therefore not deletable via
/// the settings API since ottyl doesn't own it).
///
/// Ticket 010 builds the CSS import/link HTTP API around this shape.
///
/// `is_theme` (ticket 033) marks an entry as a full colour-scheme theme (a
/// `:root { --c-*: ...; }` variable set, selectable from the theme picker)
/// rather than arbitrary CSS that's always injected. `#[serde(default)]` so
/// existing settings.json files without the field still load as `false`.
/// A single CSS entry: content ottyl stores itself ("imported") or a reference to an external path/URL ("linked", not deletable via the API). `is_theme` marks it as a selectable colour-scheme theme rather than always-injected CSS.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(tag = "kind", rename_all = "lowercase")]
pub enum CssEntry {
@@ -67,14 +51,10 @@ pub enum CssEntry {
},
}
/// Placeholder shape for profile management (ticket 009 builds CRUD on top
/// of this). Profiles are expected to live as individual files under a
/// dedicated directory; this struct only tracks known profile names and
/// which one is currently active.
/// Tracks known profile names and which one is active; the profile files themselves live elsewhere.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
pub struct Profiles {
/// Names of known profiles. Ticket 009 owns creating/loading the actual
/// profile files.
/// Names of known profiles.
pub names: Vec<String>,
/// Name of the currently active profile, if any.
pub active_profile: Option<String>,
@@ -83,27 +63,19 @@ pub struct Profiles {
pub profiles_dir: String,
}
/// The single, process-wide settings instance.
///
/// Only `PartialEq` (not `Eq`): `alert_rules` carries `f64` fields
/// (`AlertCondition::Threshold`/`Ratio`), which aren't `Eq`.
/// The single, process-wide settings instance. Only `PartialEq` (not `Eq`): `alert_rules` carries `f64` fields, which aren't `Eq`.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct Settings {
pub color_scheme: ColorScheme,
pub css_entries: Vec<CssEntry>,
/// Id of the selected custom theme entry (a `css_entries` entry with
/// `is_theme: true`), if any (ticket 033). `None` means `color_scheme`
/// alone determines the active palette. `#[serde(default)]` so existing
/// settings.json files without this field still load.
/// Id of the selected custom theme entry, if any; `None` means `color_scheme` alone determines the active palette.
#[serde(default)]
pub custom_theme_id: Option<String>,
pub profiles: Profiles,
/// Maximum number of ingested logs kept in memory. `None` (the default)
/// means unlimited — matches today's unbounded retention.
/// Maximum number of ingested logs kept in memory; `None` means unlimited.
#[serde(default)]
pub max_logs: Option<usize>,
/// Alert rule definitions (ticket 017). `#[serde(default)]` so existing
/// settings.json files without this field still load.
/// Alert rule definitions.
#[serde(default)]
pub alert_rules: Vec<AlertRule>,
}
@@ -125,23 +97,13 @@ impl Default for Settings {
}
}
/// Computes the default settings file path using OS-conventional config
/// directories: `<config_dir>/settings.json`, where `config_dir` comes from
/// `directories::ProjectDirs::from("", "", "ottyl")`.
///
/// Returns `None` if no valid config directory could be determined for the
/// current OS/user (this mirrors `ProjectDirs::from`'s own fallibility).
/// Computes the default settings file path (`<config_dir>/settings.json`) from OS-conventional config directories; `None` if none could be determined.
pub fn default_settings_path() -> Option<PathBuf> {
ProjectDirs::from("", "", "ottyl").map(|dirs| dirs.config_dir().join("settings.json"))
}
impl Settings {
/// Loads settings from `path`. If the file doesn't exist yet, a default
/// `Settings` is created, persisted to `path`, and returned.
///
/// Any I/O or parse error while reading an existing file is treated as
/// non-fatal: it is logged to stderr and a default `Settings` is
/// returned (without overwriting the unreadable file).
/// Loads settings from `path`, creating and persisting defaults if missing. Any I/O or parse error on an existing file is logged and treated as defaults, without overwriting the unreadable file.
pub fn load(path: &Path) -> Settings {
match fs::read_to_string(path) {
Ok(contents) => match serde_json::from_str(&contents) {
@@ -174,8 +136,7 @@ impl Settings {
}
}
/// Persists these settings to `path` as pretty-printed JSON, creating
/// any missing parent directories first.
/// Persists these settings to `path` as pretty-printed JSON, creating any missing parent directories first.
pub fn save(&self, path: &Path) -> io::Result<()> {
if let Some(parent) = path.parent() {
fs::create_dir_all(parent)?;
@@ -186,18 +147,14 @@ impl Settings {
}
}
/// Error returned by CSS-entry mutations and lookups (ticket 010).
/// Error returned by CSS-entry mutations and lookups.
#[derive(Debug)]
pub enum CssError {
/// No entry with the given id exists.
NotFound,
/// The entry exists but is a `Linked` entry, not an `Imported` one, so
/// the requested operation (delete / read-content) doesn't apply: ottyl
/// never owns a linked entry's content, so there's nothing to delete or
/// serve back.
/// The entry is a `Linked` entry, not `Imported` — ottyl doesn't own a linked entry's content, so there's nothing to delete or serve back.
NotImported,
/// Underlying I/O failure: writing/reading/removing the CSS file on
/// disk, or persisting `settings.json`.
/// Underlying I/O failure: writing/reading/removing the CSS file, or persisting `settings.json`.
Io(io::Error),
}
@@ -219,14 +176,7 @@ impl From<io::Error> for CssError {
}
}
/// Computes the id used to reference a `CssEntry` over the API.
///
/// For `Imported` entries this is just the stored id. `Linked` entries have
/// no stored id (ottyl doesn't own them), so one is derived deterministically
/// by hashing the `path_or_url`; this lets the frontend key/reference linked
/// entries the same way as imported ones (e.g. for list rendering) without
/// ottyl needing to persist an id it has no other use for. Because it's
/// derived from content, the same `path_or_url` always maps to the same id.
/// Computes the id used to reference a `CssEntry` over the API: the stored id for `Imported`, or a hash of `path_or_url` for `Linked` (which has no stored id), so the frontend can key both the same way.
pub fn css_entry_id(entry: &CssEntry) -> String {
match entry {
CssEntry::Imported { id, .. } => id.clone(),
@@ -240,10 +190,7 @@ fn linked_entry_id(path_or_url: &str) -> String {
format!("linked-{:016x}", hasher.finish())
}
/// Generates an id for a newly-imported CSS entry. Not cryptographically
/// meaningful — just needs to be unique enough in practice: it hashes the
/// filename, content, and current time together, avoiding a new dependency
/// (e.g. `uuid`) for what's otherwise a random-ish opaque string.
/// Generates an id for a newly-imported CSS entry by hashing filename, content, and time, avoiding a `uuid` dependency for what's otherwise an opaque string.
fn generate_imported_css_id(filename: &str, content: &str) -> String {
let mut hasher = DefaultHasher::new();
filename.hash(&mut hasher);
@@ -261,15 +208,7 @@ fn imported_css_file_path(css_dir: &Path, id: &str) -> PathBuf {
}
impl Settings {
/// Imports raw CSS `content`: writes it to `<css_dir>/<id>.css` (id
/// generated from the filename/content/timestamp, see
/// `generate_imported_css_id`), records a `CssEntry::Imported { id,
/// filename, is_theme }` in `css_entries`, and persists settings to
/// `settings_path`. Returns the generated id.
///
/// `css_dir` is expected to be the settings file's parent directory
/// (i.e. CSS files live alongside `settings.json`). `is_theme` (ticket
/// 033) marks this entry as a selectable theme rather than arbitrary CSS.
/// Imports raw CSS `content`: writes it to `<css_dir>/<id>.css`, records a `CssEntry::Imported` in `css_entries`, and persists settings. Returns the generated id. `css_dir` is expected to be the settings file's parent directory.
pub fn import_css(
&mut self,
settings_path: &Path,
@@ -290,9 +229,7 @@ impl Settings {
Ok(id)
}
/// Registers a reference to an external CSS path/URL without copying
/// its content. Persists settings to `settings_path`. `is_theme` (ticket
/// 033) marks this entry as a selectable theme rather than arbitrary CSS.
/// Registers a reference to an external CSS path/URL without copying its content, and persists settings.
pub fn link_css(
&mut self,
settings_path: &Path,
@@ -307,14 +244,7 @@ impl Settings {
Ok(())
}
/// Removes an imported CSS entry identified by `id` (see
/// `css_entry_id`): deletes its file under `css_dir` and drops it from
/// `css_entries`, then persists settings to `settings_path`.
///
/// Returns `CssError::NotFound` if no entry matches `id`, and
/// `CssError::NotImported` if `id` refers to a `Linked` entry — linked
/// entries are never deletable this way since ottyl doesn't own their
/// content.
/// Removes an imported CSS entry identified by `id`: deletes its file under `css_dir`, drops it from `css_entries`, and persists settings. Fails with `NotFound` or `NotImported` (linked entries aren't deletable this way).
pub fn remove_css(
&mut self,
settings_path: &Path,
@@ -331,8 +261,7 @@ impl Settings {
Some(CssEntry::Imported { .. }) => {
let idx = index.unwrap();
self.css_entries.remove(idx);
// Best-effort: if the file is already gone, the settings
// reference is still what matters and has been removed above.
// Best-effort: the settings entry is already removed even if the file is gone.
let _ = fs::remove_file(imported_css_file_path(css_dir, id));
self.save(settings_path)?;
Ok(())
@@ -340,13 +269,7 @@ impl Settings {
}
}
/// Reads back the raw content of an imported CSS entry identified by
/// `id` (see `css_entry_id`).
///
/// Returns `CssError::NotFound` if no entry matches `id`, and
/// `CssError::NotImported` if `id` refers to a `Linked` entry (there's no
/// local content to serve for those — the frontend should link/fetch the
/// original `path_or_url` directly instead).
/// Reads back the raw content of an imported CSS entry identified by `id`. Fails with `NotFound` or `NotImported` (linked entries have no local content — the frontend fetches `path_or_url` directly).
pub fn css_content(&self, css_dir: &Path, id: &str) -> Result<String, CssError> {
match self.css_entries.iter().find(|entry| css_entry_id(entry) == id) {
None => Err(CssError::NotFound),
@@ -364,9 +287,7 @@ mod tests {
#[test]
fn default_settings_path_ends_with_settings_json() {
// We can't assert the exact OS path here without touching real
// config dirs, but we can verify the path-computation logic shapes
// it as expected.
// Can't assert the exact OS path without touching real config dirs, so just check the shape.
if let Some(path) = default_settings_path() {
assert_eq!(path.file_name().unwrap(), "settings.json");
assert!(path.parent().is_some());