From ed140ad38a495e60eb16adab6cc552fef31b811b Mon Sep 17 00:00:00 2001 From: Romain Bertrand Date: Mon, 7 Sep 2026 13:20:39 +0200 Subject: [PATCH] refactor: cut over-engineering from repo-wide audit - delete todoist re-export shim, import todoist_api directly - drop dead backends.is_enabled/settings columns (schema v2) - replace once_cell with std LazyLock (rust-version 1.80) - replace fern with a ~30-line log::Log impl (same memory buffer and file format) - replace hand-rolled weekday_name with chrono's Weekday Display Config note: rust-version bumped 1.78 -> 1.80 for LazyLock. Existing local caches rebuild automatically via the schema version bump. --- .github/workflows/ci.yml | 4 +- CONTRIBUTING.md | 2 +- Cargo.lock | 11 ---- Cargo.toml | 4 +- README.md | 2 +- docs/DEVELOPMENT.md | 2 +- docs/PRD.md | 2 +- src/backend/todoist.rs | 22 ++++---- src/backend_registry.rs | 24 +------- src/entities/backend.rs | 2 - src/lib.rs | 4 -- src/logger.rs | 91 +++++++++++++++---------------- src/main.rs | 11 +--- src/storage.rs | 2 +- src/sync/storage.rs | 2 - src/todoist.rs | 7 --- src/utils/datetime.rs | 19 +------ tests/storage/db.rs | 8 +-- tests/ui/app_component/startup.rs | 2 - tests/utils/datetime.rs | 9 +++ 20 files changed, 82 insertions(+), 148 deletions(-) delete mode 100644 src/todoist.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64460a0..4576772 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,14 +67,14 @@ jobs: # MSRV build job msrv: - name: MSRV 1.78 Build + name: MSRV 1.80 Build runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - name: Install Rust (MSRV) uses: dtolnay/rust-toolchain@stable with: - toolchain: 1.78 + toolchain: 1.80 components: rustfmt, clippy - name: Rust cache uses: Swatinem/rust-cache@v2 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4f49eaf..54d43ba 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,7 +4,7 @@ Thanks for taking the time to contribute! ## Development setup -- Rust 1.78+ (MSRV pinned) +- Rust 1.80+ (MSRV pinned) - Install components: `rustup component add rustfmt clippy` ## Workflow diff --git a/Cargo.lock b/Cargo.lock index fdc8e10..b447cb7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -682,15 +682,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd2e7510819d6fbf51a5545c8f922716ecfb14df168a3242f7d33e0239efe6a1" -[[package]] -name = "fern" -version = "0.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4316185f709b23713e41e3195f90edef7fb00c3ed4adc79769cf09cc762a3b29" -dependencies = [ - "log", -] - [[package]] name = "filedescriptor" version = "0.8.3" @@ -3095,9 +3086,7 @@ dependencies = [ "chrono", "crossterm", "dirs", - "fern", "log", - "once_cell", "ratatui", "sea-orm", "serde", diff --git a/Cargo.toml b/Cargo.toml index d5f8d69..1380615 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "terminalist" version = "0.5.0" edition = "2021" -rust-version = "1.78" +rust-version = "1.80" keywords = ["terminal", "tui", "todoist", "productivity", "cli"] description = "A terminal-based Todoist client with modern TUI interface" authors = ["Romain Bertrand "] @@ -40,6 +40,4 @@ todoist-api = "1.0.0-alpha.3" toml = "1.0" dirs = "6.0" log = "0.4" -fern = "0.7" -once_cell = "1.21" uuid = { version = "1.23", features = ["v4", "serde"] } diff --git a/README.md b/README.md index cd71102..a2fdd1e 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Terminalist - Todoist Terminal Client -[![Rust](https://img.shields.io/badge/rust-1.78%2B-orange.svg)](https://www.rust-lang.org) +[![Rust](https://img.shields.io/badge/rust-1.80%2B-orange.svg)](https://www.rust-lang.org) [![Build Status](https://github.com/romaintb/terminalist/workflows/CI/badge.svg)](https://github.com/romaintb/terminalist/actions) [![Crates.io](https://img.shields.io/crates/v/terminalist.svg)](https://crates.io/crates/terminalist) [![License](https://img.shields.io/badge/license-MIT-blue.svg)](LICENSE) diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index f03f01f..5813953 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -50,7 +50,7 @@ GitHub Actions workflow is configured in `.github/workflows/ci.yml` with: - Format checking with rustfmt - Linting with clippy - Testing on multiple Rust versions and OSes -- MSRV 1.78 build job +- MSRV 1.80 build job - Smoke tests for `--help` and `--version` - Security auditing diff --git a/docs/PRD.md b/docs/PRD.md index f9df1bc..ae0b29c 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -159,7 +159,7 @@ Terminalist is a high-performance terminal user interface (TUI) application for - **NFR-017**: Supports terminals with 80x24 minimum size - **NFR-018**: Compatible with major terminal emulators - **NFR-019**: Works with Todoist API v2 -- **NFR-020**: Rust 1.78+ compatibility +- **NFR-020**: Rust 1.80+ compatibility ## 4. User Stories diff --git a/src/backend/todoist.rs b/src/backend/todoist.rs index 8bee0c0..190049a 100644 --- a/src/backend/todoist.rs +++ b/src/backend/todoist.rs @@ -4,8 +4,8 @@ use super::{ Backend, BackendError, BackendLabel, BackendProject, BackendSection, BackendTask, CreateLabelArgs, CreateProjectArgs, CreateTaskArgs, UpdateLabelArgs, UpdateProjectArgs, UpdateTaskArgs, }; -use crate::todoist::{TodoistError, TodoistWrapper}; use async_trait::async_trait; +use todoist_api::{TodoistError, TodoistWrapper}; fn map_err(e: TodoistError) -> BackendError { match e { @@ -38,7 +38,7 @@ impl TodoistBackend { } // Helper: Transform Todoist API project → Backend project - fn project_to_backend(api_project: &crate::todoist::Project) -> BackendProject { + fn project_to_backend(api_project: &todoist_api::Project) -> BackendProject { BackendProject { remote_id: api_project.id.clone(), name: api_project.name.clone(), @@ -50,7 +50,7 @@ impl TodoistBackend { } // Helper: Transform Todoist API task → Backend task - fn task_to_backend(api_task: &crate::todoist::Task) -> BackendTask { + fn task_to_backend(api_task: &todoist_api::Task) -> BackendTask { BackendTask { remote_id: api_task.id.clone(), content: api_task.content.clone(), @@ -71,7 +71,7 @@ impl TodoistBackend { } // Helper: Transform Todoist API label → Backend label - fn label_to_backend(api_label: &crate::todoist::Label) -> BackendLabel { + fn label_to_backend(api_label: &todoist_api::Label) -> BackendLabel { BackendLabel { remote_id: api_label.id.clone(), name: api_label.name.clone(), @@ -81,7 +81,7 @@ impl TodoistBackend { } // Helper: Transform Todoist API section → Backend section - fn section_to_backend(api_section: &crate::todoist::Section) -> BackendSection { + fn section_to_backend(api_section: &todoist_api::Section) -> BackendSection { BackendSection { remote_id: api_section.id.clone(), name: api_section.name.clone(), @@ -174,7 +174,7 @@ impl Backend for TodoistBackend { } async fn create_project(&self, args: CreateProjectArgs) -> Result { - let todoist_args = crate::todoist::CreateProjectArgs { + let todoist_args = todoist_api::CreateProjectArgs { name: args.name, color: None, is_favorite: args.is_favorite, @@ -187,7 +187,7 @@ impl Backend for TodoistBackend { } async fn update_project(&self, remote_id: &str, args: UpdateProjectArgs) -> Result { - let todoist_args = crate::todoist::UpdateProjectArgs { + let todoist_args = todoist_api::UpdateProjectArgs { name: args.name, color: None, is_favorite: args.is_favorite, @@ -203,7 +203,7 @@ impl Backend for TodoistBackend { } async fn create_task(&self, args: CreateTaskArgs) -> Result { - let todoist_args = crate::todoist::CreateTaskArgs { + let todoist_args = todoist_api::CreateTaskArgs { content: args.content, description: args.description, project_id: Some(args.project_remote_id), @@ -230,7 +230,7 @@ impl Backend for TodoistBackend { } async fn update_task(&self, remote_id: &str, args: UpdateTaskArgs) -> Result { - let todoist_args = crate::todoist::UpdateTaskArgs { + let todoist_args = todoist_api::UpdateTaskArgs { content: args.content, description: args.description, priority: args.priority, @@ -266,7 +266,7 @@ impl Backend for TodoistBackend { } async fn create_label(&self, args: CreateLabelArgs) -> Result { - let todoist_args = crate::todoist::CreateLabelArgs { + let todoist_args = todoist_api::CreateLabelArgs { name: args.name, color: None, is_favorite: args.is_favorite, @@ -278,7 +278,7 @@ impl Backend for TodoistBackend { } async fn update_label(&self, remote_id: &str, args: UpdateLabelArgs) -> Result { - let todoist_args = crate::todoist::UpdateLabelArgs { + let todoist_args = todoist_api::UpdateLabelArgs { name: args.name, color: None, is_favorite: args.is_favorite, diff --git a/src/backend_registry.rs b/src/backend_registry.rs index 35fb0f2..2870e24 100644 --- a/src/backend_registry.rs +++ b/src/backend_registry.rs @@ -81,20 +81,13 @@ impl BackendRegistry { /// * `backend_type` - Backend type (e.g., "todoist") /// * `name` - Human-readable name /// * `credentials` - JSON-encoded credentials - /// * `settings` - JSON-encoded settings /// /// # Returns /// UUID of the created backend /// /// # Errors /// Returns error if backend creation fails or database insert fails - pub async fn add_backend( - &self, - backend_type: String, - name: String, - credentials: String, - settings: String, - ) -> Result { + pub async fn add_backend(&self, backend_type: String, name: String, credentials: String) -> Result { // Validate by creating instance first let backend_instance = factory::create_backend(&backend_type, &credentials)?; @@ -104,9 +97,7 @@ impl BackendRegistry { uuid: ActiveValue::Set(uuid), backend_type: ActiveValue::Set(backend_type.clone()), name: ActiveValue::Set(name.clone()), - is_enabled: ActiveValue::Set(true), credentials: ActiveValue::Set(credentials), - settings: ActiveValue::Set(settings), }; let storage = self.storage.lock().await; @@ -126,17 +117,10 @@ impl BackendRegistry { /// * `uuid` - Backend UUID /// * `name` - Optional new name /// * `credentials` - Optional new credentials - /// * `settings` - Optional new settings /// /// # Errors /// Returns error if backend not found or update fails - pub async fn update_backend( - &self, - uuid: &Uuid, - name: Option, - credentials: Option, - settings: Option, - ) -> Result<()> { + pub async fn update_backend(&self, uuid: &Uuid, name: Option, credentials: Option) -> Result<()> { let storage = self.storage.lock().await; let backend_model = BackendRepository::get_by_uuid(&storage.conn, uuid) @@ -160,10 +144,6 @@ impl BackendRegistry { backends.insert(*uuid, Arc::new(backend_instance)); } - if let Some(settings) = settings { - active_model.settings = ActiveValue::Set(settings); - } - BackendRepository::update(&storage.conn, active_model).await?; info!("✅ Updated backend: {}", uuid); diff --git a/src/entities/backend.rs b/src/entities/backend.rs index 4b5277b..fa8073a 100644 --- a/src/entities/backend.rs +++ b/src/entities/backend.rs @@ -10,9 +10,7 @@ pub struct Model { pub uuid: Uuid, pub backend_type: String, pub name: String, - pub is_enabled: bool, pub credentials: String, // JSON-encoded credentials (to be encrypted in future) - pub settings: String, // JSON-encoded backend-specific settings } #[derive(Copy, Clone, Debug, EnumIter, DeriveRelation)] diff --git a/src/lib.rs b/src/lib.rs index abc00d0..d93c6fe 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,6 @@ //! * [`config`] - Application configuration management //! * [`storage`] - Local database and data persistence //! * [`sync`] - Synchronization with Todoist API -//! * [`todoist`] - Todoist API client and data structures //! * [`ui`] - Terminal user interface components //! * [`utils`] - Utility functions and helpers @@ -49,9 +48,6 @@ pub mod sync; /// Theme configuration (semantic color palette) for the TUI pub mod theme; -/// Todoist API client and data models -pub(crate) mod todoist; - /// Terminal user interface components and rendering pub mod ui; diff --git a/src/logger.rs b/src/logger.rs index 6d93843..6065b5c 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -1,48 +1,38 @@ use crate::constants::MEMORY_LOGS_LIMIT; use chrono::Utc; -use log::Record; +use log::{Log, Metadata, Record}; use std::collections::VecDeque; -use std::fs::OpenOptions; -use std::io; +use std::fs::{File, OpenOptions}; +use std::io::{self, Write}; use std::path::PathBuf; -use std::sync::{Arc, Mutex}; +use std::sync::{LazyLock, Mutex}; /// Global in-memory log storage for UI display -static MEMORY_LOGS: once_cell::sync::Lazy>>> = - once_cell::sync::Lazy::new(|| Arc::new(Mutex::new(VecDeque::with_capacity(5000)))); +static MEMORY_LOGS: LazyLock>> = + LazyLock::new(|| Mutex::new(VecDeque::with_capacity(MEMORY_LOGS_LIMIT))); -/// Initialize the fern logger with file and memory outputs -pub fn init_logger(enabled: bool) -> io::Result<()> { - if !enabled { - // Set up a logger that only writes to memory - // Use Trace level so MemoryLogger receives all logs - fern::Dispatch::new() - .level(log::LevelFilter::Info) - .chain(Box::new(MemoryLogger) as Box) - .apply() - .map_err(io::Error::other)?; - return Ok(()); - } +static LOGGER: TerminalistLogger = TerminalistLogger { file: Mutex::new(None) }; - let log_file_path = get_log_file_path()?; +/// Logger writing to memory (for the UI) and, when enabled, to a file. +struct TerminalistLogger { + file: Mutex>, +} - // Ensure the config directory exists - if let Some(parent) = log_file_path.parent() { - std::fs::create_dir_all(parent)?; - } +/// Initialize the logger, optionally also writing to the log file. +pub fn init_logger(enabled: bool) -> io::Result<()> { + if enabled { + let log_file_path = get_log_file_path()?; - // Open log file in append mode - let log_file = OpenOptions::new().create(true).append(true).open(&log_file_path)?; + if let Some(parent) = log_file_path.parent() { + std::fs::create_dir_all(parent)?; + } - // Configure fern logger - fern::Dispatch::new() - .format(|out, message, _record| out.finish(format_args!("[{}] {}", Utc::now().format("%H:%M:%S%.3f"), message))) - .level(log::LevelFilter::Info) - .chain(log_file) - .chain(Box::new(MemoryLogger) as Box) - .apply() - .map_err(io::Error::other)?; + let log_file = OpenOptions::new().create(true).append(true).open(&log_file_path)?; + *LOGGER.file.lock().map_err(|_| io::Error::other("logger lock poisoned"))? = Some(log_file); + } + log::set_logger(&LOGGER).map_err(|e| io::Error::other(e.to_string()))?; + log::set_max_level(log::LevelFilter::Info); Ok(()) } @@ -63,27 +53,34 @@ pub fn get_memory_logs() -> Vec { } } -/// Custom logger that stores logs in memory for UI display -struct MemoryLogger; - -impl log::Log for MemoryLogger { - fn enabled(&self, _metadata: &log::Metadata) -> bool { +impl Log for TerminalistLogger { + fn enabled(&self, _metadata: &Metadata) -> bool { true } fn log(&self, record: &Record) { - if self.enabled(record.metadata()) { - let formatted = format!("{}", record.args()); + let message = format!("{}", record.args()); + + if let Ok(mut logs) = MEMORY_LOGS.lock() { + logs.push_back(message.clone()); + // Keep only the last MEMORY_LOGS_LIMIT entries + while logs.len() > MEMORY_LOGS_LIMIT { + logs.pop_front(); + } + } - if let Ok(mut logs) = MEMORY_LOGS.lock() { - logs.push_back(formatted); - // Keep only last 5000 entries - while logs.len() > MEMORY_LOGS_LIMIT { - logs.pop_front(); - } + if let Ok(mut file) = self.file.lock() { + if let Some(file) = file.as_mut() { + let _ = writeln!(file, "[{}] {}", Utc::now().format("%H:%M:%S%.3f"), message); } } } - fn flush(&self) {} + fn flush(&self) { + if let Ok(mut file) = self.file.lock() { + if let Some(file) = file.as_mut() { + let _ = file.flush(); + } + } + } } diff --git a/src/main.rs b/src/main.rs index c6dd6ce..541aaa8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -123,18 +123,11 @@ async fn main() -> Result<()> { .find(|backend| backend.backend_type == "todoist"); let backend_uuid = if let Some(backend) = existing_todoist { - backend_registry - .update_backend(&backend.uuid, None, Some(credentials), None) - .await?; + backend_registry.update_backend(&backend.uuid, None, Some(credentials)).await?; backend.uuid } else { backend_registry - .add_backend( - "todoist".to_string(), - "My Todoist".to_string(), - credentials, - "{}".to_string(), - ) + .add_backend("todoist".to_string(), "My Todoist".to_string(), credentials) .await? }; diff --git a/src/storage.rs b/src/storage.rs index c335892..5422295 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -11,7 +11,7 @@ use crate::entities::{backend, label, project, section, task, task_label}; /// /// The database is a disposable cache, not a source of truth, so a mismatch drops every /// table and rebuilds from scratch instead of running a migration. -const SCHEMA_VERSION: i32 = 1; +const SCHEMA_VERSION: i32 = 2; /// Local storage manager for Todoist data pub struct LocalStorage { diff --git a/src/sync/storage.rs b/src/sync/storage.rs index 39bdaca..63159e4 100644 --- a/src/sync/storage.rs +++ b/src/sync/storage.rs @@ -507,9 +507,7 @@ mod tests { uuid: Set(backend_uuid), backend_type: Set("test".to_string()), name: Set("Test".to_string()), - is_enabled: Set(true), credentials: Set("{}".to_string()), - settings: Set("{}".to_string()), }) .exec(&storage.conn) .await diff --git a/src/todoist.rs b/src/todoist.rs deleted file mode 100644 index 1cc50e9..0000000 --- a/src/todoist.rs +++ /dev/null @@ -1,7 +0,0 @@ -//! Todoist API integration for the terminalist application. -//! -//! This module provides a bridge between the external `todoist_api` crate and the -//! terminalist application. It re-exports the Todoist API functionality. - -// Re-export the Todoist API library for external use -pub use todoist_api::*; diff --git a/src/utils/datetime.rs b/src/utils/datetime.rs index b27d8c4..dba5527 100644 --- a/src/utils/datetime.rs +++ b/src/utils/datetime.rs @@ -79,13 +79,11 @@ pub fn format_human_date(date_str: &str) -> String { 1 => "tomorrow".to_string(), diff if diff > 1 && diff <= 7 => { // Within the next week - show day name - let weekday = input_date.weekday(); - format!("next {}", weekday_name(weekday)) + format!("next {}", input_date.format("%A")) } diff if (-7..-1).contains(&diff) => { // Within the past week - show day name - let weekday = input_date.weekday(); - format!("last {}", weekday_name(weekday)) + format!("last {}", input_date.format("%A")) } diff if diff > 7 && diff <= 30 => { // Within the next month - show "in X days" @@ -159,16 +157,3 @@ pub fn format_human_datetime(datetime_str: &str) -> String { format_human_date(datetime_str) } } - -/// Get a human-readable weekday name -fn weekday_name(weekday: Weekday) -> &'static str { - match weekday { - Weekday::Mon => "Monday", - Weekday::Tue => "Tuesday", - Weekday::Wed => "Wednesday", - Weekday::Thu => "Thursday", - Weekday::Fri => "Friday", - Weekday::Sat => "Saturday", - Weekday::Sun => "Sunday", - } -} diff --git a/tests/storage/db.rs b/tests/storage/db.rs index 8e8054d..ec26fae 100644 --- a/tests/storage/db.rs +++ b/tests/storage/db.rs @@ -18,8 +18,8 @@ async fn test_local_storage_creation() { .execute(Statement::from_string( DbBackend::Sqlite, "INSERT INTO backends \ - (uuid, backend_type, name, is_enabled, credentials, settings) \ - VALUES ('00000000-0000-0000-0000-000000000001', 'test', 'Test', 1, '{}', '{}')" + (uuid, backend_type, name, credentials) \ + VALUES ('00000000-0000-0000-0000-000000000001', 'test', 'Test', '{}')" .to_owned(), )) .await @@ -44,8 +44,8 @@ async fn test_stale_schema_version_rebuilds_cache() { .execute(Statement::from_string( DbBackend::Sqlite, "INSERT INTO backends \ - (uuid, backend_type, name, is_enabled, credentials, settings) \ - VALUES ('00000000-0000-0000-0000-000000000002', 'test', 'Test', 1, '{}', '{}')" + (uuid, backend_type, name, credentials) \ + VALUES ('00000000-0000-0000-0000-000000000002', 'test', 'Test', '{}')" .to_owned(), )) .await diff --git a/tests/ui/app_component/startup.rs b/tests/ui/app_component/startup.rs index 5245fa7..9e47566 100644 --- a/tests/ui/app_component/startup.rs +++ b/tests/ui/app_component/startup.rs @@ -20,9 +20,7 @@ async fn startup_loads_cached_data_when_the_backend_is_unavailable() { uuid: Set(backend_uuid), backend_type: Set("test".to_string()), name: Set("Unavailable backend".to_string()), - is_enabled: Set(true), credentials: Set("{}".to_string()), - settings: Set("{}".to_string()), }) .exec(&storage.conn) .await diff --git a/tests/utils/datetime.rs b/tests/utils/datetime.rs index b6fe084..ac73215 100644 --- a/tests/utils/datetime.rs +++ b/tests/utils/datetime.rs @@ -48,6 +48,15 @@ fn test_format_human_date_yesterday() { assert_eq!(format_human_date(&yesterday), "yesterday"); } +#[test] +fn test_format_human_date_within_week_shows_full_weekday_name() { + let date = chrono::Local::now().date_naive() + chrono::Duration::days(3); + let expected = format!("next {}", date.format("%A")); + assert_eq!(format_human_date(&format_ymd(date)), expected); + // Guard against abbreviated names sneaking back in ("next Sat" instead of "next Saturday") + assert!(expected.len() > "next Sat".len()); +} + #[test] fn test_format_human_datetime_iso_format() { // Test the specific format from the user's example