fix: code review issues · tomwaddington.dev/dathan@4aec9bd

+1 -1

Cargo.lock

··· 146 146 147 147 [[package]] 148 148 name = "dathan" 149 - version = "0.7.2" 149 + version = "0.7.3" 150 150 dependencies = [ 151 151 "anyhow", 152 152 "clap",

+4 -1

Cargo.toml

··· 1 1 [package] 2 2 name = "dathan" 3 - version = "0.7.2" 3 + version = "0.7.3" 4 4 edition = "2021" 5 5 description = "Highlight source code to HTML, Hiccup (EDN/JSON), or ANSI terminal output using Helix's tree-sitter grammars" 6 + license = "MIT" 7 + repository = "https://github.com/waddie/dathan" 8 + readme = "README.md" 6 9 7 10 [dependencies] 8 11 anyhow = "1"

+8 -6

README.md

··· 80 80 dathan src/main.rs 81 81 dathan --format html src/main.rs -o main.html 82 82 cat src/main.rs | dathan --lang rust 83 - dathan --emit-css --theme ~/source/helix/theme.toml -o theme.css 84 - dathan --format terminal --theme ~/source/helix/theme.toml src/main.rs 83 + dathan --emit-css --theme acid -o theme.css 84 + dathan --format terminal --theme ~/.config/helix/themes/acid.toml src/main.rs 85 85 dathan --theme acid src/main.rs 86 86 dathan --format html --inline src/main.rs -o main.html 87 87 ``` ··· 104 104 2. `~/.config/helix/runtime` 105 105 3. `$HELIX_RUNTIME` 106 106 107 - The language registry is read from `~/source/helix/languages.toml` (or 108 - `--languages`), with `~/.config/helix/languages.toml` merged over it by language 109 - name. 107 + The base language registry is `--languages` if given, otherwise a 108 + `languages.toml` next to the `$HELIX_RUNTIME` tree (as in a Helix source 109 + checkout), otherwise `~/.config/helix/languages.toml`. When the base comes from 110 + `--languages` or `$HELIX_RUNTIME`, `~/.config/helix/languages.toml` is merged 111 + over it by language name. 110 112 111 - Language detection uses the file extension, then `file-types` globs, then a 113 + Language detection uses `file-types` globs, then the file extension, then a 112 114 `#!` shebang line. Override with `--lang`. 113 115 114 116 ## Output

+138 -22

src/languages.rs

··· 172 172 self.by_name.get(name).copied() 173 173 } 174 174 175 - /// Detect by extension, then by glob/suffix `file-types` entries. 175 + /// Detect by glob/suffix `file-types` entries first (the longest matching 176 + /// pattern wins), then by extension, mirroring Helix's precedence. 176 177 pub fn language_for_filename(&self, path: &Path) -> Option<Language> { 177 - if let Some(ext) = path.extension().and_then(|e| e.to_str()) { 178 - if let Some(&lang) = self.by_extension.get(ext) { 179 - return Some(lang); 180 - } 181 - } 178 + let path_str = path.to_str().unwrap_or_default(); 182 179 let name = path 183 180 .file_name() 184 181 .and_then(|f| f.to_str()) 185 182 .unwrap_or_default(); 183 + 184 + // (pattern length, language index) of the best glob/suffix hit so far. 185 + let mut best: Option<(usize, usize)> = None; 186 186 for (i, data) in self.langs.iter().enumerate() { 187 187 for ft in &data.file_types { 188 188 if let FileType::Special(map) = ft { 189 - let hit = map.get("glob").is_some_and(|g| glob_match(g, name)) 190 - || map 191 - .get("suffix") 192 - .is_some_and(|s| name.ends_with(s.as_str())); 193 - if hit { 194 - return Some(Language::new( 195 - u32::try_from(i).expect("language index fits in u32"), 196 - )); 189 + let hit = map 190 + .get("glob") 191 + .filter(|g| glob_match(g, path_str)) 192 + .or_else(|| map.get("suffix").filter(|s| name.ends_with(s.as_str()))); 193 + if let Some(pattern) = hit { 194 + if best.is_none_or(|(len, _)| pattern.len() > len) { 195 + best = Some((pattern.len(), i)); 196 + } 197 197 } 198 198 } 199 199 } 200 200 } 201 - None 201 + if let Some((_, i)) = best { 202 + return Some(Language::new( 203 + u32::try_from(i).expect("language index fits in u32"), 204 + )); 205 + } 206 + 207 + path.extension() 208 + .and_then(|e| e.to_str()) 209 + .and_then(|ext| self.by_extension.get(ext).copied()) 202 210 } 203 211 204 212 /// Resolve an injected-language token: exact name first, then the longest ··· 266 274 String::from(slice) 267 275 } 268 276 269 - /// Minimal glob: supports a leading `*` wildcard (suffix match) or an exact 270 - /// filename match. Sufficient for the `file-types` globs Helix ships. 271 - fn glob_match(pattern: &str, name: &str) -> bool { 272 - match pattern.strip_prefix('*') { 273 - Some(suffix) => name.ends_with(suffix), 274 - None => pattern == name, 277 + /// Match a Helix `file-types` glob against a path, mirroring how Helix uses 278 + /// `globset` with default settings: `*` matches any characters, `/` included. 279 + /// A pattern starting with `/` is anchored to the whole path; any other 280 + /// pattern matches from a path component boundary (Helix prepends `*/`), plus 281 + /// from the start so a bare filename like `.gitignore` still matches. 282 + fn glob_match(pattern: &str, path: &str) -> bool { 283 + if pattern.starts_with('/') { 284 + return wildcard_match(pattern, path); 275 285 } 286 + std::iter::once(0) 287 + .chain(path.match_indices('/').map(|(i, _)| i + 1)) 288 + .any(|start| wildcard_match(pattern, &path[start..])) 289 + } 290 + 291 + /// Wildcard match where `*` matches any (possibly empty) run of characters 292 + /// and everything else is literal. Greedy with backtracking. Operates on 293 + /// bytes, which is UTF-8 safe because `*` is the only metacharacter and it 294 + /// matches arbitrary byte runs. 295 + fn wildcard_match(pattern: &str, text: &str) -> bool { 296 + let (p, t) = (pattern.as_bytes(), text.as_bytes()); 297 + let (mut pi, mut ti) = (0, 0); 298 + // Position after the most recent `*` and the text position it matched at. 299 + let mut star: Option<(usize, usize)> = None; 300 + while ti < t.len() { 301 + if pi < p.len() && p[pi] == b'*' { 302 + star = Some((pi + 1, ti)); 303 + pi += 1; 304 + } else if pi < p.len() && p[pi] == t[ti] { 305 + pi += 1; 306 + ti += 1; 307 + } else if let Some((sp, st)) = star { 308 + // Backtrack: let the last `*` swallow one more byte. 309 + pi = sp; 310 + ti = st + 1; 311 + star = Some((sp, st + 1)); 312 + } else { 313 + return false; 314 + } 315 + } 316 + p[pi..].iter().all(|&b| b == b'*') 276 317 } 277 318 278 319 /// Deep-merge an `overlay` `languages.toml` onto a `base`, the way Helix merges ··· 334 375 335 376 #[test] 336 377 fn glob_matching() { 337 - assert!(glob_match("*.toml", "Cargo.toml")); 378 + // Bare filename patterns, at any depth or as the whole path. 338 379 assert!(glob_match("Makefile", "Makefile")); 380 + assert!(glob_match("Makefile", "src/Makefile")); 339 381 assert!(!glob_match("Makefile", "makefile")); 382 + assert!(!glob_match("Makefile", "notMakefile")); 383 + // `*` anywhere, including patterns Helix ships. 384 + assert!(glob_match("*.toml", "dir/Cargo.toml")); 385 + assert!(glob_match(".*ignore", ".gitignore")); 386 + assert!(glob_match(".env.*", "repo/.env.local")); 387 + assert!(glob_match("_environment-*", "_environment-prod")); 340 388 assert!(!glob_match("*.rs", "main.py")); 389 + // Path-qualified patterns match trailing components. 390 + assert!(glob_match(".git/config", "repo/.git/config")); 391 + assert!(glob_match(".git/config", ".git/config")); 392 + assert!(!glob_match(".git/config", "config")); 393 + assert!(glob_match( 394 + ".git/modules/*/config", 395 + ".git/modules/sub/config" 396 + )); 397 + // Absolute patterns anchor to the whole path. 398 + assert!(glob_match("/etc/hosts", "/etc/hosts")); 399 + assert!(!glob_match("/etc/hosts", "/home/x/etc/hosts")); 400 + } 401 + 402 + fn detection_loader() -> Loader { 403 + let config = r#" 404 + [[language]] 405 + name = "yaml" 406 + file-types = ["yml", "yaml"] 407 + 408 + [[language]] 409 + name = "docker-compose" 410 + grammar = "yaml" 411 + file-types = [{ glob = "docker-compose.yml" }, { glob = "docker-compose.yaml" }] 412 + 413 + [[language]] 414 + name = "gitignore" 415 + file-types = [{ glob = ".*ignore" }] 416 + 417 + [[language]] 418 + name = "git-config" 419 + file-types = [{ glob = ".git/config" }] 420 + "#; 421 + Loader::new( 422 + crate::runtime::Runtime::new(&[]), 423 + toml::from_str(config).unwrap(), 424 + ) 425 + .unwrap() 426 + } 427 + 428 + #[test] 429 + fn globs_take_precedence_over_extensions() { 430 + let loader = detection_loader(); 431 + let lang = |name: &str| loader.language_for_name(name).unwrap(); 432 + 433 + // The glob beats the `yml` extension, at any depth. 434 + assert_eq!( 435 + loader.language_for_filename(Path::new("docker-compose.yml")), 436 + Some(lang("docker-compose")) 437 + ); 438 + assert_eq!( 439 + loader.language_for_filename(Path::new("deploy/docker-compose.yml")), 440 + Some(lang("docker-compose")) 441 + ); 442 + // Extension fallback still works. 443 + assert_eq!( 444 + loader.language_for_filename(Path::new("ci.yml")), 445 + Some(lang("yaml")) 446 + ); 447 + // Wildcard and path-qualified globs. 448 + assert_eq!( 449 + loader.language_for_filename(Path::new(".gitignore")), 450 + Some(lang("gitignore")) 451 + ); 452 + assert_eq!( 453 + loader.language_for_filename(Path::new("repo/.git/config")), 454 + Some(lang("git-config")) 455 + ); 456 + assert_eq!(loader.language_for_filename(Path::new("config")), None); 341 457 } 342 458 343 459 #[test]

+25 -19

src/main.rs

··· 85 85 return write_output(cli.output.as_deref(), &css); 86 86 } 87 87 88 + if cli.inline && cli.format == Format::Terminal { 89 + return Err(anyhow!( 90 + "--inline applies only to the class-based formats (html, edn-hiccup, json-hiccup)" 91 + )); 92 + } 93 + 88 94 let file = cli.file.as_deref(); 89 95 let source = if let Some(path) = file { 90 96 std::fs::read_to_string(path).with_context(|| format!("reading {}", path.display()))? ··· 95 101 .context("reading from stdin")?; 96 102 buf 97 103 }; 98 - 99 - if cli.inline && cli.format == Format::Terminal { 100 - return Err(anyhow!( 101 - "--inline applies only to the class-based formats (html, edn-hiccup, json-hiccup)" 102 - )); 103 - } 104 104 105 105 let merged = load_languages(&cli)?; 106 106 ··· 145 145 }) 146 146 } 147 147 148 - /// Load the base `languages.toml` and merge the user config over it (by 149 - /// language name), mirroring how Helix layers user config on the default. 148 + /// Load the base `languages.toml` (`--languages`, else the `$HELIX_RUNTIME`- 149 + /// adjacent config, else the user config) and, unless the user config is itself 150 + /// the base, merge it over the base by language name, mirroring how Helix layers 151 + /// user config on the default. 150 152 fn load_languages(cli: &Cli) -> Result<toml::Value> { 151 153 let user_path = home_dir().map(|h| h.join(".config/helix/languages.toml")); 154 + 155 + // A full base `languages.toml` sits next to a Helix runtime tree (as in a 156 + // source checkout referenced by `$HELIX_RUNTIME`); the user config is an 157 + // overlay merged over it, so prefer the runtime-adjacent base when present. 158 + let env_base = std::env::var_os("HELIX_RUNTIME") 159 + .and_then(|rt| Path::new(&rt).parent().map(|p| p.join("languages.toml"))); 152 160 153 161 let base_path = cli 154 162 .languages 155 163 .clone() 156 - .or_else(|| { 157 - first_existing([ 158 - home_dir().map(|h| h.join("source/helix/languages.toml")), 159 - user_path.clone(), 160 - ]) 161 - }) 164 + .or_else(|| first_existing([env_base, user_path.clone()])) 162 165 .ok_or_else(|| anyhow!("no languages.toml found; pass --languages <path>"))?; 163 166 164 167 let base = std::fs::read_to_string(&base_path) ··· 195 198 let dirs = theme_dirs(&path, rt); 196 199 Theme::load(&path, &dirs) 197 200 } 198 - Err(_) => Ok(Theme::bundled_default()), 201 + // Only fall back when no theme was requested; a --theme that cannot 202 + // be resolved is an error. 203 + Err(_) if cli.theme.is_none() => Ok(Theme::bundled_default()), 204 + Err(e) => Err(e), 199 205 } 200 206 } 201 207 ··· 214 220 return first_existing(candidates) 215 221 .ok_or_else(|| anyhow!("theme not found by path or name: {}", theme.display())); 216 222 } 217 - let mut candidates = vec![home_dir().map(|h| h.join("source/helix/theme.toml"))]; 218 223 // A theme bundled alongside any runtime root's parent. 219 - for root in rt.roots() { 220 - candidates.push(root.parent().map(|p| p.join("theme.toml"))); 221 - } 224 + let candidates = rt 225 + .roots() 226 + .iter() 227 + .map(|root| root.parent().map(|p| p.join("theme.toml"))); 222 228 first_existing(candidates).ok_or_else(|| anyhow!("no theme.toml found; pass --theme <path>")) 223 229 } 224 230

+27 -3

src/runtime.rs

··· 75 75 #[cfg(test)] 76 76 mod tests { 77 77 use super::*; 78 + use std::ffi::OsString; 78 79 use std::fs; 79 80 80 81 /// Create `<root>/grammars/<marker>.txt` so `find_file` has something to hit, ··· 87 88 root 88 89 } 89 90 91 + /// Set an env var for the test, restoring the previous value on drop 92 + /// (including on panic) so other tests in the process are unaffected. 93 + struct EnvGuard { 94 + key: &'static str, 95 + prev: Option<OsString>, 96 + } 97 + 98 + impl EnvGuard { 99 + fn set(key: &'static str, value: impl AsRef<std::ffi::OsStr>) -> Self { 100 + let prev = std::env::var_os(key); 101 + std::env::set_var(key, value); 102 + Self { key, prev } 103 + } 104 + } 105 + 106 + impl Drop for EnvGuard { 107 + fn drop(&mut self) { 108 + match &self.prev { 109 + Some(v) => std::env::set_var(self.key, v), 110 + None => std::env::remove_var(self.key), 111 + } 112 + } 113 + } 114 + 90 115 #[test] 91 116 fn config_runtime_beats_helix_runtime_and_overrides_win() { 92 117 let base = std::env::temp_dir().join(format!("dathan-rt-{}", std::process::id())); ··· 97 122 let env_rt = seed_root(&base, "env-runtime", "env"); 98 123 let override_rt = seed_root(&base, "override", "override"); 99 124 100 - std::env::set_var("HOME", &home); 101 - std::env::set_var("HELIX_RUNTIME", &env_rt); 125 + let _home_guard = EnvGuard::set("HOME", &home); 126 + let _runtime_guard = EnvGuard::set("HELIX_RUNTIME", &env_rt); 102 127 103 128 let rel = Path::new("grammars/test.txt"); 104 129 ··· 121 146 assert_eq!(fs::read_to_string(found).unwrap(), "override"); 122 147 assert_eq!(rt.override_roots(), &[override_rt]); 123 148 124 - std::env::remove_var("HELIX_RUNTIME"); 125 149 let _ = fs::remove_dir_all(&base); 126 150 } 127 151 }

-2

src/backend/hiccup.rs

··· 41 41 } 42 42 43 43 fn text(&mut self, text: &str) { 44 - // The driver coalesces runs of equal scope, but tree-sitter can still 45 - // split text at injection/event boundaries; merge adjacent literals. 46 44 let literal = edn_string(text); 47 45 if let Some(frame) = self.stack.last_mut() { 48 46 frame.children.push(literal);

+4

src/backend/terminal.rs

··· 68 68 } 69 69 70 70 fn text(&mut self, text: &str) { 71 + // A no-op call must not disturb `line_dirty` / `ended_with_newline`. 72 + if text.is_empty() { 73 + return; 74 + } 71 75 let before = self.out.len(); 72 76 // Fill each line's trailing cells with the active background by erasing 73 77 // to end-of-line before every newline.

Configure Feed

Configure Feed