From aabc0167bc013fd2d0c61a687580f6e69305500a Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Sun, 29 Mar 2026 00:17:02 +0100 Subject: [PATCH 1/7] fix(cleaning): constant extract --- src/cmds/system/constants.rs | 28 ++++++++ src/cmds/system/ls.rs | 29 +------- src/cmds/system/mod.rs | 1 + src/cmds/system/tree.rs | 31 +------- src/core/config.rs | 5 +- src/core/constants.rs | 6 ++ src/core/mod.rs | 1 + src/core/tee.rs | 3 +- src/core/telemetry.rs | 3 +- src/core/toml_filter.rs | 3 +- src/core/tracking.rs | 7 +- src/discover/provider.rs | 3 +- src/discover/report.rs | 6 +- src/hooks/constants.rs | 9 +++ src/hooks/hook_check.rs | 11 ++- src/hooks/hook_cmd.rs | 3 +- src/hooks/init.rs | 135 +++++++++++++++++++---------------- src/hooks/integrity.rs | 9 ++- src/hooks/mod.rs | 1 + src/hooks/permissions.rs | 11 +-- src/hooks/trust.rs | 3 +- 21 files changed, 167 insertions(+), 141 deletions(-) create mode 100644 src/cmds/system/constants.rs create mode 100644 src/core/constants.rs create mode 100644 src/hooks/constants.rs diff --git a/src/cmds/system/constants.rs b/src/cmds/system/constants.rs new file mode 100644 index 00000000..2454f529 --- /dev/null +++ b/src/cmds/system/constants.rs @@ -0,0 +1,28 @@ +pub const NOISE_DIRS: &[&str] = &[ + "node_modules", + ".git", + "target", + "__pycache__", + ".next", + "dist", + "build", + ".cache", + ".turbo", + ".vercel", + ".pytest_cache", + ".mypy_cache", + ".tox", + ".venv", + "venv", + "env", + ".env", + "coverage", + ".nyc_output", + ".DS_Store", + "Thumbs.db", + ".idea", + ".vscode", + ".vs", + "*.egg-info", + ".eggs", +]; diff --git a/src/cmds/system/ls.rs b/src/cmds/system/ls.rs index 123e8968..998d4879 100644 --- a/src/cmds/system/ls.rs +++ b/src/cmds/system/ls.rs @@ -1,38 +1,11 @@ //! Filters directory listings into a compact tree format. +use super::constants::NOISE_DIRS; use crate::core::tracking; use crate::core::utils::{exit_code_from_output, resolved_command}; use anyhow::{Context, Result}; use std::io::IsTerminal; -/// Noise directories commonly excluded from LLM context -const NOISE_DIRS: &[&str] = &[ - "node_modules", - ".git", - "target", - "__pycache__", - ".next", - "dist", - "build", - ".cache", - ".turbo", - ".vercel", - ".pytest_cache", - ".mypy_cache", - ".tox", - ".venv", - "venv", - "coverage", - ".nyc_output", - ".DS_Store", - "Thumbs.db", - ".idea", - ".vscode", - ".vs", - "*.egg-info", - ".eggs", -]; - pub fn run(args: &[String], verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); diff --git a/src/cmds/system/mod.rs b/src/cmds/system/mod.rs index a7686922..b79c7165 100644 --- a/src/cmds/system/mod.rs +++ b/src/cmds/system/mod.rs @@ -1,5 +1,6 @@ //! General-purpose system command filters. +pub mod constants; pub mod deps; pub mod env_cmd; pub mod find_cmd; diff --git a/src/cmds/system/tree.rs b/src/cmds/system/tree.rs index 5a381771..62cb230e 100644 --- a/src/cmds/system/tree.rs +++ b/src/cmds/system/tree.rs @@ -6,40 +6,11 @@ //! Token optimization: automatically excludes noise directories via -I pattern //! unless -a flag is present (respecting user intent). +use super::constants::NOISE_DIRS; use crate::core::tracking; use crate::core::utils::{exit_code_from_output, resolved_command, tool_exists}; use anyhow::{Context, Result}; -/// Noise directories commonly excluded from LLM context -const NOISE_DIRS: &[&str] = &[ - "node_modules", - ".git", - "target", - "__pycache__", - ".next", - "dist", - "build", - ".cache", - ".turbo", - ".vercel", - ".pytest_cache", - ".mypy_cache", - ".tox", - ".venv", - "venv", - "env", - ".env", - "coverage", - ".nyc_output", - ".DS_Store", - "Thumbs.db", - ".idea", - ".vscode", - ".vs", - "*.egg-info", - ".eggs", -]; - pub fn run(args: &[String], verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); diff --git a/src/core/config.rs b/src/core/config.rs index 99e28c21..248f80a3 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -1,5 +1,6 @@ //! Reads user settings from config.toml. +use super::constants::{CONFIG_TOML, DEFAULT_HISTORY_DAYS, RTK_DATA_DIR}; use anyhow::Result; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -42,7 +43,7 @@ impl Default for TrackingConfig { fn default() -> Self { Self { enabled: true, - history_days: 90, + history_days: DEFAULT_HISTORY_DAYS as u32, database_path: None, } } @@ -168,7 +169,7 @@ impl Config { fn get_config_path() -> Result { let config_dir = dirs::config_dir().unwrap_or_else(|| PathBuf::from(".")); - Ok(config_dir.join("rtk").join("config.toml")) + Ok(config_dir.join(RTK_DATA_DIR).join(CONFIG_TOML)) } pub fn show_config() -> Result<()> { diff --git a/src/core/constants.rs b/src/core/constants.rs new file mode 100644 index 00000000..a5ecd384 --- /dev/null +++ b/src/core/constants.rs @@ -0,0 +1,6 @@ +pub const RTK_DATA_DIR: &str = "rtk"; +pub const HISTORY_DB: &str = "history.db"; +pub const CONFIG_TOML: &str = "config.toml"; +pub const FILTERS_TOML: &str = "filters.toml"; +pub const TRUSTED_FILTERS_JSON: &str = "trusted_filters.json"; +pub const DEFAULT_HISTORY_DAYS: i64 = 90; diff --git a/src/core/mod.rs b/src/core/mod.rs index b3a5eeb3..c5d1e930 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -1,6 +1,7 @@ //! Building blocks shared across all RTK modules. pub mod config; +pub mod constants; pub mod display_helpers; pub mod filter; pub mod runner; diff --git a/src/core/tee.rs b/src/core/tee.rs index a4cf3ccc..ef8177f2 100644 --- a/src/core/tee.rs +++ b/src/core/tee.rs @@ -1,5 +1,6 @@ //! Raw output recovery -- saves unfiltered output to disk on command failure. +use super::constants::RTK_DATA_DIR; use crate::core::config::Config; use std::path::PathBuf; @@ -46,7 +47,7 @@ fn get_tee_dir(config: &Config) -> Option { } // Default: ~/.local/share/rtk/tee/ - dirs::data_local_dir().map(|d| d.join("rtk").join("tee")) + dirs::data_local_dir().map(|d| d.join(RTK_DATA_DIR).join("tee")) } /// Rotate old tee files: keep only the last `max_files`, delete oldest. diff --git a/src/core/telemetry.rs b/src/core/telemetry.rs index 3b972955..bba19991 100644 --- a/src/core/telemetry.rs +++ b/src/core/telemetry.rs @@ -1,5 +1,6 @@ //! Optional usage ping so we know which commands people run most. +use super::constants::RTK_DATA_DIR; use crate::core::config; use crate::core::tracking; use sha2::{Digest, Sha256}; @@ -215,7 +216,7 @@ fn install_method_from_path(path: &str) -> &'static str { fn telemetry_marker_path() -> PathBuf { let data_dir = dirs::data_local_dir() .unwrap_or_else(|| PathBuf::from("/tmp")) - .join("rtk"); + .join(RTK_DATA_DIR); let _ = std::fs::create_dir_all(&data_dir); data_dir.join(".telemetry_last_ping") } diff --git a/src/core/toml_filter.rs b/src/core/toml_filter.rs index 62597717..bd294f8a 100644 --- a/src/core/toml_filter.rs +++ b/src/core/toml_filter.rs @@ -22,6 +22,7 @@ /// 6. head/tail_lines — keep first/last N lines /// 7. max_lines — absolute line cap /// 8. on_empty — message if result is empty +use super::constants::{FILTERS_TOML, RTK_DATA_DIR}; use lazy_static::lazy_static; use regex::{Regex, RegexSet}; use serde::Deserialize; @@ -210,7 +211,7 @@ impl TomlFilterRegistry { // Priority 2: user-global ~/.config/rtk/filters.toml if let Some(config_dir) = dirs::config_dir() { - let global_path = config_dir.join("rtk").join("filters.toml"); + let global_path = config_dir.join(RTK_DATA_DIR).join(FILTERS_TOML); if let Ok(content) = std::fs::read_to_string(&global_path) { match Self::parse_and_compile(&content, "user-global") { Ok(f) => filters.extend(f), diff --git a/src/core/tracking.rs b/src/core/tracking.rs index 233f0e7c..d6a248af 100644 --- a/src/core/tracking.rs +++ b/src/core/tracking.rs @@ -61,8 +61,7 @@ fn project_filter_params(project_path: Option<&str>) -> (Option, Option< } } -/// Number of days to retain tracking history before automatic cleanup. -const HISTORY_DAYS: i64 = 90; +use super::constants::{DEFAULT_HISTORY_DAYS, HISTORY_DB, RTK_DATA_DIR}; /// Main tracking interface for recording and querying command history. /// @@ -387,7 +386,7 @@ impl Tracker { } fn cleanup_old(&self) -> Result<()> { - let cutoff = Utc::now() - chrono::Duration::days(HISTORY_DAYS); + let cutoff = Utc::now() - chrono::Duration::days(DEFAULT_HISTORY_DAYS); self.conn.execute( "DELETE FROM commands WHERE timestamp < ?1", params![cutoff.to_rfc3339()], @@ -974,7 +973,7 @@ fn get_db_path() -> Result { // Priority 3: Default platform-specific location let data_dir = dirs::data_local_dir().unwrap_or_else(|| PathBuf::from(".")); - Ok(data_dir.join("rtk").join("history.db")) + Ok(data_dir.join(RTK_DATA_DIR).join(HISTORY_DB)) } /// Individual parse failure record. diff --git a/src/discover/provider.rs b/src/discover/provider.rs index 286c3891..9bd16f8a 100644 --- a/src/discover/provider.rs +++ b/src/discover/provider.rs @@ -1,5 +1,6 @@ //! Reads Claude Code session logs from disk and streams their command history. +use crate::hooks::constants::CLAUDE_DIR; use anyhow::{Context, Result}; use std::collections::HashMap; use std::fs; @@ -44,7 +45,7 @@ impl ClaudeProvider { /// Get the base directory for Claude Code projects. fn projects_dir() -> Result { let home = dirs::home_dir().context("could not determine home directory")?; - let dir = home.join(".claude").join("projects"); + let dir = home.join(CLAUDE_DIR).join("projects"); if !dir.exists() { anyhow::bail!( "Claude Code projects directory not found: {}\nMake sure Claude Code has been used at least once.", diff --git a/src/discover/report.rs b/src/discover/report.rs index 1fffa327..652bb348 100644 --- a/src/discover/report.rs +++ b/src/discover/report.rs @@ -1,5 +1,6 @@ //! Data types for reporting which commands RTK can and cannot optimize. +use crate::hooks::constants::{HOOKS_SUBDIR, REWRITE_HOOK_FILE}; use serde::Serialize; /// RTK support status for a command. @@ -169,7 +170,10 @@ pub fn format_text(report: &DiscoverReport, limit: usize, verbose: bool) -> Stri // Cursor note: check if Cursor hooks are installed if let Some(home) = dirs::home_dir() { - let cursor_hook = home.join(".cursor").join("hooks").join("rtk-rewrite.sh"); + let cursor_hook = home + .join(".cursor") + .join(HOOKS_SUBDIR) + .join(REWRITE_HOOK_FILE); if cursor_hook.exists() { out.push_str("\nNote: Cursor sessions are tracked via `rtk gain` (discover scans Claude Code only)\n"); } diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs new file mode 100644 index 00000000..b2b1d16e --- /dev/null +++ b/src/hooks/constants.rs @@ -0,0 +1,9 @@ +pub const REWRITE_HOOK_FILE: &str = "rtk-rewrite.sh"; +pub const GEMINI_HOOK_FILE: &str = "rtk-hook-gemini.sh"; +pub const CLAUDE_DIR: &str = ".claude"; +pub const HOOKS_SUBDIR: &str = "hooks"; +pub const SETTINGS_JSON: &str = "settings.json"; +pub const SETTINGS_LOCAL_JSON: &str = "settings.local.json"; +pub const HOOKS_JSON: &str = "hooks.json"; +pub const PRE_TOOL_USE_KEY: &str = "PreToolUse"; +pub const BEFORE_TOOL_KEY: &str = "BeforeTool"; diff --git a/src/hooks/hook_check.rs b/src/hooks/hook_check.rs index 9fb0b337..ea3a6f41 100644 --- a/src/hooks/hook_check.rs +++ b/src/hooks/hook_check.rs @@ -1,5 +1,7 @@ //! Detects whether RTK hooks are installed and warns if they are outdated. +use super::constants::{CLAUDE_DIR, HOOKS_SUBDIR, REWRITE_HOOK_FILE}; +use crate::core::constants::RTK_DATA_DIR; use std::path::PathBuf; const CURRENT_HOOK_VERSION: u8 = 3; @@ -24,7 +26,7 @@ pub fn status() -> HookStatus { Some(h) => h, None => return HookStatus::Ok, }; - if !home.join(".claude").exists() { + if !home.join(CLAUDE_DIR).exists() { return HookStatus::Ok; } @@ -90,7 +92,10 @@ pub fn parse_hook_version(content: &str) -> u8 { fn hook_installed_path() -> Option { let home = dirs::home_dir()?; - let path = home.join(".claude").join("hooks").join("rtk-rewrite.sh"); + let path = home + .join(CLAUDE_DIR) + .join(HOOKS_SUBDIR) + .join(REWRITE_HOOK_FILE); if path.exists() { Some(path) } else { @@ -99,7 +104,7 @@ fn hook_installed_path() -> Option { } fn warn_marker_path() -> Option { - let data_dir = dirs::data_local_dir()?.join("rtk"); + let data_dir = dirs::data_local_dir()?.join(RTK_DATA_DIR); Some(data_dir.join(".hook_warn_last")) } diff --git a/src/hooks/hook_cmd.rs b/src/hooks/hook_cmd.rs index 8eb4e2fa..5d72c0b3 100644 --- a/src/hooks/hook_cmd.rs +++ b/src/hooks/hook_cmd.rs @@ -1,5 +1,6 @@ //! Processes incoming hook calls from AI agents and rewrites commands on the fly. +use super::constants::PRE_TOOL_USE_KEY; use anyhow::{Context, Result}; use serde_json::{json, Value}; use std::io::{self, Read}; @@ -112,7 +113,7 @@ fn handle_vscode(cmd: &str) -> Result<()> { let output = json!({ "hookSpecificOutput": { - "hookEventName": "PreToolUse", + "hookEventName": PRE_TOOL_USE_KEY, "permissionDecision": "allow", "permissionDecisionReason": "RTK auto-rewrite", "updatedInput": { "command": rewritten } diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 438aca7a..47faf116 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -6,6 +6,10 @@ use std::io::Write; use std::path::{Path, PathBuf}; use tempfile::NamedTempFile; +use super::constants::{ + BEFORE_TOOL_KEY, CLAUDE_DIR, GEMINI_HOOK_FILE, HOOKS_JSON, HOOKS_SUBDIR, PRE_TOOL_USE_KEY, + REWRITE_HOOK_FILE, SETTINGS_JSON, +}; use super::integrity; use crate::core::config; @@ -53,6 +57,12 @@ schema_version = 1 # max_lines = 40 "#; +const RTK_MD: &str = "RTK.md"; +const CLAUDE_MD: &str = "CLAUDE.md"; +const AGENTS_MD: &str = "AGENTS.md"; +const RTK_MD_REF: &str = "@RTK.md"; +const GEMINI_MD: &str = "GEMINI.md"; + /// Control flow for settings.json patching #[derive(Debug, Clone, Copy, PartialEq)] pub enum PatchMode { @@ -301,7 +311,7 @@ fn prepare_hook_paths() -> Result<(PathBuf, PathBuf)> { let hook_dir = claude_dir.join("hooks"); fs::create_dir_all(&hook_dir) .with_context(|| format!("Failed to create hook directory: {}", hook_dir.display()))?; - let hook_path = hook_dir.join("rtk-rewrite.sh"); + let hook_path = hook_dir.join(REWRITE_HOOK_FILE); Ok((hook_dir, hook_path)) } @@ -456,7 +466,10 @@ fn print_manual_instructions(hook_path: &Path, include_opencode: bool) { /// Remove RTK hook entry from settings.json /// Returns true if hook was found and removed fn remove_hook_from_json(root: &mut serde_json::Value) -> bool { - let hooks = match root.get_mut("hooks").and_then(|h| h.get_mut("PreToolUse")) { + let hooks = match root + .get_mut("hooks") + .and_then(|h| h.get_mut(PRE_TOOL_USE_KEY)) + { Some(pre_tool_use) => pre_tool_use, None => return false, }; @@ -472,7 +485,7 @@ fn remove_hook_from_json(root: &mut serde_json::Value) -> bool { if let Some(hooks_array) = entry.get("hooks").and_then(|h| h.as_array()) { for hook in hooks_array { if let Some(command) = hook.get("command").and_then(|c| c.as_str()) { - if command.contains("rtk-rewrite.sh") { + if command.contains(REWRITE_HOOK_FILE) { return false; // Remove this entry } } @@ -488,7 +501,7 @@ fn remove_hook_from_json(root: &mut serde_json::Value) -> bool { /// Backs up before modification, returns true if hook was found and removed fn remove_hook_from_settings(verbose: u8) -> Result { let claude_dir = resolve_claude_dir()?; - let settings_path = claude_dir.join("settings.json"); + let settings_path = claude_dir.join(SETTINGS_JSON); if !settings_path.exists() { if verbose > 0 { @@ -576,7 +589,7 @@ pub fn uninstall(global: bool, gemini: bool, codex: bool, cursor: bool, verbose: } // 1. Remove hook file - let hook_path = claude_dir.join("hooks").join("rtk-rewrite.sh"); + let hook_path = claude_dir.join(HOOKS_SUBDIR).join(REWRITE_HOOK_FILE); if hook_path.exists() { fs::remove_file(&hook_path) .with_context(|| format!("Failed to remove hook: {}", hook_path.display()))?; @@ -589,7 +602,7 @@ pub fn uninstall(global: bool, gemini: bool, codex: bool, cursor: bool, verbose: } // 2. Remove RTK.md - let rtk_md_path = claude_dir.join("RTK.md"); + let rtk_md_path = claude_dir.join(RTK_MD); if rtk_md_path.exists() { fs::remove_file(&rtk_md_path) .with_context(|| format!("Failed to remove RTK.md: {}", rtk_md_path.display()))?; @@ -597,15 +610,15 @@ pub fn uninstall(global: bool, gemini: bool, codex: bool, cursor: bool, verbose: } // 3. Remove @RTK.md reference from CLAUDE.md - let claude_md_path = claude_dir.join("CLAUDE.md"); + let claude_md_path = claude_dir.join(CLAUDE_MD); if claude_md_path.exists() { let content = fs::read_to_string(&claude_md_path) .with_context(|| format!("Failed to read CLAUDE.md: {}", claude_md_path.display()))?; - if content.contains("@RTK.md") { + if content.contains(RTK_MD_REF) { let new_content = content .lines() - .filter(|line| !line.trim().starts_with("@RTK.md")) + .filter(|line| !line.trim().starts_with(RTK_MD_REF)) .collect::>() .join("\n"); @@ -673,7 +686,7 @@ fn uninstall_codex(global: bool, verbose: u8) -> Result<()> { fn uninstall_codex_at(codex_dir: &Path, verbose: u8) -> Result> { let mut removed = Vec::new(); - let rtk_md_path = codex_dir.join("RTK.md"); + let rtk_md_path = codex_dir.join(RTK_MD); if rtk_md_path.exists() { fs::remove_file(&rtk_md_path) .with_context(|| format!("Failed to remove RTK.md: {}", rtk_md_path.display()))?; @@ -683,7 +696,7 @@ fn uninstall_codex_at(codex_dir: &Path, verbose: u8) -> Result> { removed.push(format!("RTK.md: {}", rtk_md_path.display())); } - let agents_md_path = codex_dir.join("AGENTS.md"); + let agents_md_path = codex_dir.join(AGENTS_MD); if remove_rtk_reference_from_agents(&agents_md_path, verbose)? { removed.push("AGENTS.md: removed @RTK.md reference".to_string()); } @@ -700,7 +713,7 @@ fn patch_settings_json( include_opencode: bool, ) -> Result { let claude_dir = resolve_claude_dir()?; - let settings_path = claude_dir.join("settings.json"); + let settings_path = claude_dir.join(SETTINGS_JSON); let hook_command = hook_path .to_str() .context("Hook path contains invalid UTF-8")?; @@ -830,7 +843,7 @@ fn insert_hook_entry(root: &mut serde_json::Value, hook_command: &str) { .expect("hooks must be an object"); let pre_tool_use = hooks - .entry("PreToolUse") + .entry(PRE_TOOL_USE_KEY) .or_insert_with(|| serde_json::json!([])) .as_array_mut() .expect("PreToolUse must be an array"); @@ -850,7 +863,7 @@ fn insert_hook_entry(root: &mut serde_json::Value, hook_command: &str) { fn hook_already_present(root: &serde_json::Value, hook_command: &str) -> bool { let pre_tool_use_array = match root .get("hooks") - .and_then(|h| h.get("PreToolUse")) + .and_then(|h| h.get(PRE_TOOL_USE_KEY)) .and_then(|p| p.as_array()) { Some(arr) => arr, @@ -865,7 +878,7 @@ fn hook_already_present(root: &serde_json::Value, hook_command: &str) -> bool { .any(|cmd| { // Exact match OR both contain rtk-rewrite.sh cmd == hook_command - || (cmd.contains("rtk-rewrite.sh") && hook_command.contains("rtk-rewrite.sh")) + || (cmd.contains(REWRITE_HOOK_FILE) && hook_command.contains(REWRITE_HOOK_FILE)) }) } @@ -898,15 +911,15 @@ fn run_default_mode( } let claude_dir = resolve_claude_dir()?; - let rtk_md_path = claude_dir.join("RTK.md"); - let claude_md_path = claude_dir.join("CLAUDE.md"); + let rtk_md_path = claude_dir.join(RTK_MD); + let claude_md_path = claude_dir.join(CLAUDE_MD); // 1. Prepare hook directory and install hook let (_hook_dir, hook_path) = prepare_hook_paths()?; let hook_changed = ensure_hook_installed(&hook_path, verbose)?; // 2. Write RTK.md - write_if_changed(&rtk_md_path, RTK_SLIM, "RTK.md", verbose)?; + write_if_changed(&rtk_md_path, RTK_SLIM, RTK_MD, verbose)?; let opencode_plugin_path = if install_opencode { let path = prepare_opencode_plugin_path()?; @@ -994,7 +1007,7 @@ fn generate_project_filters_template(verbose: u8) -> Result<()> { /// Generate ~/.config/rtk/filters.toml template if not present. fn generate_global_filters_template(verbose: u8) -> Result<()> { let config_dir = dirs::config_dir().unwrap_or_else(|| std::path::PathBuf::from(".config")); - let rtk_dir = config_dir.join("rtk"); + let rtk_dir = config_dir.join(crate::core::constants::RTK_DATA_DIR); let path = rtk_dir.join("filters.toml"); if path.exists() { @@ -1095,9 +1108,9 @@ fn run_hook_only_mode( /// Legacy mode: full 137-line injection into CLAUDE.md fn run_claude_md_mode(global: bool, verbose: u8, install_opencode: bool) -> Result<()> { let path = if global { - resolve_claude_dir()?.join("CLAUDE.md") + resolve_claude_dir()?.join(CLAUDE_MD) } else { - PathBuf::from("CLAUDE.md") + PathBuf::from(CLAUDE_MD) }; if global { @@ -1248,9 +1261,9 @@ fn run_windsurf_mode(verbose: u8) -> Result<()> { fn run_codex_mode(global: bool, verbose: u8) -> Result<()> { let (agents_md_path, rtk_md_path) = if global { let codex_dir = resolve_codex_dir()?; - (codex_dir.join("AGENTS.md"), codex_dir.join("RTK.md")) + (codex_dir.join(AGENTS_MD), codex_dir.join(RTK_MD)) } else { - (PathBuf::from("AGENTS.md"), PathBuf::from("RTK.md")) + (PathBuf::from(AGENTS_MD), PathBuf::from(RTK_MD)) }; if global { @@ -1264,7 +1277,7 @@ fn run_codex_mode(global: bool, verbose: u8) -> Result<()> { } } - write_if_changed(&rtk_md_path, RTK_SLIM_CODEX, "RTK.md", verbose)?; + write_if_changed(&rtk_md_path, RTK_SLIM_CODEX, RTK_MD, verbose)?; let added_ref = patch_agents_md(&agents_md_path, verbose)?; println!("\nRTK configured for Codex CLI.\n"); @@ -1375,7 +1388,7 @@ fn patch_claude_md(path: &Path, verbose: u8) -> Result { } // Check if @RTK.md already present - if content.contains("@RTK.md") { + if content.contains(RTK_MD_REF) { if verbose > 0 { eprintln!("@RTK.md reference already present in CLAUDE.md"); } @@ -1422,7 +1435,7 @@ fn patch_agents_md(path: &Path, verbose: u8) -> Result { } } - if content.contains("@RTK.md") { + if content.contains(RTK_MD_REF) { if verbose > 0 { eprintln!("@RTK.md reference already present in AGENTS.md"); } @@ -1455,13 +1468,13 @@ fn remove_rtk_reference_from_agents(path: &Path, verbose: u8) -> Result { let content = fs::read_to_string(path) .with_context(|| format!("Failed to read AGENTS.md: {}", path.display()))?; - if !content.contains("@RTK.md") { + if !content.contains(RTK_MD_REF) { return Ok(false); } let new_content = content .lines() - .filter(|line| !line.trim().starts_with("@RTK.md")) + .filter(|line| !line.trim().starts_with(RTK_MD_REF)) .collect::>() .join("\n"); let cleaned = clean_double_blanks(&new_content); @@ -1519,7 +1532,7 @@ fn remove_rtk_block(content: &str) -> (String, bool) { /// Resolve ~/.claude directory with proper home expansion fn resolve_claude_dir() -> Result { dirs::home_dir() - .map(|h| h.join(".claude")) + .map(|h| h.join(CLAUDE_DIR)) .context("Cannot determine home directory. Is $HOME set?") } @@ -1602,7 +1615,7 @@ fn install_cursor_hooks(verbose: u8) -> Result<()> { })?; // 1. Write hook script - let hook_path = hooks_dir.join("rtk-rewrite.sh"); + let hook_path = hooks_dir.join(REWRITE_HOOK_FILE); let hook_changed = write_if_changed(&hook_path, CURSOR_REWRITE_HOOK, "Cursor hook", verbose)?; #[cfg(unix)] @@ -1617,7 +1630,7 @@ fn install_cursor_hooks(verbose: u8) -> Result<()> { } // 2. Create or patch hooks.json - let hooks_json_path = cursor_dir.join("hooks.json"); + let hooks_json_path = cursor_dir.join(HOOKS_JSON); let patched = patch_cursor_hooks_json(&hooks_json_path, verbose)?; // Report @@ -1701,7 +1714,7 @@ fn cursor_hook_already_present(root: &serde_json::Value) -> bool { entry .get("command") .and_then(|c| c.as_str()) - .is_some_and(|cmd| cmd.contains("rtk-rewrite.sh")) + .is_some_and(|cmd| cmd.contains(REWRITE_HOOK_FILE)) }) } @@ -1743,7 +1756,7 @@ fn remove_cursor_hooks(verbose: u8) -> Result> { let mut removed = Vec::new(); // 1. Remove hook script - let hook_path = cursor_dir.join("hooks").join("rtk-rewrite.sh"); + let hook_path = cursor_dir.join(HOOKS_SUBDIR).join(REWRITE_HOOK_FILE); if hook_path.exists() { fs::remove_file(&hook_path) .with_context(|| format!("Failed to remove Cursor hook: {}", hook_path.display()))?; @@ -1751,7 +1764,7 @@ fn remove_cursor_hooks(verbose: u8) -> Result> { } // 2. Remove RTK entry from hooks.json - let hooks_json_path = cursor_dir.join("hooks.json"); + let hooks_json_path = cursor_dir.join(HOOKS_JSON); if hooks_json_path.exists() { let content = fs::read_to_string(&hooks_json_path) .with_context(|| format!("Failed to read {}", hooks_json_path.display()))?; @@ -1796,7 +1809,7 @@ fn remove_cursor_hook_from_json(root: &mut serde_json::Value) -> bool { !entry .get("command") .and_then(|c| c.as_str()) - .is_some_and(|cmd| cmd.contains("rtk-rewrite.sh")) + .is_some_and(|cmd| cmd.contains(REWRITE_HOOK_FILE)) }); pre_tool_use.len() < original_len @@ -1813,10 +1826,10 @@ pub fn show_config(codex: bool) -> Result<()> { fn show_claude_config() -> Result<()> { let claude_dir = resolve_claude_dir()?; - let hook_path = claude_dir.join("hooks").join("rtk-rewrite.sh"); - let rtk_md_path = claude_dir.join("RTK.md"); - let global_claude_md = claude_dir.join("CLAUDE.md"); - let local_claude_md = PathBuf::from("CLAUDE.md"); + let hook_path = claude_dir.join(HOOKS_SUBDIR).join(REWRITE_HOOK_FILE); + let rtk_md_path = claude_dir.join(RTK_MD); + let global_claude_md = claude_dir.join(CLAUDE_MD); + let local_claude_md = PathBuf::from(CLAUDE_MD); println!("rtk Configuration:\n"); @@ -1900,7 +1913,7 @@ fn show_claude_config() -> Result<()> { // Check global CLAUDE.md if global_claude_md.exists() { let content = fs::read_to_string(&global_claude_md)?; - if content.contains("@RTK.md") { + if content.contains(RTK_MD_REF) { println!("[ok] Global (~/.claude/CLAUDE.md): @RTK.md reference"); } else if content.contains("").unwrap(); @@ -160,7 +161,24 @@ fn extract_identifier_and_extra_args(args: &[String]) -> Option<(String, Vec(cmd: Command, label: &str, filter_fn: F) -> Result +where + F: Fn(&Value) -> String, +{ + runner::run_filtered( + cmd, + "gh", + label, + |stdout| match serde_json::from_str::(stdout) { + Ok(json) => filter_fn(&json), + Err(_) => stdout.to_string(), + }, + RunOptions::stdout_only() + .early_exit_on_failure() + .no_trailing_newline(), + ) +} + pub fn run(subcommand: &str, args: &[String], verbose: u8, ultra_compact: bool) -> Result { // When user explicitly passes --json, they want raw gh JSON output, not RTK filtering if has_json_flag(args) { @@ -200,8 +218,6 @@ fn run_pr(args: &[String], verbose: u8, ultra_compact: bool) -> Result { } fn list_prs(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { - let timer = tracking::TimedExecution::start(); - let mut cmd = resolved_command("gh"); cmd.args([ "pr", @@ -209,78 +225,62 @@ fn list_prs(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { "--json", "number,title,state,author,updatedAt", ]); - - // Pass through additional flags for arg in args { cmd.arg(arg); } + run_gh_json(cmd, "pr list", |json| format_pr_list(json, ultra_compact)) +} - let output = cmd.output().context("Failed to run gh pr list")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track("gh pr list", "rtk gh pr list", &stderr, &stderr); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let json: Value = - serde_json::from_slice(&output.stdout).context("Failed to parse gh pr list output")?; - - let mut filtered = String::new(); - - if let Some(prs) = json.as_array() { - if ultra_compact { - filtered.push_str("PRs\n"); - println!("PRs"); - } else { - filtered.push_str("Pull Requests\n"); - println!("Pull Requests"); - } - - for pr in prs.iter().take(20) { - let number = pr["number"].as_i64().unwrap_or(0); - let title = pr["title"].as_str().unwrap_or("???"); - let state = pr["state"].as_str().unwrap_or("???"); - let author = pr["author"]["login"].as_str().unwrap_or("???"); - - let state_icon = if ultra_compact { - match state { - "OPEN" => "O", - "MERGED" => "M", - "CLOSED" => "C", - _ => "?", - } - } else { - match state { - "OPEN" => "[open]", - "MERGED" => "[merged]", - "CLOSED" => "[closed]", - _ => "[unknown]", - } - }; +fn format_pr_list(json: &Value, ultra_compact: bool) -> String { + let prs = match json.as_array() { + Some(prs) => prs, + None => return String::new(), + }; + let mut out = String::new(); + out.push_str(if ultra_compact { + "PRs\n" + } else { + "Pull Requests\n" + }); + for pr in prs.iter().take(20) { + let number = pr["number"].as_i64().unwrap_or(0); + let title = pr["title"].as_str().unwrap_or("???"); + let state = pr["state"].as_str().unwrap_or("???"); + let author = pr["author"]["login"].as_str().unwrap_or("???"); + let icon = state_icon(state, ultra_compact); + out.push_str(&format!( + " {} #{} {} ({})\n", + icon, + number, + truncate(title, 60), + author + )); + } + if prs.len() > 20 { + out.push_str(&format!( + " ... {} more (use gh pr list for all)\n", + prs.len() - 20 + )); + } + out +} - let line = format!( - " {} #{} {} ({})\n", - state_icon, - number, - truncate(title, 60), - author - ); - filtered.push_str(&line); - print!("{}", line); +fn state_icon(state: &str, ultra_compact: bool) -> &'static str { + if ultra_compact { + match state { + "OPEN" => "O", + "MERGED" => "M", + "CLOSED" => "C", + _ => "?", } - - if prs.len() > 20 { - let more_line = format!(" ... {} more (use gh pr list for all)\n", prs.len() - 20); - filtered.push_str(&more_line); - print!("{}", more_line); + } else { + match state { + "OPEN" => "[open]", + "MERGED" => "[merged]", + "CLOSED" => "[closed]", + _ => "[unknown]", } } - - timer.track("gh pr list", "rtk gh pr list", &raw, &filtered); - Ok(0) } fn should_passthrough_pr_view(extra_args: &[String]) -> bool { @@ -296,19 +296,13 @@ fn should_passthrough_issue_view(extra_args: &[String]) -> bool { } fn view_pr(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { - let timer = tracking::TimedExecution::start(); - let (pr_number, extra_args) = match extract_identifier_and_extra_args(args) { Some(result) => result, None => return Err(anyhow::anyhow!("PR number required")), }; - - // If the user provides --jq or --web, pass through directly. - // Note: --json is already handled globally by run() via has_json_flag. if should_passthrough_pr_view(&extra_args) { return run_passthrough_with_extra("gh", &["pr", "view", &pr_number], &extra_args); } - let mut cmd = resolved_command("gh"); cmd.args([ "pr", @@ -320,28 +314,13 @@ fn view_pr(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { for arg in &extra_args { cmd.arg(arg); } + run_gh_json(cmd, &format!("pr view {}", pr_number), |json| { + format_pr_view(json, ultra_compact) + }) +} - let output = cmd.output().context("Failed to run gh pr view")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track( - &format!("gh pr view {}", pr_number), - &format!("rtk gh pr view {}", pr_number), - &stderr, - &stderr, - ); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let json: Value = - serde_json::from_slice(&output.stdout).context("Failed to parse gh pr view output")?; - - let mut filtered = String::new(); - - // Extract essential info +fn format_pr_view(json: &Value, ultra_compact: bool) -> String { + let mut out = String::new(); let number = json["number"].as_i64().unwrap_or(0); let title = json["title"].as_str().unwrap_or("???"); let state = json["state"].as_str().unwrap_or("???"); @@ -349,40 +328,17 @@ fn view_pr(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { let url = json["url"].as_str().unwrap_or(""); let mergeable = json["mergeable"].as_str().unwrap_or("UNKNOWN"); - let state_icon = if ultra_compact { - match state { - "OPEN" => "O", - "MERGED" => "M", - "CLOSED" => "C", - _ => "?", - } - } else { - match state { - "OPEN" => "[open]", - "MERGED" => "[merged]", - "CLOSED" => "[closed]", - _ => "[unknown]", - } - }; - - let line = format!("{} PR #{}: {}\n", state_icon, number, title); - filtered.push_str(&line); - print!("{}", line); - - let line = format!(" {}\n", author); - filtered.push_str(&line); - print!("{}", line); + let icon = state_icon(state, ultra_compact); + out.push_str(&format!("{} PR #{}: {}\n", icon, number, title)); + out.push_str(&format!(" {}\n", author)); let mergeable_str = match mergeable { "MERGEABLE" => "[ok]", "CONFLICTING" => "[x]", _ => "?", }; - let line = format!(" {} | {}\n", state, mergeable_str); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" {} | {}\n", state, mergeable_str)); - // Show reviews summary if let Some(reviews) = json["reviews"]["nodes"].as_array() { let approved = reviews .iter() @@ -392,18 +348,14 @@ fn view_pr(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { .iter() .filter(|r| r["state"].as_str() == Some("CHANGES_REQUESTED")) .count(); - if approved > 0 || changes > 0 { - let line = format!( + out.push_str(&format!( " Reviews: {} approved, {} changes requested\n", approved, changes - ); - filtered.push_str(&line); - print!("{}", line); + )); } } - // Show checks summary if let Some(checks) = json["statusCheckRollup"].as_array() { let total = checks.len(); let passed = checks @@ -420,90 +372,59 @@ fn view_pr(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { || c["state"].as_str() == Some("FAILURE") }) .count(); - if ultra_compact { if failed > 0 { - let line = format!(" [x]{}/{} {} fail\n", passed, total, failed); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" [x]{}/{} {} fail\n", passed, total, failed)); } else { - let line = format!(" {}/{}\n", passed, total); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" {}/{}\n", passed, total)); } } else { - let line = format!(" Checks: {}/{} passed\n", passed, total); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" Checks: {}/{} passed\n", passed, total)); if failed > 0 { - let line = format!(" [warn] {} checks failed\n", failed); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" [warn] {} checks failed\n", failed)); } } } - let line = format!(" {}\n", url); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" {}\n", url)); - // Show filtered body if let Some(body) = json["body"].as_str() { if !body.is_empty() { let body_filtered = filter_markdown_body(body); if !body_filtered.is_empty() { - filtered.push('\n'); - println!(); + out.push('\n'); for line in body_filtered.lines() { - let formatted = format!(" {}\n", line); - filtered.push_str(&formatted); - print!("{}", formatted); + out.push_str(&format!(" {}\n", line)); } } } } - timer.track( - &format!("gh pr view {}", pr_number), - &format!("rtk gh pr view {}", pr_number), - &raw, - &filtered, - ); - Ok(0) + out } fn pr_checks(args: &[String], _verbose: u8, _ultra_compact: bool) -> Result { - let timer = tracking::TimedExecution::start(); - let (pr_number, extra_args) = match extract_identifier_and_extra_args(args) { Some(result) => result, None => return Err(anyhow::anyhow!("PR number required")), }; - let mut cmd = resolved_command("gh"); cmd.args(["pr", "checks", &pr_number]); for arg in &extra_args { cmd.arg(arg); } + runner::run_filtered( + cmd, + "gh", + &format!("pr checks {}", pr_number), + format_pr_checks, + RunOptions::stdout_only() + .early_exit_on_failure() + .no_trailing_newline(), + ) +} - let output = cmd.output().context("Failed to run gh pr checks")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track( - &format!("gh pr checks {}", pr_number), - &format!("rtk gh pr checks {}", pr_number), - &stderr, - &stderr, - ); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let stdout = String::from_utf8_lossy(&output.stdout); - - // Parse and compress checks output +fn format_pr_checks(stdout: &str) -> String { let mut passed = 0; let mut failed = 0; let mut pending = 0; @@ -520,49 +441,23 @@ fn pr_checks(args: &[String], _verbose: u8, _ultra_compact: bool) -> Result } } - let mut filtered = String::new(); - - let line = "CI Checks Summary:\n"; - filtered.push_str(line); - print!("{}", line); - - let line = format!(" [ok] Passed: {}\n", passed); - filtered.push_str(&line); - print!("{}", line); - - let line = format!(" [FAIL] Failed: {}\n", failed); - filtered.push_str(&line); - print!("{}", line); - + let mut out = String::new(); + out.push_str("CI Checks Summary:\n"); + out.push_str(&format!(" [ok] Passed: {}\n", passed)); + out.push_str(&format!(" [FAIL] Failed: {}\n", failed)); if pending > 0 { - let line = format!(" [pending] Pending: {}\n", pending); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" [pending] Pending: {}\n", pending)); } - if !failed_checks.is_empty() { - let line = "\n Failed checks:\n"; - filtered.push_str(line); - print!("{}", line); + out.push_str("\n Failed checks:\n"); for check in failed_checks { - let line = format!(" {}\n", check); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" {}\n", check)); } } - - timer.track( - &format!("gh pr checks {}", pr_number), - &format!("rtk gh pr checks {}", pr_number), - &raw, - &filtered, - ); - Ok(0) + out } fn pr_status(_verbose: u8, _ultra_compact: bool) -> Result { - let timer = tracking::TimedExecution::start(); - let mut cmd = resolved_command("gh"); cmd.args([ "pr", @@ -570,38 +465,26 @@ fn pr_status(_verbose: u8, _ultra_compact: bool) -> Result { "--json", "currentBranch,createdBy,reviewDecision,statusCheckRollup", ]); + run_gh_json(cmd, "pr status", format_pr_status) +} - let output = cmd.output().context("Failed to run gh pr status")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track("gh pr status", "rtk gh pr status", &stderr, &stderr); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let json: Value = - serde_json::from_slice(&output.stdout).context("Failed to parse gh pr status output")?; - - let mut filtered = String::new(); - +fn format_pr_status(json: &Value) -> String { + let mut out = String::new(); if let Some(created_by) = json["createdBy"].as_array() { - let line = format!("Your PRs ({}):\n", created_by.len()); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!("Your PRs ({}):\n", created_by.len())); for pr in created_by.iter().take(5) { let number = pr["number"].as_i64().unwrap_or(0); let title = pr["title"].as_str().unwrap_or("???"); let reviews = pr["reviewDecision"].as_str().unwrap_or("PENDING"); - let line = format!(" #{} {} [{}]\n", number, truncate(title, 50), reviews); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!( + " #{} {} [{}]\n", + number, + truncate(title, 50), + reviews + )); } } - - timer.track("gh pr status", "rtk gh pr status", &raw, &filtered); - Ok(0) + out } fn run_issue(args: &[String], verbose: u8, ultra_compact: bool) -> Result { @@ -617,82 +500,54 @@ fn run_issue(args: &[String], verbose: u8, ultra_compact: bool) -> Result { } fn list_issues(args: &[String], _verbose: u8, ultra_compact: bool) -> Result { - let timer = tracking::TimedExecution::start(); - let mut cmd = resolved_command("gh"); cmd.args(["issue", "list", "--json", "number,title,state,author"]); - for arg in args { cmd.arg(arg); } + run_gh_json(cmd, "issue list", |json| { + format_issue_list(json, ultra_compact) + }) +} - let output = cmd.output().context("Failed to run gh issue list")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track("gh issue list", "rtk gh issue list", &stderr, &stderr); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let json: Value = - serde_json::from_slice(&output.stdout).context("Failed to parse gh issue list output")?; - - let mut filtered = String::new(); - - if let Some(issues) = json.as_array() { - filtered.push_str("Issues\n"); - println!("Issues"); - for issue in issues.iter().take(20) { - let number = issue["number"].as_i64().unwrap_or(0); - let title = issue["title"].as_str().unwrap_or("???"); - let state = issue["state"].as_str().unwrap_or("???"); - - let icon = if ultra_compact { - if state == "OPEN" { - "O" - } else { - "C" - } +fn format_issue_list(json: &Value, ultra_compact: bool) -> String { + let issues = match json.as_array() { + Some(issues) => issues, + None => return String::new(), + }; + let mut out = String::new(); + out.push_str("Issues\n"); + for issue in issues.iter().take(20) { + let number = issue["number"].as_i64().unwrap_or(0); + let title = issue["title"].as_str().unwrap_or("???"); + let state = issue["state"].as_str().unwrap_or("???"); + let icon = if ultra_compact { + if state == "OPEN" { + "O" } else { - if state == "OPEN" { - "[open]" - } else { - "[closed]" - } - }; - let line = format!(" {} #{} {}\n", icon, number, truncate(title, 60)); - filtered.push_str(&line); - print!("{}", line); - } - - if issues.len() > 20 { - let line = format!(" ... {} more\n", issues.len() - 20); - filtered.push_str(&line); - print!("{}", line); - } + "C" + } + } else if state == "OPEN" { + "[open]" + } else { + "[closed]" + }; + out.push_str(&format!(" {} #{} {}\n", icon, number, truncate(title, 60))); } - - timer.track("gh issue list", "rtk gh issue list", &raw, &filtered); - Ok(0) + if issues.len() > 20 { + out.push_str(&format!(" ... {} more\n", issues.len() - 20)); + } + out } fn view_issue(args: &[String], _verbose: u8) -> Result { - let timer = tracking::TimedExecution::start(); - let (issue_number, extra_args) = match extract_identifier_and_extra_args(args) { Some(result) => result, None => return Err(anyhow::anyhow!("Issue number required")), }; - - // Passthrough when --comments, --json, --jq, or --web is present. - // --comments changes the output to include comments which our JSON - // field list doesn't request, causing silent data loss. if should_passthrough_issue_view(&extra_args) { return run_passthrough_with_extra("gh", &["issue", "view", &issue_number], &extra_args); } - let mut cmd = resolved_command("gh"); cmd.args([ "issue", @@ -704,25 +559,13 @@ fn view_issue(args: &[String], _verbose: u8) -> Result { for arg in &extra_args { cmd.arg(arg); } + run_gh_json(cmd, &format!("issue view {}", issue_number), |json| { + format_issue_view(json) + }) +} - let output = cmd.output().context("Failed to run gh issue view")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track( - &format!("gh issue view {}", issue_number), - &format!("rtk gh issue view {}", issue_number), - &stderr, - &stderr, - ); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let json: Value = - serde_json::from_slice(&output.stdout).context("Failed to parse gh issue view output")?; - +fn format_issue_view(json: &Value) -> String { + let mut out = String::new(); let number = json["number"].as_i64().unwrap_or(0); let title = json["title"].as_str().unwrap_or("???"); let state = json["state"].as_str().unwrap_or("???"); @@ -734,48 +577,23 @@ fn view_issue(args: &[String], _verbose: u8) -> Result { } else { "[closed]" }; - - let mut filtered = String::new(); - - let line = format!("{} Issue #{}: {}\n", icon, number, title); - filtered.push_str(&line); - print!("{}", line); - - let line = format!(" Author: @{}\n", author); - filtered.push_str(&line); - print!("{}", line); - - let line = format!(" Status: {}\n", state); - filtered.push_str(&line); - print!("{}", line); - - let line = format!(" URL: {}\n", url); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!("{} Issue #{}: {}\n", icon, number, title)); + out.push_str(&format!(" Author: @{}\n", author)); + out.push_str(&format!(" Status: {}\n", state)); + out.push_str(&format!(" URL: {}\n", url)); if let Some(body) = json["body"].as_str() { if !body.is_empty() { let body_filtered = filter_markdown_body(body); if !body_filtered.is_empty() { - let line = "\n Description:\n"; - filtered.push_str(line); - print!("{}", line); + out.push_str("\n Description:\n"); for line in body_filtered.lines() { - let formatted = format!(" {}\n", line); - filtered.push_str(&formatted); - print!("{}", formatted); + out.push_str(&format!(" {}\n", line)); } } } } - - timer.track( - &format!("gh issue view {}", issue_number), - &format!("rtk gh issue view {}", issue_number), - &raw, - &filtered, - ); - Ok(0) + out } fn run_workflow(args: &[String], verbose: u8, ultra_compact: bool) -> Result { @@ -791,8 +609,6 @@ fn run_workflow(args: &[String], verbose: u8, ultra_compact: bool) -> Result Result { - let timer = tracking::TimedExecution::start(); - let mut cmd = resolved_command("gh"); cmd.args([ "run", @@ -801,76 +617,48 @@ fn list_runs(args: &[String], _verbose: u8, ultra_compact: bool) -> Result "databaseId,name,status,conclusion,createdAt", ]); cmd.arg("--limit").arg("10"); - for arg in args { cmd.arg(arg); } + run_gh_json(cmd, "run list", |json| format_run_list(json, ultra_compact)) +} - let output = cmd.output().context("Failed to run gh run list")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track("gh run list", "rtk gh run list", &stderr, &stderr); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let json: Value = - serde_json::from_slice(&output.stdout).context("Failed to parse gh run list output")?; - - let mut filtered = String::new(); - - if let Some(runs) = json.as_array() { - if ultra_compact { - filtered.push_str("Runs\n"); - println!("Runs"); +fn format_run_list(json: &Value, ultra_compact: bool) -> String { + let runs = match json.as_array() { + Some(runs) => runs, + None => return String::new(), + }; + let mut out = String::new(); + out.push_str(if ultra_compact { + "Runs\n" + } else { + "Workflow Runs\n" + }); + for run in runs { + let id = run["databaseId"].as_i64().unwrap_or(0); + let name = run["name"].as_str().unwrap_or("???"); + let status = run["status"].as_str().unwrap_or("???"); + let conclusion = run["conclusion"].as_str().unwrap_or(""); + let icon = if ultra_compact { + match conclusion { + "success" => "[ok]", + "failure" => "[x]", + "cancelled" => "X", + _ if status == "in_progress" => "~", + _ => "?", + } } else { - filtered.push_str("Workflow Runs\n"); - println!("Workflow Runs"); - } - for run in runs { - let id = run["databaseId"].as_i64().unwrap_or(0); - let name = run["name"].as_str().unwrap_or("???"); - let status = run["status"].as_str().unwrap_or("???"); - let conclusion = run["conclusion"].as_str().unwrap_or(""); - - let icon = if ultra_compact { - match conclusion { - "success" => "[ok]", - "failure" => "[x]", - "cancelled" => "X", - _ => { - if status == "in_progress" { - "~" - } else { - "?" - } - } - } - } else { - match conclusion { - "success" => "[ok]", - "failure" => "[FAIL]", - "cancelled" => "[X]", - _ => { - if status == "in_progress" { - "[time]" - } else { - "[pending]" - } - } - } - }; - - let line = format!(" {} {} [{}]\n", icon, truncate(name, 50), id); - filtered.push_str(&line); - print!("{}", line); - } + match conclusion { + "success" => "[ok]", + "failure" => "[FAIL]", + "cancelled" => "[X]", + _ if status == "in_progress" => "[time]", + _ => "[pending]", + } + }; + out.push_str(&format!(" {} {} [{}]\n", icon, truncate(name, 50), id)); } - - timer.track("gh run list", "rtk gh run list", &raw, &filtered); - Ok(0) + out } /// Check if run view args should bypass filtering and pass through directly. @@ -887,115 +675,72 @@ fn view_run(args: &[String], _verbose: u8) -> Result { Some(result) => result, None => return Err(anyhow::anyhow!("Run ID required")), }; - - // Pass through when user requests logs or JSON — the filter would strip them if should_passthrough_run_view(&extra_args) { return run_passthrough_with_extra("gh", &["run", "view", &run_id], &extra_args); } - - let timer = tracking::TimedExecution::start(); - let mut cmd = resolved_command("gh"); cmd.args(["run", "view", &run_id]); for arg in &extra_args { cmd.arg(arg); } + let run_id_owned = run_id.clone(); + runner::run_filtered( + cmd, + "gh", + &format!("run view {}", run_id), + move |stdout| format_run_view(stdout, &run_id_owned), + RunOptions::stdout_only() + .early_exit_on_failure() + .no_trailing_newline(), + ) +} - let output = cmd.output().context("Failed to run gh run view")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track( - &format!("gh run view {}", run_id), - &format!("rtk gh run view {}", run_id), - &stderr, - &stderr, - ); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - // Parse output and show only failures - let stdout = String::from_utf8_lossy(&output.stdout); +fn format_run_view(stdout: &str, run_id: &str) -> String { + let mut out = String::new(); let mut in_jobs = false; - let mut filtered = String::new(); - - let line = format!("Workflow Run #{}\n", run_id); - filtered.push_str(&line); - print!("{}", line); - + out.push_str(&format!("Workflow Run #{}\n", run_id)); for line in stdout.lines() { if line.contains("JOBS") { in_jobs = true; } - if in_jobs { if line.contains('✓') || line.contains("success") { - // Skip successful jobs in compact mode continue; } if line.contains("[x]") || line.contains("fail") { - let formatted = format!(" [FAIL] {}\n", line.trim()); - filtered.push_str(&formatted); - print!("{}", formatted); + out.push_str(&format!(" [FAIL] {}\n", line.trim())); } } else if line.contains("Status:") || line.contains("Conclusion:") { - let formatted = format!(" {}\n", line.trim()); - filtered.push_str(&formatted); - print!("{}", formatted); + out.push_str(&format!(" {}\n", line.trim())); } } - - timer.track( - &format!("gh run view {}", run_id), - &format!("rtk gh run view {}", run_id), - &raw, - &filtered, - ); - Ok(0) + out } fn run_repo(args: &[String], _verbose: u8, _ultra_compact: bool) -> Result { - // Parse subcommand (default to "view") let (subcommand, rest_args) = if args.is_empty() { ("view", args) } else { (args[0].as_str(), &args[1..]) }; - if subcommand != "view" { return run_passthrough("gh", "repo", args); } - - let timer = tracking::TimedExecution::start(); - let mut cmd = resolved_command("gh"); cmd.arg("repo").arg("view"); - for arg in rest_args { cmd.arg(arg); } - cmd.args([ "--json", "name,owner,description,url,stargazerCount,forkCount,isPrivate", ]); + run_gh_json(cmd, "repo view", format_repo_view) +} - let output = cmd.output().context("Failed to run gh repo view")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track("gh repo view", "rtk gh repo view", &stderr, &stderr); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let json: Value = - serde_json::from_slice(&output.stdout).context("Failed to parse gh repo view output")?; - +fn format_repo_view(json: &Value) -> String { + let mut out = String::new(); let name = json["name"].as_str().unwrap_or("???"); let owner = json["owner"]["login"].as_str().unwrap_or("???"); let description = json["description"].as_str().unwrap_or(""); @@ -1003,119 +748,68 @@ fn run_repo(args: &[String], _verbose: u8, _ultra_compact: bool) -> Result let stars = json["stargazerCount"].as_i64().unwrap_or(0); let forks = json["forkCount"].as_i64().unwrap_or(0); let private = json["isPrivate"].as_bool().unwrap_or(false); - let visibility = if private { "[private]" } else { "[public]" }; - let mut filtered = String::new(); - - let line = format!("{}/{}\n", owner, name); - filtered.push_str(&line); - print!("{}", line); - - let line = format!(" {}\n", visibility); - filtered.push_str(&line); - print!("{}", line); - + out.push_str(&format!("{}/{}\n", owner, name)); + out.push_str(&format!(" {}\n", visibility)); if !description.is_empty() { - let line = format!(" {}\n", truncate(description, 80)); - filtered.push_str(&line); - print!("{}", line); + out.push_str(&format!(" {}\n", truncate(description, 80))); } - - let line = format!(" {} stars | {} forks\n", stars, forks); - filtered.push_str(&line); - print!("{}", line); - - let line = format!(" {}\n", url); - filtered.push_str(&line); - print!("{}", line); - - timer.track("gh repo view", "rtk gh repo view", &raw, &filtered); - Ok(0) + out.push_str(&format!(" {} stars | {} forks\n", stars, forks)); + out.push_str(&format!(" {}\n", url)); + out } fn pr_create(args: &[String], _verbose: u8) -> Result { - let timer = tracking::TimedExecution::start(); - let mut cmd = resolved_command("gh"); cmd.args(["pr", "create"]); for arg in args { cmd.arg(arg); } - - let output = cmd.output().context("Failed to run gh pr create")?; - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - - if !output.status.success() { - timer.track("gh pr create", "rtk gh pr create", &stderr, &stderr); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - // gh pr create outputs the URL on success - let url = stdout.trim(); - - // Try to extract PR number from URL (e.g., https://github.com/owner/repo/pull/42) - let pr_num = url.rsplit('/').next().unwrap_or(""); - - let detail = if !pr_num.is_empty() && pr_num.chars().all(|c| c.is_ascii_digit()) { - format!("#{} {}", pr_num, url) - } else { - url.to_string() - }; - - let filtered = ok_confirmation("created", &detail); - println!("{}", filtered); - - timer.track("gh pr create", "rtk gh pr create", &stdout, &filtered); - Ok(0) + runner::run_filtered( + cmd, + "gh", + "pr create", + |stdout| { + let url = stdout.trim(); + let pr_num = url.rsplit('/').next().unwrap_or(""); + let detail = if !pr_num.is_empty() && pr_num.chars().all(|c| c.is_ascii_digit()) { + format!("#{} {}", pr_num, url) + } else { + url.to_string() + }; + ok_confirmation("created", &detail) + }, + RunOptions::stdout_only().early_exit_on_failure(), + ) } fn pr_merge(args: &[String], _verbose: u8) -> Result { - let timer = tracking::TimedExecution::start(); - + let pr_num = args + .iter() + .find(|a| !a.starts_with('-')) + .map(|s| s.as_str()) + .unwrap_or("") + .to_string(); let mut cmd = resolved_command("gh"); cmd.args(["pr", "merge"]); for arg in args { cmd.arg(arg); } - - let output = cmd.output().context("Failed to run gh pr merge")?; - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - - if !output.status.success() { - timer.track("gh pr merge", "rtk gh pr merge", &stderr, &stderr); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - // Extract PR number from args (first non-flag arg) - let pr_num = args - .iter() - .find(|a| !a.starts_with('-')) - .map(|s| s.as_str()) - .unwrap_or(""); - - let detail = if !pr_num.is_empty() { - format!("#{}", pr_num) - } else { - String::new() - }; - - let filtered = ok_confirmation("merged", &detail); - println!("{}", filtered); - - // Use stdout or detail as raw input (gh pr merge doesn't output much) - let raw = if !stdout.trim().is_empty() { - stdout - } else { - detail.clone() - }; - - timer.track("gh pr merge", "rtk gh pr merge", &raw, &filtered); - Ok(0) + runner::run_filtered( + cmd, + "gh", + "pr merge", + move |_stdout| { + let detail = if !pr_num.is_empty() { + format!("#{}", pr_num) + } else { + String::new() + }; + ok_confirmation("merged", &detail) + }, + RunOptions::stdout_only().early_exit_on_failure(), + ) } /// Flags that change `gh pr diff` output from unified diff to a different format. @@ -1131,104 +825,55 @@ fn has_non_diff_format_flag(args: &[String]) -> bool { } fn pr_diff(args: &[String], _verbose: u8) -> Result { - // --no-compact: pass full diff through (gh CLI doesn't know this flag, strip it) let no_compact = args.iter().any(|a| a == "--no-compact"); let gh_args: Vec = args .iter() .filter(|a| *a != "--no-compact") .cloned() .collect(); - - // Passthrough when --no-compact or when a format flag changes output away from - // unified diff (e.g. --name-only produces a filename list, not diff hunks). if no_compact || has_non_diff_format_flag(&gh_args) { return run_passthrough_with_extra("gh", &["pr", "diff"], &gh_args); } - - let timer = tracking::TimedExecution::start(); - let mut cmd = resolved_command("gh"); cmd.args(["pr", "diff"]); for arg in gh_args.iter() { cmd.arg(arg); } - - let output = cmd.output().context("Failed to run gh pr diff")?; - let raw = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track("gh pr diff", "rtk gh pr diff", &stderr, &stderr); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - let filtered = if raw.trim().is_empty() { - let msg = "No diff\n"; - print!("{}", msg); - msg.to_string() - } else { - let compacted = git::compact_diff(&raw, 500); - println!("{}", compacted); - compacted - }; - - timer.track("gh pr diff", "rtk gh pr diff", &raw, &filtered); - Ok(0) + runner::run_filtered( + cmd, + "gh", + "pr diff", + |raw| { + if raw.trim().is_empty() { + "No diff".to_string() + } else { + git::compact_diff(raw, 500) + } + }, + RunOptions::stdout_only().early_exit_on_failure(), + ) } -/// Generic PR action handler for comment/edit fn pr_action(action: &str, args: &[String], _verbose: u8) -> Result { - let timer = tracking::TimedExecution::start(); let subcmd = &args[0]; - - let mut cmd = resolved_command("gh"); - cmd.arg("pr"); - for arg in args { - cmd.arg(arg); - } - - let output = cmd - .output() - .context(format!("Failed to run gh pr {}", subcmd))?; - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - timer.track( - &format!("gh pr {}", subcmd), - &format!("rtk gh pr {}", subcmd), - &stderr, - &stderr, - ); - eprintln!("{}", stderr.trim()); - return Ok(exit_code_from_output(&output, "gh")); - } - - // Extract PR number from args (skip args[0] which is the subcommand) let pr_num = args[1..] .iter() .find(|a| !a.starts_with('-')) .map(|s| format!("#{}", s)) .unwrap_or_default(); - - let filtered = ok_confirmation(action, &pr_num); - println!("{}", filtered); - - // Use stdout or pr_num as raw input - let raw = if !stdout.trim().is_empty() { - stdout - } else { - pr_num.clone() - }; - - timer.track( - &format!("gh pr {}", subcmd), - &format!("rtk gh pr {}", subcmd), - &raw, - &filtered, - ); - Ok(0) + let mut cmd = resolved_command("gh"); + cmd.arg("pr"); + for arg in args { + cmd.arg(arg); + } + let action = action.to_string(); + runner::run_filtered( + cmd, + "gh", + &format!("pr {}", subcmd), + move |_stdout| ok_confirmation(&action, &pr_num), + RunOptions::stdout_only().early_exit_on_failure(), + ) } fn run_api(args: &[String], _verbose: u8) -> Result { From 08aabb95c3ae6e0b734f696264e1e1a8c0f0b22e Mon Sep 17 00:00:00 2001 From: aesoft <43991222+aeppling@users.noreply.github.com> Date: Sun, 29 Mar 2026 12:31:24 +0200 Subject: [PATCH 5/7] fix(standardize): merge pattern into rues --- src/cmds/go/go_cmd.rs | 7 +- src/cmds/go/golangci_cmd.rs | 3 +- src/cmds/js/prettier_cmd.rs | 5 +- src/cmds/system/wc_cmd.rs | 5 +- src/discover/registry.rs | 51 ++++-------- src/discover/rules.rs | 158 ++++++++++++++++-------------------- 6 files changed, 96 insertions(+), 133 deletions(-) diff --git a/src/cmds/go/go_cmd.rs b/src/cmds/go/go_cmd.rs index 5d64f6a1..5935f9b0 100644 --- a/src/cmds/go/go_cmd.rs +++ b/src/cmds/go/go_cmd.rs @@ -1,5 +1,6 @@ //! Filters Go command output — test results, build errors, vet warnings. +use crate::core::runner; use crate::core::tracking; use crate::core::utils::{exit_code_from_output, resolved_command, truncate}; use crate::golangci_cmd; @@ -55,7 +56,7 @@ pub fn run_test(args: &[String], verbose: u8) -> Result { eprintln!("Running: go test -json {}", args.join(" ")); } - crate::core::runner::run_filtered( + runner::run_filtered( cmd, "go test", &args.join(" "), @@ -76,7 +77,7 @@ pub fn run_build(args: &[String], verbose: u8) -> Result { eprintln!("Running: go build {}", args.join(" ")); } - crate::core::runner::run_filtered( + runner::run_filtered( cmd, "go build", &args.join(" "), @@ -97,7 +98,7 @@ pub fn run_vet(args: &[String], verbose: u8) -> Result { eprintln!("Running: go vet {}", args.join(" ")); } - crate::core::runner::run_filtered( + runner::run_filtered( cmd, "go vet", &args.join(" "), diff --git a/src/cmds/go/golangci_cmd.rs b/src/cmds/go/golangci_cmd.rs index 94a50e6c..f24a9e05 100644 --- a/src/cmds/go/golangci_cmd.rs +++ b/src/cmds/go/golangci_cmd.rs @@ -1,6 +1,7 @@ //! Filters golangci-lint output, grouping issues by rule. use crate::core::config; +use crate::core::runner; use crate::core::utils::{resolved_command, truncate}; use anyhow::Result; use serde::Deserialize; @@ -114,7 +115,7 @@ pub fn run(args: &[String], verbose: u8) -> Result { } } - let exit_code = crate::core::runner::run_filtered( + let exit_code = runner::run_filtered( cmd, "golangci-lint", &args.join(" "), diff --git a/src/cmds/js/prettier_cmd.rs b/src/cmds/js/prettier_cmd.rs index 5dffa346..5a6b6c6b 100644 --- a/src/cmds/js/prettier_cmd.rs +++ b/src/cmds/js/prettier_cmd.rs @@ -1,5 +1,6 @@ //! Filters Prettier output to show only files that need formatting. +use crate::core::runner::{self, RunOptions}; use crate::core::utils::package_manager_exec; use anyhow::Result; @@ -14,12 +15,12 @@ pub fn run(args: &[String], verbose: u8) -> Result { eprintln!("Running: prettier {}", args.join(" ")); } - crate::core::runner::run_filtered( + runner::run_filtered( cmd, "prettier", &args.join(" "), filter_prettier_output, - crate::core::runner::RunOptions::stdout_only(), + RunOptions::stdout_only(), ) } diff --git a/src/cmds/system/wc_cmd.rs b/src/cmds/system/wc_cmd.rs index 8d95161f..dcfe98b3 100644 --- a/src/cmds/system/wc_cmd.rs +++ b/src/cmds/system/wc_cmd.rs @@ -6,6 +6,7 @@ /// - `wc -w file.py` → `96` /// - `wc -c file.py` → `978` /// - `wc -l *.py` → table with common path prefix stripped +use crate::core::runner::{self, RunOptions}; use crate::core::utils::resolved_command; use anyhow::Result; @@ -20,12 +21,12 @@ pub fn run(args: &[String], verbose: u8) -> Result { } let mode = detect_mode(args); - crate::core::runner::run_filtered( + runner::run_filtered( cmd, "wc", &args.join(" "), |stdout| filter_wc_output(stdout, &mode), - crate::core::runner::RunOptions::stdout_only(), + RunOptions::stdout_only(), ) } diff --git a/src/discover/registry.rs b/src/discover/registry.rs index f8d37c31..44609b0d 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -3,7 +3,7 @@ use lazy_static::lazy_static; use regex::{Regex, RegexSet}; -use super::rules::{IGNORED_EXACT, IGNORED_PREFIXES, PATTERNS, RULES}; +use super::rules::{IGNORED_EXACT, IGNORED_PREFIXES, RULES}; /// Result of classifying a command. #[derive(Debug, PartialEq)] @@ -43,10 +43,11 @@ pub fn category_avg_tokens(category: &str, subcmd: &str) -> usize { } lazy_static! { - static ref REGEX_SET: RegexSet = RegexSet::new(PATTERNS).expect("invalid regex patterns"); - static ref COMPILED: Vec = PATTERNS + static ref REGEX_SET: RegexSet = + RegexSet::new(RULES.iter().map(|r| r.pattern)).expect("invalid regex patterns"); + static ref COMPILED: Vec = RULES .iter() - .map(|p| Regex::new(p).expect("invalid regex")) + .map(|r| Regex::new(r.pattern).expect("invalid regex")) .collect(); static ref ENV_PREFIX: Regex = Regex::new(r"^(?:sudo\s+|env\s+|[A-Z_][A-Z0-9_]*=[^\s]*\s+)+").unwrap(); @@ -889,15 +890,6 @@ mod tests { ); } - #[test] - fn test_patterns_rules_length_match() { - assert_eq!( - PATTERNS.len(), - RULES.len(), - "PATTERNS and RULES must be aligned" - ); - } - #[test] fn test_registry_covers_all_cargo_subcommands() { // Verify that every CargoCommand variant (Build, Test, Clippy, Check, Fmt) @@ -2103,25 +2095,14 @@ mod tests { ); } - // --- Ensure PATTERNS and RULES stay aligned after modifications --- - - #[test] - fn test_patterns_rules_aligned_after_aws_psql() { - // If this fails, someone added a PATTERN without a matching RULE (or vice versa) - assert_eq!( - PATTERNS.len(), - RULES.len(), - "PATTERNS[{}] != RULES[{}] — they must stay 1:1", - PATTERNS.len(), - RULES.len() - ); - } - - // --- All RULES have non-empty rtk_cmd and at least one rewrite_prefix --- - #[test] - fn test_all_rules_have_valid_rtk_cmd() { + fn test_all_rules_are_complete() { for rule in RULES { + assert!( + !rule.pattern.is_empty(), + "Rule '{}' has empty pattern", + rule.rtk_cmd + ); assert!(!rule.rtk_cmd.is_empty(), "Rule with empty rtk_cmd found"); assert!( rule.rtk_cmd.starts_with("rtk "), @@ -2172,15 +2153,15 @@ mod tests { ); } - // --- Every PATTERN compiles to a valid Regex --- - #[test] fn test_all_patterns_are_valid_regex() { use regex::Regex; - for (i, pattern) in PATTERNS.iter().enumerate() { + for (i, rule) in RULES.iter().enumerate() { assert!( - Regex::new(pattern).is_ok(), - "PATTERNS[{i}] = '{pattern}' is not a valid regex" + Regex::new(rule.pattern).is_ok(), + "RULES[{i}] ({}) has invalid pattern '{}'", + rule.rtk_cmd, + rule.pattern ); } } diff --git a/src/discover/rules.rs b/src/discover/rules.rs index e5a36e7c..8b7e0652 100644 --- a/src/discover/rules.rs +++ b/src/discover/rules.rs @@ -1,11 +1,8 @@ -//! The master list of shell commands RTK knows how to rewrite. - use super::report::RtkStatus; -/// A rule mapping a shell command pattern to its RTK equivalent. pub struct RtkRule { + pub pattern: &'static str, pub rtk_cmd: &'static str, - /// Original command prefixes to replace with rtk_cmd (longest first for correct matching). pub rewrite_prefixes: &'static [&'static str], pub category: &'static str, pub savings_pct: f64, @@ -13,85 +10,9 @@ pub struct RtkRule { pub subcmd_status: &'static [(&'static str, RtkStatus)], } -// Patterns ordered to match RULES indices exactly. -pub const PATTERNS: &[&str] = &[ - r"^git\s+(?:-[Cc]\s+\S+\s+)*(status|log|diff|show|add|commit|push|pull|branch|fetch|stash|worktree)", - r"^gh\s+(pr|issue|run|repo|api|release)", - r"^cargo\s+(build|test|clippy|check|fmt|install)", - r"^pnpm\s+(list|ls|outdated|install)", - r"^npm\s+(run|exec)", - r"^npx\s+", - r"^(cat|head|tail)\s+", - r"^(rg|grep)\s+", - r"^ls(\s|$)", - r"^find\s+", - r"^(npx\s+|pnpm\s+)?tsc(\s|$)", - r"^(npx\s+|pnpm\s+)?(eslint|biome|lint)(\s|$)", - r"^(npx\s+|pnpm\s+)?prettier", - r"^(npx\s+|pnpm\s+)?next\s+build", - r"^(pnpm\s+|npx\s+)?(vitest|jest|test)(\s|$)", - r"^(npx\s+|pnpm\s+)?playwright", - r"^(npx\s+|pnpm\s+)?prisma", - r"^docker\s+(ps|images|logs|run|exec|build|compose\s+(ps|logs|build))", - r"^kubectl\s+(get|logs|describe|apply)", - r"^tree(\s|$)", - r"^diff\s+", - r"^curl\s+", - r"^wget\s+", - r"^(python3?\s+-m\s+)?mypy(\s|$)", - // Python tooling - r"^ruff\s+(check|format)", - r"^(python\s+-m\s+)?pytest(\s|$)", - r"^(pip3?|uv\s+pip)\s+(list|outdated|install)", - // Go tooling - r"^go\s+(test|build|vet)", - r"^golangci-lint(\s|$)", - // Ruby tooling - r"^bundle\s+(install|update)\b", - r"^(?:bundle\s+exec\s+)?(?:bin/)?(?:rake|rails)\s+test", - r"^(?:bundle\s+exec\s+)?rspec(?:\s|$)", - r"^(?:bundle\s+exec\s+)?rubocop(?:\s|$)", - // AWS CLI - r"^aws\s+", - // PostgreSQL - r"^psql(\s|$)", - // TOML-filtered commands - r"^ansible-playbook\b", - r"^brew\s+(install|upgrade)\b", - r"^composer\s+(install|update|require)\b", - r"^df(\s|$)", - r"^dotnet\s+build\b", - r"^du\b", - r"^fail2ban-client\b", - r"^gcloud\b", - r"^hadolint\b", - r"^helm\b", - r"^iptables\b", - r"^make\b", - r"^markdownlint\b", - r"^mix\s+(compile|format)(\s|$)", - r"^mvn\s+(compile|package|clean|install)\b", - r"^ping\b", - r"^pio\s+run", - r"^poetry\s+(install|lock|update)\b", - r"^pre-commit\b", - r"^ps(\s|$)", - r"^quarto\s+render", - r"^rsync\b", - r"^shellcheck\b", - r"^shopify\s+theme\s+(push|pull)", - r"^sops\b", - r"^swift\s+(build|test)\b", - r"^systemctl\s+status\b", - r"^terraform\s+plan", - r"^tofu\s+(fmt|init|plan|validate)(\s|$)", - r"^trunk\s+build", - r"^uv\s+(sync|pip\s+install)\b", - r"^yamllint\b", -]; - pub const RULES: &[RtkRule] = &[ RtkRule { + pattern: r"^git\s+(?:-[Cc]\s+\S+\s+)*(status|log|diff|show|add|commit|push|pull|branch|fetch|stash|worktree)", rtk_cmd: "rtk git", rewrite_prefixes: &["git"], category: "Git", @@ -105,6 +26,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^gh\s+(pr|issue|run|repo|api|release)", rtk_cmd: "rtk gh", rewrite_prefixes: &["gh"], category: "GitHub", @@ -113,6 +35,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^cargo\s+(build|test|clippy|check|fmt|install)", rtk_cmd: "rtk cargo", rewrite_prefixes: &["cargo"], category: "Cargo", @@ -121,6 +44,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[("fmt", RtkStatus::Passthrough)], }, RtkRule { + pattern: r"^pnpm\s+(list|ls|outdated|install)", rtk_cmd: "rtk pnpm", rewrite_prefixes: &["pnpm"], category: "PackageManager", @@ -129,6 +53,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^npm\s+(run|exec)", rtk_cmd: "rtk npm", rewrite_prefixes: &["npm"], category: "PackageManager", @@ -137,6 +62,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^npx\s+", rtk_cmd: "rtk npx", rewrite_prefixes: &["npx"], category: "PackageManager", @@ -145,6 +71,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(cat|head|tail)\s+", rtk_cmd: "rtk read", rewrite_prefixes: &["cat", "head", "tail"], category: "Files", @@ -153,6 +80,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(rg|grep)\s+", rtk_cmd: "rtk grep", rewrite_prefixes: &["rg", "grep"], category: "Files", @@ -161,6 +89,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^ls(\s|$)", rtk_cmd: "rtk ls", rewrite_prefixes: &["ls"], category: "Files", @@ -169,6 +98,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^find\s+", rtk_cmd: "rtk find", rewrite_prefixes: &["find"], category: "Files", @@ -177,7 +107,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { - // Longest prefixes first for correct matching + pattern: r"^(npx\s+|pnpm\s+)?tsc(\s|$)", rtk_cmd: "rtk tsc", rewrite_prefixes: &["pnpm tsc", "npx tsc", "tsc"], category: "Build", @@ -186,6 +116,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(npx\s+|pnpm\s+)?(eslint|biome|lint)(\s|$)", rtk_cmd: "rtk lint", rewrite_prefixes: &[ "npx eslint", @@ -201,6 +132,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(npx\s+|pnpm\s+)?prettier", rtk_cmd: "rtk prettier", rewrite_prefixes: &["npx prettier", "pnpm prettier", "prettier"], category: "Build", @@ -209,7 +141,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { - // "next build" is stripped to "rtk next" — the build subcommand is internal + pattern: r"^(npx\s+|pnpm\s+)?next\s+build", rtk_cmd: "rtk next", rewrite_prefixes: &["npx next build", "pnpm next build", "next build"], category: "Build", @@ -218,6 +150,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(pnpm\s+|npx\s+)?(vitest|jest|test)(\s|$)", rtk_cmd: "rtk vitest", rewrite_prefixes: &["pnpm vitest", "npx vitest", "vitest", "jest"], category: "Tests", @@ -226,6 +159,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(npx\s+|pnpm\s+)?playwright", rtk_cmd: "rtk playwright", rewrite_prefixes: &["npx playwright", "pnpm playwright", "playwright"], category: "Tests", @@ -234,6 +168,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(npx\s+|pnpm\s+)?prisma", rtk_cmd: "rtk prisma", rewrite_prefixes: &["npx prisma", "pnpm prisma", "prisma"], category: "Build", @@ -242,6 +177,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^docker\s+(ps|images|logs|run|exec|build|compose\s+(ps|logs|build))", rtk_cmd: "rtk docker", rewrite_prefixes: &["docker"], category: "Infra", @@ -250,6 +186,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^kubectl\s+(get|logs|describe|apply)", rtk_cmd: "rtk kubectl", rewrite_prefixes: &["kubectl"], category: "Infra", @@ -258,6 +195,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^tree(\s|$)", rtk_cmd: "rtk tree", rewrite_prefixes: &["tree"], category: "Files", @@ -266,6 +204,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^diff\s+", rtk_cmd: "rtk diff", rewrite_prefixes: &["diff"], category: "Files", @@ -274,6 +213,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^curl\s+", rtk_cmd: "rtk curl", rewrite_prefixes: &["curl"], category: "Network", @@ -282,6 +222,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^wget\s+", rtk_cmd: "rtk wget", rewrite_prefixes: &["wget"], category: "Network", @@ -290,6 +231,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(python3?\s+-m\s+)?mypy(\s|$)", rtk_cmd: "rtk mypy", rewrite_prefixes: &["python3 -m mypy", "python -m mypy", "mypy"], category: "Build", @@ -297,8 +239,8 @@ pub const RULES: &[RtkRule] = &[ subcmd_savings: &[], subcmd_status: &[], }, - // Python tooling RtkRule { + pattern: r"^ruff\s+(check|format)", rtk_cmd: "rtk ruff", rewrite_prefixes: &["ruff"], category: "Python", @@ -307,6 +249,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(python\s+-m\s+)?pytest(\s|$)", rtk_cmd: "rtk pytest", rewrite_prefixes: &["python -m pytest", "pytest"], category: "Python", @@ -315,6 +258,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(pip3?|uv\s+pip)\s+(list|outdated|install)", rtk_cmd: "rtk pip", rewrite_prefixes: &["pip3", "pip", "uv pip"], category: "Python", @@ -322,8 +266,8 @@ pub const RULES: &[RtkRule] = &[ subcmd_savings: &[("list", 75.0), ("outdated", 80.0)], subcmd_status: &[], }, - // Go tooling RtkRule { + pattern: r"^go\s+(test|build|vet)", rtk_cmd: "rtk go", rewrite_prefixes: &["go"], category: "Go", @@ -332,6 +276,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^golangci-lint(\s|$)", rtk_cmd: "rtk golangci-lint", rewrite_prefixes: &["golangci-lint", "golangci"], category: "Go", @@ -339,8 +284,8 @@ pub const RULES: &[RtkRule] = &[ subcmd_savings: &[], subcmd_status: &[], }, - // Ruby tooling RtkRule { + pattern: r"^bundle\s+(install|update)\b", rtk_cmd: "rtk bundle", rewrite_prefixes: &["bundle"], category: "Ruby", @@ -349,6 +294,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(?:bundle\s+exec\s+)?(?:bin/)?(?:rake|rails)\s+test", rtk_cmd: "rtk rake", rewrite_prefixes: &[ "bundle exec rails", @@ -363,6 +309,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(?:bundle\s+exec\s+)?rspec(?:\s|$)", rtk_cmd: "rtk rspec", rewrite_prefixes: &["bundle exec rspec", "bin/rspec", "rspec"], category: "Tests", @@ -371,6 +318,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^(?:bundle\s+exec\s+)?rubocop(?:\s|$)", rtk_cmd: "rtk rubocop", rewrite_prefixes: &["bundle exec rubocop", "rubocop"], category: "Build", @@ -378,8 +326,8 @@ pub const RULES: &[RtkRule] = &[ subcmd_savings: &[], subcmd_status: &[], }, - // AWS CLI RtkRule { + pattern: r"^aws\s+", rtk_cmd: "rtk aws", rewrite_prefixes: &["aws"], category: "Infra", @@ -387,8 +335,8 @@ pub const RULES: &[RtkRule] = &[ subcmd_savings: &[], subcmd_status: &[], }, - // PostgreSQL RtkRule { + pattern: r"^psql(\s|$)", rtk_cmd: "rtk psql", rewrite_prefixes: &["psql"], category: "Infra", @@ -396,8 +344,8 @@ pub const RULES: &[RtkRule] = &[ subcmd_savings: &[], subcmd_status: &[], }, - // TOML-filtered commands RtkRule { + pattern: r"^ansible-playbook\b", rtk_cmd: "rtk ansible-playbook", rewrite_prefixes: &["ansible-playbook"], category: "Infra", @@ -406,6 +354,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^brew\s+(install|upgrade)\b", rtk_cmd: "rtk brew", rewrite_prefixes: &["brew"], category: "PackageManager", @@ -414,6 +363,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^composer\s+(install|update|require)\b", rtk_cmd: "rtk composer", rewrite_prefixes: &["composer"], category: "PackageManager", @@ -422,6 +372,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^df(\s|$)", rtk_cmd: "rtk df", rewrite_prefixes: &["df"], category: "System", @@ -430,6 +381,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^dotnet\s+build\b", rtk_cmd: "rtk dotnet", rewrite_prefixes: &["dotnet"], category: "Build", @@ -438,6 +390,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^du\b", rtk_cmd: "rtk du", rewrite_prefixes: &["du"], category: "System", @@ -446,6 +399,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^fail2ban-client\b", rtk_cmd: "rtk fail2ban-client", rewrite_prefixes: &["fail2ban-client"], category: "Infra", @@ -454,6 +408,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^gcloud\b", rtk_cmd: "rtk gcloud", rewrite_prefixes: &["gcloud"], category: "Infra", @@ -462,6 +417,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^hadolint\b", rtk_cmd: "rtk hadolint", rewrite_prefixes: &["hadolint"], category: "Build", @@ -470,6 +426,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^helm\b", rtk_cmd: "rtk helm", rewrite_prefixes: &["helm"], category: "Infra", @@ -478,6 +435,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^iptables\b", rtk_cmd: "rtk iptables", rewrite_prefixes: &["iptables"], category: "Infra", @@ -486,6 +444,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^make\b", rtk_cmd: "rtk make", rewrite_prefixes: &["make"], category: "Build", @@ -494,6 +453,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^markdownlint\b", rtk_cmd: "rtk markdownlint", rewrite_prefixes: &["markdownlint"], category: "Build", @@ -502,6 +462,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^mix\s+(compile|format)(\s|$)", rtk_cmd: "rtk mix", rewrite_prefixes: &["mix"], category: "Build", @@ -510,6 +471,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^mvn\s+(compile|package|clean|install)\b", rtk_cmd: "rtk mvn", rewrite_prefixes: &["mvn"], category: "Build", @@ -518,6 +480,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^ping\b", rtk_cmd: "rtk ping", rewrite_prefixes: &["ping"], category: "Network", @@ -526,6 +489,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^pio\s+run", rtk_cmd: "rtk pio", rewrite_prefixes: &["pio"], category: "Build", @@ -534,6 +498,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^poetry\s+(install|lock|update)\b", rtk_cmd: "rtk poetry", rewrite_prefixes: &["poetry"], category: "Python", @@ -542,6 +507,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^pre-commit\b", rtk_cmd: "rtk pre-commit", rewrite_prefixes: &["pre-commit"], category: "Build", @@ -550,6 +516,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^ps(\s|$)", rtk_cmd: "rtk ps", rewrite_prefixes: &["ps"], category: "System", @@ -558,6 +525,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^quarto\s+render", rtk_cmd: "rtk quarto", rewrite_prefixes: &["quarto"], category: "Build", @@ -566,6 +534,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^rsync\b", rtk_cmd: "rtk rsync", rewrite_prefixes: &["rsync"], category: "Network", @@ -574,6 +543,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^shellcheck\b", rtk_cmd: "rtk shellcheck", rewrite_prefixes: &["shellcheck"], category: "Build", @@ -582,6 +552,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^shopify\s+theme\s+(push|pull)", rtk_cmd: "rtk shopify", rewrite_prefixes: &["shopify"], category: "Build", @@ -590,6 +561,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^sops\b", rtk_cmd: "rtk sops", rewrite_prefixes: &["sops"], category: "Infra", @@ -598,6 +570,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^swift\s+(build|test)\b", rtk_cmd: "rtk swift", rewrite_prefixes: &["swift"], category: "Build", @@ -606,6 +579,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^systemctl\s+status\b", rtk_cmd: "rtk systemctl", rewrite_prefixes: &["systemctl"], category: "System", @@ -614,6 +588,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^terraform\s+plan", rtk_cmd: "rtk terraform", rewrite_prefixes: &["terraform"], category: "Infra", @@ -622,6 +597,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^tofu\s+(fmt|init|plan|validate)(\s|$)", rtk_cmd: "rtk tofu", rewrite_prefixes: &["tofu"], category: "Infra", @@ -630,6 +606,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^trunk\s+build", rtk_cmd: "rtk trunk", rewrite_prefixes: &["trunk"], category: "Build", @@ -638,6 +615,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^uv\s+(sync|pip\s+install)\b", rtk_cmd: "rtk uv", rewrite_prefixes: &["uv"], category: "Python", @@ -646,6 +624,7 @@ pub const RULES: &[RtkRule] = &[ subcmd_status: &[], }, RtkRule { + pattern: r"^yamllint\b", rtk_cmd: "rtk yamllint", rewrite_prefixes: &["yamllint"], category: "Build", @@ -655,7 +634,6 @@ pub const RULES: &[RtkRule] = &[ }, ]; -/// Commands to ignore (shell builtins, trivial, already rtk). pub const IGNORED_PREFIXES: &[&str] = &[ "cd ", "cd\t", From f3217a467b543a3181605b257162f2b3ab5d5df0 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Sun, 29 Mar 2026 18:43:53 +0200 Subject: [PATCH 6/7] fix(registry): quoted env prefix + inline regex cleanup + routing docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - ENV_PREFIX now handles FOO="bar baz", FOO='val', escaped quotes - Move 6 inline lazy_static regex to module level - Merge rewrite_head_numeric + rewrite_tail_lines → rewrite_line_range - Consolidate head/tail call sites in rewrite_segment - Add rewrite pipeline documentation to TECHNICAL.md - Rewrite discover README as concept/flow documentation - Add lexer-based tokenizer for shell command parsing Co-Authored-By: Andrew Hundt --- Cargo.lock | 1 + docs/TECHNICAL.md | 115 +++++ src/discover/README.md | 81 +++- src/discover/lexer.rs | 949 +++++++++++++++++++++++++++++++++++++++ src/discover/mod.rs | 1 + src/discover/registry.rs | 402 +++++++---------- src/main.rs | 23 +- 7 files changed, 1293 insertions(+), 279 deletions(-) create mode 100644 src/discover/lexer.rs diff --git a/Cargo.lock b/Cargo.lock index 92d75c06..db7f8950 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -912,6 +912,7 @@ dependencies = [ "colored", "dirs", "flate2", + "getrandom 0.4.2", "hostname", "ignore", "lazy_static", diff --git a/docs/TECHNICAL.md b/docs/TECHNICAL.md index 541f712d..04522683 100644 --- a/docs/TECHNICAL.md +++ b/docs/TECHNICAL.md @@ -94,6 +94,121 @@ All rewrite logic lives in Rust (`src/discover/registry.rs`). Hooks are thin del > **Details**: [`hooks/README.md`](../hooks/README.md) covers each agent's JSON format, the rewrite registry, compound command handling, and the `RTK_DISABLED` override. +#### Rewrite Pipeline + +The rewrite pipeline is how RTK intercepts and rewrites commands. The call chain is: + +``` +hook shell → rewrite_cmd.rs → rewrite_command() → rewrite_compound() → rewrite_segment() → classify_command() +``` + +Traced step by step for `cargo fmt --all && cargo test 2>&1 | tail -20`: + +``` +LLM Agent: "cargo fmt --all && cargo test 2>&1 | tail -20" + | + | Hook shell (hooks/claude/rtk-rewrite.sh) + | Reads JSON from agent, extracts command, calls `rtk rewrite "$CMD"` + | On failure (jq missing, rtk missing, old version): exit 0 (passthrough) + | + v +rewrite_cmd::run(cmd) [src/hooks/rewrite_cmd.rs] + | 1. Load config → hooks.exclude_commands + | 2. check_command(cmd) → Deny → exit(2) + | 3. registry::rewrite_command(cmd, excluded) + | → None → exit(1) (no RTK equivalent, passthrough) + | → Some + Allow → print, exit(0) + | → Some + Ask → print, exit(3) + | + v +rewrite_command(cmd, excluded) [src/discover/registry.rs] + | Early exits: + | - Empty → None + | - Contains "<<" or "$((" (heredoc/arithmetic) → None + | - Simple "rtk ..." (no operators) → return as-is + | - Otherwise → rewrite_compound(cmd, excluded) + | + v +rewrite_compound(cmd, excluded) [src/discover/registry.rs] + | + | Step 1 — Tokenize (lexer.rs) + | tokenize() produces typed tokens with byte offsets: + | Arg("cargo") Arg("fmt") Arg("--all") + | Operator("&&") + | Arg("cargo") Arg("test") Redirect("2>&1") + | Pipe("|") + | Arg("tail") Arg("-20") + | + | Step 2 — Split on operators, rewrite each segment + | Operator (&&, ||, ;) → rewrite both sides + | Pipe (|) → rewrite left side only, keep right side raw + | exception: find/fd before pipe → skip rewrite + | Shellism (&) → rewrite both sides (background) + | + | Calls rewrite_segment() per segment: + | segment 1: "cargo fmt --all" + | segment 2: "cargo test 2>&1" + | after pipe: "tail -20" kept raw + | + v +rewrite_segment(seg, excluded) [src/discover/registry.rs] + | + | Step 3 — Strip trailing redirects + | strip_trailing_redirects() re-tokenizes the segment: + | "cargo test 2>&1" → cmd_part="cargo test", redirect=" 2>&1" + | (simple commands like "cargo fmt --all" → no redirect, suffix is "") + | + | Step 4 — Already RTK → return as-is + | + | Step 5 — Special cases (short-circuit before classification) + | head -N / --lines=N → rewrite_line_range() → "rtk read file --max-lines N" + | tail -N / -n N / --lines N → rewrite_line_range() → "rtk read file --tail-lines N" + | head/tail with unsupported flag (-c, -f) → None (skip rewrite) + | cat with incompatible flag (-A, -v, -e) → None (skip rewrite) + | + | Step 6 — classify_command(cmd_part) [see below] + | → Supported → check excluded list → continue + | → Unsupported/Ignored → None (skip rewrite) + | + | Step 7 — Build rewritten command + | a. Find matching rule from rules.rs + | b. Extract env prefix (ENV_PREFIX regex, second pass — first was in classify) + | e.g. "GIT_SSH_COMMAND=\"ssh -o ...\" git push" → prefix="GIT_SSH_COMMAND=..." + | c. Guard: RTK_DISABLED=1 in prefix → None + | d. Guard: gh with --json/--jq/--template → None + | e. Apply rule's rewrite_prefixes: "cargo fmt" → "rtk cargo fmt" + | f. Reassemble: env_prefix + rtk_cmd + args + redirect_suffix + | + v +classify_command(cmd) [src/discover/registry.rs] + | 1. Check IGNORED_EXACT (cd, echo, fi, done, ...) + | 2. Check IGNORED_PREFIXES (rtk, mkdir, mv, ...) + | 3. Strip env prefix with ENV_PREFIX regex (for pattern matching only) + | 4. Normalize absolute paths: /usr/bin/grep → grep + | 5. Strip git global opts: git -C /tmp status → git status + | 6. Guard: cat/head/tail with redirect (>, >>) → Unsupported (write, not read) + | 7. Match against REGEX_SET (60+ compiled patterns from rules.rs) + | 8. Extract subcommand → lookup custom savings/status overrides + | 9. Return Classification::Supported { rtk_equivalent, category, savings, status } + | + v +Result: "rtk cargo fmt --all && rtk cargo test 2>&1 | tail -20" + | + | Hook response + | Hook wraps result in agent-specific JSON, returns to LLM agent + | + v +LLM Agent executes rewritten command + (bash handles && and |, each rtk invocation is a separate process) +``` + +Key design decisions: +- **Lexer-based tokenization**: A single-pass state machine (`lexer.rs`) handles all shell constructs (quotes, escapes, redirects, operators). Used for both compound splitting and redirect stripping. +- **Segment-level rewriting**: Compound commands are split by operators, each segment rewritten independently. Bash recombines them at execution time. +- **Pipe semantics**: Only the left side of `|` is rewritten. The pipe consumer (grep, head, wc) runs raw. `find`/`fd` before a pipe is never rewritten (output format incompatible with xargs). +- **Double env prefix handling**: `classify_command()` strips env prefixes to match the underlying command against rules. `rewrite_segment()` extracts the same prefix separately to re-prepend it to the rewritten command. +- **Fallback contract**: If any segment fails to match, it stays raw. `rewrite_command()` returns `None` only when zero segments were rewritten. + ### 3.3 CLI Parsing and Routing Once the rewritten command reaches RTK: diff --git a/src/discover/README.md b/src/discover/README.md index cb523f4d..02f6aaab 100644 --- a/src/discover/README.md +++ b/src/discover/README.md @@ -1,32 +1,73 @@ -# Discover — Claude Code History Analysis +# Discover — History Analysis & Command Rewrite -> See also [docs/TECHNICAL.md](../../docs/TECHNICAL.md) for the full architecture overview +> Full rewrite pipeline diagram: [docs/TECHNICAL.md](../../docs/TECHNICAL.md#32-hook-interception-command-rewriting) -## Purpose +## What This Module Does -Scans Claude Code JSONL session files to identify commands that could benefit from RTK filtering. Powers the `rtk discover` command, which reports missed savings opportunities and adoption metrics. +This module has two jobs: -Also provides the **command rewrite registry** — the single source of truth for all rewrite patterns used by every LLM agent hook to decide which commands to rewrite. +1. **Rewrite commands** — Every LLM agent hook calls `rtk rewrite "git status"`. This module decides whether to rewrite it (`rtk git status`) or pass it through unchanged. This is the hot path — every command the LLM runs goes through here. -## Key Types +2. **Analyze history** — `rtk discover` scans past LLM sessions to find commands that *could have been* rewritten but weren't. Same classification logic, different consumer. -- **`Classification`** — Result of `classify_command()`: `Supported { rtk_equivalent, category, savings_pct, status }`, `Unsupported { base_command }`, or `Ignored` -- **`RtkStatus`** — `Existing` (dedicated handler), `Passthrough` (external_subcommand), `NotSupported` -- **`SessionProvider`** trait — abstraction for session file discovery (currently only `ClaudeProvider`) -- **`ExtractedCommand`** — command string + output length + error flag extracted from JSONL +## How Command Rewriting Works -## Dependencies +When a hook sends `cargo fmt --all && cargo test 2>&1 | tail -20`: -- **Uses**: `walkdir` (session file discovery), `lazy_static`/`regex` (pattern matching), `serde_json` (JSONL parsing) -- **Used by**: `src/hooks/rewrite_cmd.rs` (imports `registry::classify_command` for `rtk rewrite`), `src/learn/` (imports `provider::ClaudeProvider` for session extraction), `src/main.rs` (routes `rtk discover` command) +**Tokenization** — The lexer (`lexer.rs`) turns the raw string into typed tokens. It's a single-pass state machine that understands shell quoting, escapes, redirects, and operators. This is critical because naive string splitting breaks on quoted content like `git commit -m "fix && update"`. -## Registry Architecture +``` +"cargo test 2>&1 && git status" +→ [Arg("cargo"), Arg("test"), Redirect("2>&1"), Operator("&&"), Arg("git"), Arg("status")] +``` -`registry.rs` is the largest file in the project. It contains: +**Compound splitting** — The rewrite engine walks the tokens, splitting on `Operator` (`&&`, `||`, `;`) and `Pipe` (`|`). Each segment is rewritten independently. For pipes, only the left side is rewritten (the pipe consumer like `grep` or `head` runs raw). `find`/`fd` before a pipe is never rewritten because rtk's grouped output format breaks pipe consumers like `xargs`. -1. **Pattern matching** — Compiled regexes in `lazy_static!` matching command prefixes (e.g., `^git\s+(status|log|diff|...)`) -2. **Compound splitting** — `split_command_chain()` handles `&&`, `||`, `;`, `|`, `&` operators with shell quoting awareness -3. **RTK_DISABLED detection** — `has_rtk_disabled_prefix()` / `strip_disabled_prefix()` for per-command override -4. **Category averages** — `category_avg_tokens()` estimates output tokens when real data unavailable +**Per-segment rewriting** — Each segment goes through: -The registry is used by both `rtk discover` (analysis) and `rtk rewrite` (live rewriting). Same patterns, different consumers. +1. Strip trailing redirects (`2>&1`, `>/dev/null`) — matched via lexer tokens, set aside, re-appended after rewriting +2. Short-circuit special cases — `head -20 file` → `rtk read file --max-lines 20`, `tail -n 5 file` → `rtk read file --tail-lines 5`. These can't go through generic prefix replacement because it would produce `rtk read -20 file` (wrong flag position) +3. Classify the command — strip env prefixes (`sudo`, `FOO="bar baz"`), normalize paths (`/usr/bin/grep` → `grep`), strip git global opts (`git -C /tmp` → `git`), then match against 60+ regex patterns from `rules.rs` +4. Apply the rewrite — find the matching rule, replace the command prefix with `rtk `, re-prepend the env prefix, re-append the redirect suffix + +**Guards along the way:** +- `RTK_DISABLED=1` in the env prefix → skip rewrite +- `gh` with `--json`/`--jq`/`--template` → skip (structured output, rtk would corrupt it) +- `cat` with flags other than `-n` → skip (different semantics than `rtk read`) +- `cat`/`head`/`tail` with `>` or `>>` → skip (write operation, not a read) +- Command in `hooks.exclude_commands` config → skip + +**Result**: `rtk cargo fmt --all && rtk cargo test 2>&1 | tail -20`. Bash handles the `&&` and `|` at execution time — each `rtk` invocation is a separate process. + +## How History Analysis Works + +`rtk discover` reads Claude Code JSONL session files. Each file contains `tool_use`/`tool_result` pairs for every command the LLM ran. The module: + +1. Extracts commands from the JSONL (via `SessionProvider` trait — currently only Claude Code) +2. Splits compound commands using the same lexer-based tokenization +3. Classifies each command against the same rules used for live rewriting +4. Aggregates results: which commands could have been rewritten, estimated token savings, adoption rate + +The classification logic is shared between discover and rewrite — same patterns, same rules, different consumers. + +## Env Prefix Handling + +The `ENV_PREFIX` regex strips env variable assignments, `sudo`, and `env` from the front of commands. It handles: +- Unquoted: `FOO=bar` +- Double-quoted with spaces: `FOO="bar baz"` +- Single-quoted: `FOO='bar baz'` +- Escaped quotes: `FOO="he said \"hello\""` +- Chained: `A="x y" B=1 sudo git status` + +The prefix is stripped twice: once in `classify_command()` to match the underlying command against rules, and again in `rewrite_segment()` to extract it for re-prepending to the rewritten command. + +## Adding a New Rewrite Rule + +Add an entry to `rules.rs`. Each rule has: +- `pattern` — regex that matches the command (used by `RegexSet` for fast matching) +- `rtk_cmd` — the RTK command it maps to (e.g., `"rtk cargo"`) +- `rewrite_prefixes` — command prefixes to replace (e.g., `&["cargo"]`) +- `category`, `savings_pct` — metadata for discover reports +- `subcmd_savings`, `subcmd_status` — per-subcommand overrides + +No other files need to change. The registry compiles the patterns at first use via `lazy_static`. diff --git a/src/discover/lexer.rs b/src/discover/lexer.rs new file mode 100644 index 00000000..0ee0d44a --- /dev/null +++ b/src/discover/lexer.rs @@ -0,0 +1,949 @@ +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum TokenKind { + Arg, + Operator, + Pipe, + Redirect, + Shellism, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ParsedToken { + pub kind: TokenKind, + pub value: String, + pub offset: usize, +} + +pub fn tokenize(input: &str) -> Vec { + let mut tokens = Vec::new(); + let mut current = String::new(); + let mut current_start: usize = 0; + let mut byte_pos: usize = 0; + let mut chars = input.chars().peekable(); + let mut quote: Option = None; + let mut escaped = false; + + while let Some(c) = chars.next() { + let char_len = c.len_utf8(); + + if escaped { + current.push('\\'); + current.push(c); + byte_pos += char_len; + escaped = false; + continue; + } + if c == '\\' && quote != Some('\'') { + escaped = true; + if current.is_empty() { + current_start = byte_pos; + } + byte_pos += char_len; + continue; + } + + if let Some(q) = quote { + if c == q { + quote = None; + } + current.push(c); + byte_pos += char_len; + continue; + } + if c == '\'' || c == '"' { + quote = Some(c); + if current.is_empty() { + current_start = byte_pos; + } + current.push(c); + byte_pos += char_len; + continue; + } + + match c { + '$' => { + flush_arg(&mut tokens, &mut current, current_start); + let start = byte_pos; + byte_pos += char_len; + if chars + .peek() + .is_some_and(|&nc| nc.is_ascii_alphabetic() || nc == '_') + { + let mut name = String::from("$"); + while chars + .peek() + .is_some_and(|&nc| nc.is_ascii_alphanumeric() || nc == '_') + { + let nc = chars.next().unwrap(); + byte_pos += nc.len_utf8(); + name.push(nc); + } + tokens.push(ParsedToken { + kind: TokenKind::Arg, + value: name, + offset: start, + }); + } else { + tokens.push(ParsedToken { + kind: TokenKind::Shellism, + value: "$".into(), + offset: start, + }); + } + current_start = byte_pos; + } + '*' | '?' | '`' | '(' | ')' | '{' | '}' | '!' => { + flush_arg(&mut tokens, &mut current, current_start); + tokens.push(ParsedToken { + kind: TokenKind::Shellism, + value: c.to_string(), + offset: byte_pos, + }); + byte_pos += char_len; + current_start = byte_pos; + } + '|' => { + flush_arg(&mut tokens, &mut current, current_start); + let start = byte_pos; + byte_pos += char_len; + if chars.peek() == Some(&'|') { + byte_pos += chars.next().unwrap().len_utf8(); + tokens.push(ParsedToken { + kind: TokenKind::Operator, + value: "||".into(), + offset: start, + }); + } else { + tokens.push(ParsedToken { + kind: TokenKind::Pipe, + value: "|".into(), + offset: start, + }); + } + current_start = byte_pos; + } + ';' => { + flush_arg(&mut tokens, &mut current, current_start); + tokens.push(ParsedToken { + kind: TokenKind::Operator, + value: ";".into(), + offset: byte_pos, + }); + byte_pos += char_len; + current_start = byte_pos; + } + '&' => { + flush_arg(&mut tokens, &mut current, current_start); + let start = byte_pos; + byte_pos += char_len; + if chars.peek() == Some(&'&') { + byte_pos += chars.next().unwrap().len_utf8(); + tokens.push(ParsedToken { + kind: TokenKind::Operator, + value: "&&".into(), + offset: start, + }); + } else if chars.peek() == Some(&'>') { + byte_pos += chars.next().unwrap().len_utf8(); + let mut val = String::from("&>"); + if chars.peek() == Some(&'>') { + val.push('>'); + byte_pos += chars.next().unwrap().len_utf8(); + } + tokens.push(ParsedToken { + kind: TokenKind::Redirect, + value: val, + offset: start, + }); + } else { + tokens.push(ParsedToken { + kind: TokenKind::Shellism, + value: "&".into(), + offset: start, + }); + } + current_start = byte_pos; + } + '>' => { + let fd_prefix = + if !current.is_empty() && current.chars().all(|ch| ch.is_ascii_digit()) { + Some(std::mem::take(&mut current)) + } else { + flush_arg(&mut tokens, &mut current, current_start); + None + }; + let redir_start = if fd_prefix.is_some() { + current_start + } else { + byte_pos + }; + let mut val = fd_prefix.unwrap_or_default(); + val.push('>'); + byte_pos += char_len; + if chars.peek() == Some(&'>') { + val.push('>'); + byte_pos += chars.next().unwrap().len_utf8(); + } + if chars.peek() == Some(&'&') { + val.push('&'); + byte_pos += chars.next().unwrap().len_utf8(); + while chars + .peek() + .is_some_and(|&c| c.is_ascii_digit() || c == '-') + { + let nc = chars.next().unwrap(); + val.push(nc); + byte_pos += nc.len_utf8(); + } + } + tokens.push(ParsedToken { + kind: TokenKind::Redirect, + value: val, + offset: redir_start, + }); + current_start = byte_pos; + } + '<' => { + flush_arg(&mut tokens, &mut current, current_start); + let start = byte_pos; + let mut val = String::from("<"); + byte_pos += char_len; + if chars.peek() == Some(&'<') { + val.push('<'); + byte_pos += chars.next().unwrap().len_utf8(); + } + tokens.push(ParsedToken { + kind: TokenKind::Redirect, + value: val, + offset: start, + }); + current_start = byte_pos; + } + c if c.is_whitespace() => { + flush_arg(&mut tokens, &mut current, current_start); + byte_pos += c.len_utf8(); + current_start = byte_pos; + } + _ => { + if current.is_empty() { + current_start = byte_pos; + } + current.push(c); + byte_pos += char_len; + } + } + } + + if escaped { + current.push('\\'); + } + flush_arg(&mut tokens, &mut current, current_start); + tokens +} + +fn flush_arg(tokens: &mut Vec, current: &mut String, offset: usize) { + if !current.is_empty() { + tokens.push(ParsedToken { + kind: TokenKind::Arg, + value: std::mem::take(current), + offset, + }); + } +} + +#[allow(dead_code)] +pub fn strip_quotes(s: &str) -> String { + let chars: Vec = s.chars().collect(); + if chars.len() >= 2 + && ((chars[0] == '"' && chars[chars.len() - 1] == '"') + || (chars[0] == '\'' && chars[chars.len() - 1] == '\'')) + { + return chars[1..chars.len() - 1].iter().collect(); + } + s.to_string() +} + +pub fn shell_split(input: &str) -> Vec { + let mut tokens = Vec::new(); + let mut current = String::new(); + let mut chars = input.chars().peekable(); + let mut in_single = false; + let mut in_double = false; + + while let Some(c) = chars.next() { + match c { + '\\' if !in_single => { + if let Some(next) = chars.next() { + current.push(next); + } + } + '\'' if !in_double => { + in_single = !in_single; + } + '"' if !in_single => { + in_double = !in_double; + } + ' ' | '\t' if !in_single && !in_double => { + if !current.is_empty() { + tokens.push(std::mem::take(&mut current)); + } + } + _ => { + current.push(c); + } + } + } + + if !current.is_empty() { + tokens.push(current); + } + + tokens +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_simple_command() { + let tokens = tokenize("git status"); + assert_eq!(tokens.len(), 2); + assert_eq!(tokens[0].kind, TokenKind::Arg); + assert_eq!(tokens[0].value, "git"); + assert_eq!(tokens[1].value, "status"); + } + + #[test] + fn test_command_with_args() { + let tokens = tokenize("git commit -m message"); + assert_eq!(tokens.len(), 4); + assert_eq!(tokens[0].value, "git"); + assert_eq!(tokens[1].value, "commit"); + assert_eq!(tokens[2].value, "-m"); + assert_eq!(tokens[3].value, "message"); + } + + #[test] + fn test_quoted_operator_not_split() { + let tokens = tokenize(r#"git commit -m "Fix && Bug""#); + assert!(!tokens + .iter() + .any(|t| matches!(t.kind, TokenKind::Operator) && t.value == "&&")); + assert!(tokens.iter().any(|t| t.value.contains("Fix && Bug"))); + } + + #[test] + fn test_single_quoted_string() { + let tokens = tokenize("echo 'hello world'"); + assert!(tokens.iter().any(|t| t.value == "'hello world'")); + } + + #[test] + fn test_double_quoted_string() { + let tokens = tokenize(r#"echo "hello world""#); + assert!(tokens.iter().any(|t| t.value == "\"hello world\"")); + } + + #[test] + fn test_empty_quoted_string() { + let tokens = tokenize("echo \"\""); + assert!(tokens.iter().any(|t| t.value == "\"\"")); + } + + #[test] + fn test_nested_quotes() { + let tokens = tokenize(r#"echo "outer 'inner' outer""#); + assert!(tokens.iter().any(|t| t.value.contains("'inner'"))); + } + + #[test] + fn test_escaped_space() { + let tokens = tokenize("echo hello\\ world"); + assert!(tokens.iter().any(|t| t.value.contains("hello"))); + } + + #[test] + fn test_backslash_in_single_quotes() { + let tokens = tokenize(r#"echo 'hello\nworld'"#); + assert!(tokens.iter().any(|t| t.value.contains(r"\n"))); + } + + #[test] + fn test_escaped_quote_in_double() { + let tokens = tokenize(r#"echo "hello\"world""#); + assert!(tokens.iter().any(|t| t.value.contains("hello"))); + } + + #[test] + fn test_empty_input() { + assert!(tokenize("").is_empty()); + } + + #[test] + fn test_whitespace_only() { + assert!(tokenize(" ").is_empty()); + } + + #[test] + fn test_unclosed_single_quote() { + let tokens = tokenize("'unclosed"); + assert!(!tokens.is_empty()); + } + + #[test] + fn test_unclosed_double_quote() { + let tokens = tokenize("\"unclosed"); + assert!(!tokens.is_empty()); + } + + #[test] + fn test_unicode_preservation() { + let tokens = tokenize("echo \"héllo wörld\""); + assert!(tokens.iter().any(|t| t.value.contains("héllo"))); + } + + #[test] + fn test_multiple_spaces() { + let tokens = tokenize("git status"); + assert_eq!(tokens.len(), 2); + } + + #[test] + fn test_leading_trailing_spaces() { + let tokens = tokenize(" git status "); + assert_eq!(tokens.len(), 2); + } + + #[test] + fn test_and_operator() { + let tokens = tokenize("cmd1 && cmd2"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Operator && t.value == "&&")); + } + + #[test] + fn test_or_operator() { + let tokens = tokenize("cmd1 || cmd2"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Operator && t.value == "||")); + } + + #[test] + fn test_semicolon() { + let tokens = tokenize("cmd1 ; cmd2"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Operator && t.value == ";")); + } + + #[test] + fn test_multiple_and() { + let tokens = tokenize("a && b && c"); + let ops: Vec<_> = tokens + .iter() + .filter(|t| t.kind == TokenKind::Operator) + .collect(); + assert_eq!(ops.len(), 2); + } + + #[test] + fn test_mixed_operators() { + let tokens = tokenize("a && b || c"); + let ops: Vec<_> = tokens + .iter() + .filter(|t| t.kind == TokenKind::Operator) + .collect(); + assert_eq!(ops.len(), 2); + } + + #[test] + fn test_operator_at_start() { + let tokens = tokenize("&& cmd"); + assert!(tokens.iter().any(|t| t.value == "&&")); + } + + #[test] + fn test_operator_at_end() { + let tokens = tokenize("cmd &&"); + assert!(tokens.iter().any(|t| t.value == "&&")); + } + + #[test] + fn test_pipe_detection() { + let tokens = tokenize("cat file | grep pattern"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Pipe)); + } + + #[test] + fn test_quoted_pipe_not_pipe() { + let tokens = tokenize("\"a|b\""); + assert!(!tokens.iter().any(|t| t.kind == TokenKind::Pipe)); + } + + #[test] + fn test_multiple_pipes() { + let tokens = tokenize("a | b | c"); + let pipes: Vec<_> = tokens + .iter() + .filter(|t| t.kind == TokenKind::Pipe) + .collect(); + assert_eq!(pipes.len(), 2); + } + + #[test] + fn test_glob_detection() { + let tokens = tokenize("ls *.rs"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Shellism)); + } + + #[test] + fn test_quoted_glob_not_shellism() { + let tokens = tokenize("echo \"*.txt\""); + assert!(!tokens.iter().any(|t| t.kind == TokenKind::Shellism)); + } + + #[test] + fn test_simple_var_is_arg() { + let tokens = tokenize("echo $HOME"); + assert!( + tokens + .iter() + .any(|t| t.kind == TokenKind::Arg && t.value == "$HOME"), + "Simple $VAR must be Arg — shell expands at execution time" + ); + assert!( + !tokens.iter().any(|t| t.kind == TokenKind::Shellism), + "No Shellism expected for simple $VAR" + ); + } + + #[test] + fn test_simple_var_enables_native_routing() { + let tokens = tokenize("git log $BRANCH"); + assert!( + !tokens.iter().any(|t| t.kind == TokenKind::Shellism), + "git log $BRANCH must have no Shellism" + ); + } + + #[test] + fn test_dollar_subshell_stays_shellism() { + let tokens = tokenize("echo $(date)"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Shellism)); + } + + #[test] + fn test_dollar_brace_stays_shellism() { + let tokens = tokenize("echo ${HOME}"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Shellism)); + } + + #[test] + fn test_dollar_special_vars_stay_shellism() { + for s in &["echo $?", "echo $$", "echo $!"] { + let tokens = tokenize(s); + assert!( + tokens.iter().any(|t| t.kind == TokenKind::Shellism), + "{} should produce Shellism", + s + ); + } + } + + #[test] + fn test_dollar_digit_stays_shellism() { + let tokens = tokenize("echo $1"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Shellism)); + } + + #[test] + fn test_quoted_variable_not_shellism() { + let tokens = tokenize("echo \"$HOME\""); + assert!(!tokens.iter().any(|t| t.kind == TokenKind::Shellism)); + } + + #[test] + fn test_backtick_substitution() { + let tokens = tokenize("echo `date`"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Shellism)); + } + + #[test] + fn test_subshell_detection() { + let tokens = tokenize("echo $(date)"); + let shellisms: Vec<_> = tokens + .iter() + .filter(|t| t.kind == TokenKind::Shellism) + .collect(); + assert!(!shellisms.is_empty()); + } + + #[test] + fn test_brace_expansion() { + let tokens = tokenize("echo {a,b}.txt"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Shellism)); + } + + #[test] + fn test_escaped_glob() { + let tokens = tokenize("echo \\*.txt"); + assert!(!tokens + .iter() + .any(|t| t.kind == TokenKind::Shellism && t.value == "*")); + } + + #[test] + fn test_redirect_out() { + let tokens = tokenize("cmd > file"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Redirect)); + } + + #[test] + fn test_redirect_append() { + let tokens = tokenize("cmd >> file"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == ">>")); + } + + #[test] + fn test_redirect_in() { + let tokens = tokenize("cmd < file"); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Redirect)); + } + + #[test] + fn test_redirect_stderr() { + let tokens = tokenize("cmd 2> file"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value.starts_with("2>"))); + } + + #[test] + fn test_redirect_stderr_no_space() { + let tokens = tokenize("cmd 2>/dev/null"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == "2>")); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Arg && t.value == "/dev/null")); + } + + #[test] + fn test_redirect_dev_null() { + let tokens = tokenize("cmd > /dev/null"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == ">")); + } + + #[test] + fn test_redirect_2_to_1_single_token() { + let tokens = tokenize("cmd 2>&1"); + assert_eq!(tokens.len(), 2); + assert_eq!(tokens[1].kind, TokenKind::Redirect); + assert_eq!(tokens[1].value, "2>&1"); + assert!(!tokens + .iter() + .any(|t| t.kind == TokenKind::Shellism && t.value == "&")); + } + + #[test] + fn test_redirect_1_to_2_single_token() { + let tokens = tokenize("cmd 1>&2"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == "1>&2")); + } + + #[test] + fn test_redirect_fd_close() { + let tokens = tokenize("cmd 2>&-"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == "2>&-")); + } + + #[test] + fn test_redirect_shorthand_dup() { + let tokens = tokenize("cmd >&2"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == ">&2")); + } + + #[test] + fn test_redirect_amp_gt() { + let tokens = tokenize("cmd &>/dev/null"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == "&>")); + } + + #[test] + fn test_redirect_amp_gt_gt() { + let tokens = tokenize("cmd &>>/dev/null"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == "&>>")); + } + + #[test] + fn test_combined_redirect_chain() { + let tokens = tokenize("cmd > /dev/null 2>&1"); + let redirects: Vec<_> = tokens + .iter() + .filter(|t| t.kind == TokenKind::Redirect) + .collect(); + assert_eq!(redirects.len(), 2); + assert_eq!(redirects[0].value, ">"); + assert_eq!(redirects[1].value, "2>&1"); + } + + #[test] + fn test_redirect_append_to_file() { + let tokens = tokenize("echo hello >> /tmp/output.txt"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == ">>")); + } + + #[test] + fn test_redirect_heredoc_marker() { + let tokens = tokenize("cat <&1 | head"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == "2>&1")); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Pipe)); + } + + #[test] + fn test_redirect_2_to_1_with_and() { + let tokens = tokenize("cargo test 2>&1 && echo done"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == "2>&1")); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Operator && t.value == "&&")); + } + + #[test] + fn test_exclamation_is_shellism() { + let tokens = tokenize("if ! grep -q pattern file; then echo missing; fi"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Shellism && t.value == "!")); + } + + #[test] + fn test_background_job_is_shellism() { + let tokens = tokenize("sleep 10 &"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Shellism && t.value == "&")); + } + + #[test] + fn test_background_not_confused_with_amp_redirect() { + let tokens = tokenize("cargo test &>/dev/null"); + assert!(!tokens + .iter() + .any(|t| t.kind == TokenKind::Shellism && t.value == "&")); + assert!(tokens.iter().any(|t| t.kind == TokenKind::Redirect)); + } + + #[test] + fn test_semicolon_no_space() { + let tokens = tokenize("git status;cargo test"); + assert_eq!( + tokens + .iter() + .filter(|t| t.kind == TokenKind::Operator) + .count(), + 1 + ); + assert_eq!( + tokens.iter().filter(|t| t.kind == TokenKind::Arg).count(), + 4 + ); + } + + #[test] + fn test_offset_tracking() { + let tokens = tokenize("a && b"); + assert_eq!(tokens[0].offset, 0); + assert_eq!(tokens[1].offset, 2); + assert_eq!(tokens[2].offset, 5); + } + + #[test] + fn test_offset_segment_extraction() { + let cmd = "git add . && cargo test"; + let tokens = tokenize(cmd); + let op = tokens + .iter() + .find(|t| t.kind == TokenKind::Operator) + .unwrap(); + let left = cmd[..op.offset].trim(); + let right_start = op.offset + op.value.len(); + let right = cmd[right_start..].trim(); + assert_eq!(left, "git add ."); + assert_eq!(right, "cargo test"); + } + + #[test] + fn test_env_prefix_is_arg() { + let tokens = tokenize("GIT_SSH_COMMAND=ssh git push"); + assert_eq!(tokens[0].kind, TokenKind::Arg); + assert_eq!(tokens[0].value, "GIT_SSH_COMMAND=ssh"); + } + + #[test] + fn test_complex_compound() { + let tokens = tokenize("cargo fmt --all && cargo clippy --all-targets && cargo test"); + let operators: Vec<_> = tokens + .iter() + .filter(|t| t.kind == TokenKind::Operator) + .collect(); + assert_eq!(operators.len(), 2); + assert!(operators.iter().all(|t| t.value == "&&")); + } + + #[test] + fn test_find_pipe_xargs() { + let tokens = tokenize("find . -name '*.rs' | xargs grep 'fn run'"); + let pipe_idx = tokens + .iter() + .position(|t| t.kind == TokenKind::Pipe) + .unwrap(); + assert!(pipe_idx > 0); + let before_pipe: Vec<_> = tokens[..pipe_idx] + .iter() + .filter(|t| t.kind == TokenKind::Arg) + .collect(); + assert!(before_pipe.iter().any(|t| t.value == "find")); + } + + #[test] + fn test_fd_redirect_needs_adjacent_digit() { + let tokens = tokenize("echo 2 > file"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Arg && t.value == "2")); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == ">")); + } + + #[test] + fn test_fd_redirect_no_space() { + let tokens = tokenize("echo 2>file"); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Redirect && t.value == "2>")); + assert!(tokens + .iter() + .any(|t| t.kind == TokenKind::Arg && t.value == "file")); + } + + #[test] + fn test_strip_quotes_double() { + assert_eq!(strip_quotes("\"hello\""), "hello"); + } + + #[test] + fn test_strip_quotes_single() { + assert_eq!(strip_quotes("'hello'"), "hello"); + } + + #[test] + fn test_strip_quotes_none() { + assert_eq!(strip_quotes("hello"), "hello"); + } + + #[test] + fn test_strip_quotes_mismatched() { + assert_eq!(strip_quotes("\"hello'"), "\"hello'"); + } + + #[test] + fn test_shell_split_simple() { + assert_eq!( + shell_split("head -50 file.php"), + vec!["head", "-50", "file.php"] + ); + } + + #[test] + fn test_shell_split_double_quotes() { + assert_eq!( + shell_split(r#"git log --format="%H %s""#), + vec!["git", "log", "--format=%H %s"] + ); + } + + #[test] + fn test_shell_split_single_quotes() { + assert_eq!( + shell_split("grep -r 'hello world' ."), + vec!["grep", "-r", "hello world", "."] + ); + } + + #[test] + fn test_shell_split_single_word() { + assert_eq!(shell_split("ls"), vec!["ls"]); + } + + #[test] + fn test_shell_split_empty() { + let result: Vec = shell_split(""); + assert!(result.is_empty()); + } + + #[test] + fn test_shell_split_backslash_escape() { + assert_eq!( + shell_split(r"echo hello\ world"), + vec!["echo", "hello world"] + ); + } + + #[test] + fn test_shell_split_unclosed_quote() { + let result = shell_split("echo 'hello"); + assert_eq!(result, vec!["echo", "hello"]); + } + + #[test] + fn test_shell_split_mixed_quotes() { + assert_eq!( + shell_split(r#"echo "it's" 'a "test"'"#), + vec!["echo", "it's", "a \"test\""] + ); + } + + #[test] + fn test_shell_split_tabs() { + assert_eq!(shell_split("a\tb\tc"), vec!["a", "b", "c"]); + } + + #[test] + fn test_shell_split_multiple_spaces() { + assert_eq!(shell_split("a b c"), vec!["a", "b", "c"]); + } +} diff --git a/src/discover/mod.rs b/src/discover/mod.rs index 4be67a50..ada51f8e 100644 --- a/src/discover/mod.rs +++ b/src/discover/mod.rs @@ -1,5 +1,6 @@ //! Scans AI coding sessions to find commands that could benefit from RTK filtering. +pub mod lexer; pub mod provider; pub mod registry; mod report; diff --git a/src/discover/registry.rs b/src/discover/registry.rs index 44609b0d..53d3967a 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -3,6 +3,7 @@ use lazy_static::lazy_static; use regex::{Regex, RegexSet}; +use super::lexer::{tokenize, TokenKind}; use super::rules::{IGNORED_EXACT, IGNORED_PREFIXES, RULES}; /// Result of classifying a command. @@ -49,12 +50,24 @@ lazy_static! { .iter() .map(|r| Regex::new(r.pattern).expect("invalid regex")) .collect(); - static ref ENV_PREFIX: Regex = - Regex::new(r"^(?:sudo\s+|env\s+|[A-Z_][A-Z0-9_]*=[^\s]*\s+)+").unwrap(); + static ref ENV_PREFIX: Regex = { + let double_quoted = r#""(?:[^"\\]|\\.)*""#; + let single_quoted = r#"'(?:[^'\\]|\\.)*'"#; + let unquoted = r#"[^\s]*"#; + let env_value = format!("(?:{}|{}|{})", double_quoted, single_quoted, unquoted); + let env_assign = format!(r#"[A-Z_][A-Z0-9_]*={}"#, env_value); + Regex::new(&format!(r#"^(?:sudo\s+|env\s+|{}\s+)+"#, env_assign)).unwrap() + }; // Git global options that appear before the subcommand: -C , -c , // --git-dir , --work-tree , and flag-only options (#163) static ref GIT_GLOBAL_OPT: Regex = Regex::new(r"^(?:(?:-C\s+\S+|-c\s+\S+|--git-dir(?:=\S+|\s+\S+)|--work-tree(?:=\S+|\s+\S+)|--no-pager|--no-optional-locks|--bare|--literal-pathspecs)\s+)+").unwrap(); + static ref HEAD_N: Regex = Regex::new(r"^head\s+-(\d+)\s+(.+)$").unwrap(); + static ref HEAD_LINES: Regex = Regex::new(r"^head\s+--lines=(\d+)\s+(.+)$").unwrap(); + static ref TAIL_N: Regex = Regex::new(r"^tail\s+-(\d+)\s+(.+)$").unwrap(); + static ref TAIL_N_SPACE: Regex = Regex::new(r"^tail\s+-n\s+(\d+)\s+(.+)$").unwrap(); + static ref TAIL_LINES_EQ: Regex = Regex::new(r"^tail\s+--lines=(\d+)\s+(.+)$").unwrap(); + static ref TAIL_LINES_SPACE: Regex = Regex::new(r"^tail\s+--lines\s+(\d+)\s+(.+)$").unwrap(); } /// Classify a single (already-split) command. @@ -190,86 +203,43 @@ fn extract_base_command(cmd: &str) -> &str { } } -/// Split a command chain on `&&`, `||`, `;` outside quotes. -/// For pipes `|`, only keep the first command. -/// Lines with `<<` (heredoc) or `$((` are returned whole. pub fn split_command_chain(cmd: &str) -> Vec<&str> { let trimmed = cmd.trim(); if trimmed.is_empty() { return vec![]; } - // Heredoc or arithmetic expansion: treat as single command if trimmed.contains("<<") || trimmed.contains("$((") { return vec![trimmed]; } + let tokens = tokenize(trimmed); let mut results = Vec::new(); - let mut start = 0; - let bytes = trimmed.as_bytes(); - let len = bytes.len(); - let mut i = 0; - let mut in_single = false; - let mut in_double = false; - let mut pipe_seen = false; - - while i < len { - let b = bytes[i]; - match b { - b'\'' if !in_double => { - in_single = !in_single; - i += 1; - } - b'"' if !in_single => { - in_double = !in_double; - i += 1; - } - b'|' if !in_single && !in_double => { - if i + 1 < len && bytes[i + 1] == b'|' { - // || - let segment = trimmed[start..i].trim(); - if !segment.is_empty() { - results.push(segment); - } - i += 2; - start = i; - } else { - // pipe: keep only first command - let segment = trimmed[start..i].trim(); - if !segment.is_empty() { - results.push(segment); - } - pipe_seen = true; - break; - } - } - b'&' if !in_single && !in_double && i + 1 < len && bytes[i + 1] == b'&' => { - let segment = trimmed[start..i].trim(); + let mut seg_start: usize = 0; + + for tok in &tokens { + match tok.kind { + TokenKind::Operator => { + let segment = trimmed[seg_start..tok.offset].trim(); if !segment.is_empty() { results.push(segment); } - i += 2; - start = i; + seg_start = tok.offset + tok.value.len(); } - b';' if !in_single && !in_double => { - let segment = trimmed[start..i].trim(); + TokenKind::Pipe => { + let segment = trimmed[seg_start..tok.offset].trim(); if !segment.is_empty() { results.push(segment); } - i += 1; - start = i; - } - _ => { - i += 1; + return results; } + _ => {} } } - if !pipe_seen && start < len { - let segment = trimmed[start..].trim(); - if !segment.is_empty() { - results.push(segment); - } + let segment = trimmed[seg_start..].trim(); + if !segment.is_empty() { + results.push(segment); } results @@ -330,32 +300,40 @@ pub fn strip_disabled_prefix(cmd: &str) -> &str { trimmed[prefix_len..].trim_start() } -lazy_static! { - // Match trailing shell redirections: - // Alt 1: N>&M or N>&- (fd redirect/close): 2>&1, 1>&2, 2>&- - // Alt 2: &>file or &>>file (bash redirect both): &>/dev/null - // Alt 3: N>file or N>>file (fd to file): 2>/dev/null, >/tmp/out, 1>>log - // Note: [^(\\s] excludes process substitutions like >(tee) from false-positive matching - static ref TRAILING_REDIRECT: Regex = - Regex::new(r"\s+(?:[0-9]?>&[0-9-]|&>>?\S+|[0-9]?>>?\s*[^(\s]\S*)\s*$").unwrap(); -} - -/// Strip trailing stderr/stdout redirects from a command segment (#530). -/// Returns (command_without_redirects, redirect_suffix). fn strip_trailing_redirects(cmd: &str) -> (&str, &str) { - if let Some(m) = TRAILING_REDIRECT.find(cmd) { - // Verify redirect is not inside quotes (single-pass count) - let before = &cmd[..m.start()]; - let (sq, dq) = before.chars().fold((0u32, 0u32), |(s, d), c| match c { - '\'' => (s + 1, d), - '"' => (s, d + 1), - _ => (s, d), - }); - if sq % 2 == 0 && dq % 2 == 0 { - return (&cmd[..m.start()], &cmd[m.start()..]); + let tokens = tokenize(cmd); + if tokens.is_empty() { + return (cmd, ""); + } + + let mut redir_boundary = tokens.len(); + let mut i = tokens.len(); + while i > 0 { + i -= 1; + match tokens[i].kind { + TokenKind::Redirect => { + redir_boundary = i; + } + TokenKind::Arg => { + if i > 0 && tokens[i - 1].kind == TokenKind::Redirect { + redir_boundary = i - 1; + i -= 1; + } else { + break; + } + } + _ => break, } } - (cmd, "") + + if redir_boundary >= tokens.len() { + return (cmd, ""); + } + + let cut = tokens[redir_boundary].offset; + let cmd_part = cmd[..cut].trim_end(); + let redir_part = &cmd[cmd_part.len()..]; + (cmd_part, redir_part) } /// Returns `None` if the command is unsupported or ignored (hook should pass through). @@ -390,131 +368,73 @@ pub fn rewrite_command(cmd: &str, excluded: &[String]) -> Option { /// Rewrite a compound command (with `&&`, `||`, `;`, `|`) by rewriting each segment. fn rewrite_compound(cmd: &str, excluded: &[String]) -> Option { - let bytes = cmd.as_bytes(); - let len = bytes.len(); - let mut result = String::with_capacity(len + 32); + let tokens = tokenize(cmd); + let mut result = String::with_capacity(cmd.len() + 32); let mut any_changed = false; - let mut seg_start = 0; - let mut i = 0; - let mut in_single = false; - let mut in_double = false; - - while i < len { - let b = bytes[i]; - match b { - b'\'' if !in_double => { - in_single = !in_single; - i += 1; - } - b'"' if !in_single => { - in_double = !in_double; - i += 1; - } - b'|' if !in_single && !in_double => { - if i + 1 < len && bytes[i + 1] == b'|' { - // `||` operator — rewrite left, continue - let seg = cmd[seg_start..i].trim(); - let rewritten = - rewrite_segment(seg, excluded).unwrap_or_else(|| seg.to_string()); - if rewritten != seg { - any_changed = true; - } - result.push_str(&rewritten); - result.push_str(" || "); - i += 2; - while i < len && bytes[i] == b' ' { - i += 1; - } - seg_start = i; - } else { - // `|` pipe — rewrite first segment only, pass through the rest unchanged - let seg = cmd[seg_start..i].trim(); - // Skip rewriting `find`/`fd` in pipes — rtk find outputs a grouped - // format that is incompatible with pipe consumers like xargs, grep, - // wc, sort, etc. which expect one path per line (#439). - let is_pipe_incompatible = seg.starts_with("find ") - || seg == "find" - || seg.starts_with("fd ") - || seg == "fd"; - let rewritten = if is_pipe_incompatible { - seg.to_string() - } else { - rewrite_segment(seg, excluded).unwrap_or_else(|| seg.to_string()) - }; - if rewritten != seg { - any_changed = true; - } - result.push_str(&rewritten); - // Preserve the space before the pipe that was lost by trim() - result.push(' '); - result.push_str(cmd[i..].trim_start()); - return if any_changed { Some(result) } else { None }; - } - } - b'&' if !in_single && !in_double && i + 1 < len && bytes[i + 1] == b'&' => { - // `&&` operator — rewrite left, continue - let seg = cmd[seg_start..i].trim(); + let mut seg_start: usize = 0; + + for tok in &tokens { + match tok.kind { + TokenKind::Operator => { + let seg = cmd[seg_start..tok.offset].trim(); let rewritten = rewrite_segment(seg, excluded).unwrap_or_else(|| seg.to_string()); if rewritten != seg { any_changed = true; } result.push_str(&rewritten); - result.push_str(" && "); - i += 2; - while i < len && bytes[i] == b' ' { - i += 1; + if tok.value == ";" { + result.push(';'); + let after = tok.offset + tok.value.len(); + if after < cmd.len() { + result.push(' '); + } + } else { + result.push(' '); + result.push_str(&tok.value); + result.push(' '); + } + seg_start = tok.offset + tok.value.len(); + while seg_start < cmd.len() && cmd.as_bytes().get(seg_start) == Some(&b' ') { + seg_start += 1; } - seg_start = i; } - b'&' if !in_single && !in_double => { - // #346: redirect detection — 2>&1 / >&2 (> before &) or &>file / &>>file (> after &) - let is_redirect = - (i > 0 && bytes[i - 1] == b'>') || (i + 1 < len && bytes[i + 1] == b'>'); - if is_redirect { - i += 1; + TokenKind::Pipe => { + let seg = cmd[seg_start..tok.offset].trim(); + let is_pipe_incompatible = seg.starts_with("find ") + || seg == "find" + || seg.starts_with("fd ") + || seg == "fd"; + let rewritten = if is_pipe_incompatible { + seg.to_string() } else { - // single `&` background execution operator - let seg = cmd[seg_start..i].trim(); - let rewritten = - rewrite_segment(seg, excluded).unwrap_or_else(|| seg.to_string()); - if rewritten != seg { - any_changed = true; - } - result.push_str(&rewritten); - result.push_str(" & "); - i += 1; - while i < len && bytes[i] == b' ' { - i += 1; - } - seg_start = i; + rewrite_segment(seg, excluded).unwrap_or_else(|| seg.to_string()) + }; + if rewritten != seg { + any_changed = true; } + result.push_str(&rewritten); + result.push(' '); + result.push_str(cmd[tok.offset..].trim_start()); + return if any_changed { Some(result) } else { None }; } - b';' if !in_single && !in_double => { - // `;` separator - let seg = cmd[seg_start..i].trim(); + TokenKind::Shellism if tok.value == "&" => { + let seg = cmd[seg_start..tok.offset].trim(); let rewritten = rewrite_segment(seg, excluded).unwrap_or_else(|| seg.to_string()); if rewritten != seg { any_changed = true; } result.push_str(&rewritten); - result.push(';'); - i += 1; - while i < len && bytes[i] == b' ' { - i += 1; - } - if i < len { - result.push(' '); + result.push_str(" & "); + seg_start = tok.offset + tok.value.len(); + while seg_start < cmd.len() && cmd.as_bytes().get(seg_start) == Some(&b' ') { + seg_start += 1; } - seg_start = i; - } - _ => { - i += 1; } + _ => {} } } - // Last (or only) segment - let seg = cmd[seg_start..len].trim(); + let seg = cmd[seg_start..].trim(); let rewritten = rewrite_segment(seg, excluded).unwrap_or_else(|| seg.to_string()); if rewritten != seg { any_changed = true; @@ -528,45 +448,17 @@ fn rewrite_compound(cmd: &str, excluded: &[String]) -> Option { } } -/// Rewrite `head -N file` → `rtk read file --max-lines N`. -/// Returns `None` if the command doesn't match this pattern (fall through to generic logic). -fn rewrite_head_numeric(cmd: &str) -> Option { - // Match: head - (with optional env prefix) - lazy_static! { - static ref HEAD_N: Regex = Regex::new(r"^head\s+-(\d+)\s+(.+)$").expect("valid regex"); - static ref HEAD_LINES: Regex = - Regex::new(r"^head\s+--lines=(\d+)\s+(.+)$").expect("valid regex"); - } - if let Some(caps) = HEAD_N.captures(cmd) { - let n = caps.get(1)?.as_str(); - let file = caps.get(2)?.as_str(); - return Some(format!("rtk read {} --max-lines {}", file, n)); - } - if let Some(caps) = HEAD_LINES.captures(cmd) { - let n = caps.get(1)?.as_str(); - let file = caps.get(2)?.as_str(); - return Some(format!("rtk read {} --max-lines {}", file, n)); - } - // head with any other flag (e.g. -c, -q): skip rewriting to avoid clap errors +fn rewrite_line_range(cmd: &str) -> Option { + for re in [&*HEAD_N, &*HEAD_LINES] { + if let Some(caps) = re.captures(cmd) { + let n = caps.get(1)?.as_str(); + let file = caps.get(2)?.as_str(); + return Some(format!("rtk read {} --max-lines {}", file, n)); + } + } if cmd.starts_with("head -") { return None; } - None -} - -/// Rewrite `tail` numeric line forms to `rtk read ... --tail-lines N`. -/// Returns `None` when the pattern is unsupported (caller falls through / skips rewrite). -fn rewrite_tail_lines(cmd: &str) -> Option { - lazy_static! { - static ref TAIL_N: Regex = Regex::new(r"^tail\s+-(\d+)\s+(.+)$").expect("valid regex"); - static ref TAIL_N_SPACE: Regex = - Regex::new(r"^tail\s+-n\s+(\d+)\s+(.+)$").expect("valid regex"); - static ref TAIL_LINES_EQ: Regex = - Regex::new(r"^tail\s+--lines=(\d+)\s+(.+)$").expect("valid regex"); - static ref TAIL_LINES_SPACE: Regex = - Regex::new(r"^tail\s+--lines\s+(\d+)\s+(.+)$").expect("valid regex"); - } - for re in [ &*TAIL_N, &*TAIL_N_SPACE, @@ -579,8 +471,6 @@ fn rewrite_tail_lines(cmd: &str) -> Option { return Some(format!("rtk read {} --tail-lines {}", file, n)); } } - - // Unknown tail form: skip rewrite to preserve native behavior. None } @@ -602,18 +492,8 @@ fn rewrite_segment(seg: &str, excluded: &[String]) -> Option { return Some(trimmed.to_string()); } - // Special case: `head -N file` / `head --lines=N file` → `rtk read file --max-lines N` - // Must intercept before generic prefix replacement, which would produce `rtk read -20 file`. - // Only intercept when head has a flag (-N, --lines=N, -c, etc.); plain `head file` falls - // through to the generic rewrite below and produces `rtk read file` as expected. - if cmd_part.starts_with("head -") { - return rewrite_head_numeric(cmd_part).map(|r| format!("{}{}", r, redirect_suffix)); - } - - // tail has several forms that are not compatible with generic prefix replacement. - // Only rewrite recognized numeric line forms; otherwise skip rewrite. - if cmd_part.starts_with("tail ") { - return rewrite_tail_lines(cmd_part).map(|r| format!("{}{}", r, redirect_suffix)); + if cmd_part.starts_with("head -") || cmd_part.starts_with("tail ") { + return rewrite_line_range(cmd_part).map(|r| format!("{}{}", r, redirect_suffix)); } // Most cat flags (-v, -A, -e, -t, -s, -b, --show-all, etc.) have different @@ -1295,6 +1175,54 @@ mod tests { ); } + #[test] + fn test_rewrite_env_quoted_value_with_spaces() { + assert_eq!( + rewrite_command( + r#"GIT_SSH_COMMAND="ssh -o StrictHostKeyChecking=no" git push"#, + &[] + ), + Some(r#"GIT_SSH_COMMAND="ssh -o StrictHostKeyChecking=no" rtk git push"#.into()) + ); + } + + #[test] + fn test_rewrite_env_single_quoted_value_with_spaces() { + assert_eq!( + rewrite_command("EDITOR='vim -u NONE' git commit", &[]), + Some("EDITOR='vim -u NONE' rtk git commit".into()) + ); + } + + #[test] + fn test_rewrite_env_quoted_plus_unquoted() { + assert_eq!( + rewrite_command(r#"FOO="bar baz" BAR=1 git status"#, &[]), + Some(r#"FOO="bar baz" BAR=1 rtk git status"#.into()) + ); + } + + #[test] + fn test_rewrite_env_escaped_quotes_in_value() { + assert_eq!( + rewrite_command(r#"FOO="he said \"hello\"" git status"#, &[]), + Some(r#"FOO="he said \"hello\"" rtk git status"#.into()) + ); + } + + #[test] + fn test_classify_env_quoted_value_stripped() { + assert_eq!( + classify_command(r#"GIT_SSH_COMMAND="ssh -o StrictHostKeyChecking=no" git push"#), + Classification::Supported { + rtk_equivalent: "rtk git", + category: "Git", + estimated_savings_pct: 70.0, + status: RtkStatus::Existing, + } + ); + } + // --- #346: 2>&1 and &> redirect detection --- #[test] diff --git a/src/main.rs b/src/main.rs index c8be0aca..77a4be8c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1199,28 +1199,7 @@ enum GtCommands { /// Split a string into shell-like tokens, respecting single and double quotes. /// e.g. `git log --format="%H %s"` → ["git", "log", "--format=%H %s"] fn shell_split(input: &str) -> Vec { - let mut tokens = Vec::new(); - let mut current = String::new(); - let chars = input.chars(); - let mut in_single = false; - let mut in_double = false; - - for c in chars { - match c { - '\'' if !in_double => in_single = !in_single, - '"' if !in_single => in_double = !in_double, - ' ' | '\t' if !in_single && !in_double => { - if !current.is_empty() { - tokens.push(std::mem::take(&mut current)); - } - } - _ => current.push(c), - } - } - if !current.is_empty() { - tokens.push(current); - } - tokens + discover::lexer::shell_split(input) } fn main() { From 0a8b8fd0693fa504f376146cbbcafe9ddf4632c8 Mon Sep 17 00:00:00 2001 From: Adrien Eppling Date: Mon, 30 Mar 2026 18:13:39 +0200 Subject: [PATCH 7/7] fix(review): address PR #910 review feedback - container.rs: warn on kubectl JSON parse failure instead of silent fallback - runner.rs: fix savings inflation when filter_stdout_only by tracking stdout only - README.md: document automod behavior in module registration step --- src/cmds/README.md | 2 +- src/cmds/cloud/container.rs | 5 ++++- src/core/runner.rs | 7 ++++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/cmds/README.md b/src/cmds/README.md index f48119d5..5e0f633b 100644 --- a/src/cmds/README.md +++ b/src/cmds/README.md @@ -189,7 +189,7 @@ Adding a new filter or command requires changes in multiple places. For TOML-vs- - Add `.tee("label")` when the filter parses structured output (enables raw output recovery on failure) - **Exit codes**: handled automatically by `run_filtered()` — just return its result 2. **Register module**: - - Add `pub mod mycmd_cmd;` to the ecosystem's `mod.rs` + - Ecosystem `mod.rs` files use `automod::dir!()` — any `.rs` file in the directory becomes a public module automatically. No manual `pub mod` needed, but be aware: WIP or helper files will also be exposed. Only commit command-ready modules. - Add variant to `Commands` enum in `main.rs` with `#[arg(trailing_var_arg = true, allow_hyphen_values = true)]` - Add routing match arm in `main.rs`: `Commands::Mycmd { args } => mycmd_cmd::run(&args, cli.verbose)?,` 3. **Add rewrite pattern** — Entry in `src/discover/rules.rs` (PATTERNS + RULES arrays at matching index) so hooks auto-rewrite the command diff --git a/src/cmds/cloud/container.rs b/src/cmds/cloud/container.rs index 8de75649..25027a37 100644 --- a/src/cmds/cloud/container.rs +++ b/src/cmds/cloud/container.rs @@ -39,7 +39,10 @@ where label, |stdout| match serde_json::from_str::(stdout) { Ok(json) => filter_fn(&json), - Err(_) => stdout.to_string(), + Err(e) => { + eprintln!("[rtk] kubectl: JSON parse failed: {}", e); + stdout.to_string() + } }, RunOptions::stdout_only() .early_exit_on_failure() diff --git a/src/core/runner.rs b/src/core/runner.rs index 0370988c..3a048d6e 100644 --- a/src/core/runner.rs +++ b/src/core/runner.rs @@ -108,10 +108,15 @@ where eprintln!("{}", stderr.trim()); } + let raw_for_tracking = if opts.filter_stdout_only { + stdout.as_ref() + } else { + raw.as_str() + }; timer.track( &format!("{} {}", tool_name, args_display), &format!("rtk {} {}", tool_name, args_display), - &raw, + raw_for_tracking, &filtered, );