Skip to content

Commit a30c33d

Browse files
committed
docs(core): add inline comments throughout core crate
Add concise, high-signal inline comments to function bodies across the core configuration and utility implementation. Comments explain intent, edge cases, boundary math, ownership/cloning decisions, and performance trade-offs rather than restating code behavior. Key areas covered: - Config schema (validation, parsing, bounds checking) - Config storage (file I/O, atomic writes, migration logic) - Utility functions (time formatting, size formatting, unit conversions)
1 parent 0fda135 commit a30c33d

3 files changed

Lines changed: 50 additions & 5 deletions

File tree

keyless-core/src/config/schema.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,18 @@ impl FromStr for OutputMode {
122122
type Err = KeylessError;
123123

124124
fn from_str(s: &str) -> Result<Self, Self::Err> {
125+
// Trim whitespace before comparison (handles user input edge cases).
125126
let v = s.trim();
127+
// Case-insensitive comparison for user-friendly parsing.
126128
if v.eq_ignore_ascii_case("paste") {
127129
Ok(OutputMode::Paste)
128130
} else if v.eq_ignore_ascii_case("clipboard") {
129131
Ok(OutputMode::Clipboard)
130132
} else if v.eq_ignore_ascii_case("file") {
133+
// File mode requires a path; cannot be parsed from string alone.
131134
Err(KeylessError::Config("file sink requires a path".into()))
132135
} else {
136+
// Unknown sink: return error with the attempted value for debugging.
133137
Err(KeylessError::Config(format!("unknown sink: {}", v)))
134138
}
135139
}
@@ -254,21 +258,26 @@ impl Config {
254258
/// - Model file existence (done separately at startup for better error messages)
255259
/// - Hotkey validity (platform-specific, checked during registration)
256260
pub fn validate(&self) -> KeylessResult<()> {
257-
// If outputting to a file, ensure the parent directory exists
261+
// If outputting to a file, ensure the parent directory exists.
262+
// Check parent() first (returns None for root paths, which we skip).
263+
// exists() check prevents silent failures when writing to non-existent directory.
258264
if let OutputMode::File(path) = &self.output_mode
259265
&& let Some(parent) = path.parent()
260266
&& !parent.exists()
261267
{
262268
return Err(KeylessError::InvalidPath(parent.to_path_buf()));
263269
}
264270

265-
// Validate language code length (ISO 639-1 = 2 chars, ISO 639-3 = 3 chars)
271+
// Validate language code length (ISO 639-1 = 2 chars, ISO 639-3 = 3 chars).
272+
// Allow up to 8 chars for future extensions (some codes include script/region suffixes).
273+
// Reject empty or excessively long codes (defensive against malformed config).
266274
if let Some(lang) = &self.language
267275
&& (lang.len() < 2 || lang.len() > 8)
268276
{
269277
return Err(KeylessError::Config("invalid language code".into()));
270278
}
271279

280+
// Delegate EQ validation; propagate errors via ? operator.
272281
self.eq.validate()?;
273282

274283
Ok(())

keyless-core/src/config/storage.rs

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ pub const CURRENT_VERSION: u32 = 1;
1919
/// Migrate config to the current version.
2020
fn migrate_config(mut config: Config) -> Config {
2121
// Placeholder for future migrations. For now, just ensure version is current.
22+
// Update version field if config is from an older schema (enables future migration logic).
2223
if config.version < CURRENT_VERSION {
2324
config.version = CURRENT_VERSION;
2425
}
@@ -27,7 +28,10 @@ fn migrate_config(mut config: Config) -> Config {
2728

2829
/// Compute the cross-platform path for the config file.
2930
pub fn config_path() -> PathBuf {
31+
// Get platform-specific config directory; fallback to current directory if unavailable.
32+
// Fallback to "." is defensive (rare edge case where config_dir() fails).
3033
let base = config_dir().unwrap_or_else(|| PathBuf::from("."));
34+
// Join app name and filename to form full path (e.g., ~/.config/keyless/config.json).
3135
let p = base.join("keyless").join("config.json");
3236
tracing::trace!(path=%p.display(), "config: computed config path");
3337
p
@@ -36,20 +40,25 @@ pub fn config_path() -> PathBuf {
3640
/// Load config from disk (returns None if missing or invalid).
3741
pub fn load_config() -> Option<Config> {
3842
let path = config_path();
43+
// Read entire file into memory; small file (<10KB) so one-shot read is fine.
3944
let data = match fs::read(&path) {
4045
Ok(d) => d,
4146
Err(e) => {
47+
// File missing is expected on first run; log as debug, not error.
4248
tracing::debug!(path=%path.display(), error=%e, "config: no config file yet");
4349
return None;
4450
}
4551
};
52+
// Parse JSON; handle parse errors gracefully (malformed config = None, not panic).
4653
match serde_json::from_slice::<Config>(&data) {
4754
Ok(config) => {
55+
// Apply migrations if needed (updates version field for older configs).
4856
let config = migrate_config(config);
4957
tracing::debug!(path=%path.display(), "config: loaded successfully");
5058
Some(config)
5159
}
5260
Err(e) => {
61+
// Log parse error as warning (user may have corrupted config manually).
5362
tracing::warn!(path=%path.display(), error=%e, "config: failed to parse");
5463
None
5564
}
@@ -59,22 +68,29 @@ pub fn load_config() -> Option<Config> {
5968
/// Save config to disk (atomic-ish: create parent, write file).
6069
pub fn save_config(config: &Config) -> io::Result<()> {
6170
let path = config_path();
71+
// Create parent directory if missing (e.g., ~/.config/keyless/).
72+
// Propagate errors to surface permission issues early (better than silent failure).
6273
if let Some(dir) = path.parent() {
6374
// Propagate directory creation errors to surface permission issues early
6475
fs::create_dir_all(dir)?;
6576
}
77+
// Serialize to pretty-printed JSON (indented, human-readable).
78+
// Convert serde error to io::Error for consistent error handling.
6679
let data = serde_json::to_vec_pretty(config)
6780
.map_err(|e| io::Error::other(format!("serialize config: {}", e)))?;
68-
// Atomic-ish write: write to a temp file then rename
81+
// Atomic-ish write: write to temp file then rename (prevents partial writes).
82+
// Temp file: config.json.tmp (same directory, atomic rename on most platforms).
6983
let tmp = path.with_extension("json.tmp");
7084
fs::write(&tmp, &data)?;
7185
match fs::rename(&tmp, &path) {
7286
Ok(()) => {}
7387
Err(_) => {
74-
// Retry once after a short delay (helps on platforms with transient locks)
88+
// Retry once after a short delay (helps on platforms with transient locks).
89+
// Some filesystems/antivirus temporarily lock files during writes.
7590
std::thread::sleep(std::time::Duration::from_millis(10));
7691
if fs::rename(&tmp, &path).is_err() {
77-
// Fallback: copy then remove the temp file
92+
// Fallback: copy then remove temp file (non-atomic but ensures write succeeds).
93+
// Prefer copy over leaving temp file (avoids stale temp files on disk).
7894
fs::copy(&tmp, &path)?;
7995
let _ = fs::remove_file(&tmp);
8096
}

keyless-core/src/utils.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,58 @@
33
/// Current time in milliseconds since UNIX epoch (saturating to 0 on error).
44
pub fn now_millis() -> u64 {
55
use std::time::{SystemTime, UNIX_EPOCH};
6+
// Get duration since epoch; unwrap_or_default returns Duration::ZERO if system time < epoch.
7+
// This handles clock skew edge cases (shouldn't happen on normal systems).
68
SystemTime::now()
79
.duration_since(UNIX_EPOCH)
810
.unwrap_or_default()
11+
// Convert to milliseconds (u128), then cast to u64 (truncates if > u64::MAX ms).
12+
// u64::MAX ms ≈ 584 million years, so truncation is not a practical concern.
913
.as_millis() as u64
1014
}
1115

1216
/// Format bytes into a human-readable string (KB/MB/GB) with one decimal place.
1317
pub fn human_size(bytes: u64) -> String {
18+
// Binary units (1024-based), not decimal (1000-based); standard for file sizes.
19+
// KB: 1024 bytes
20+
// MB: 1024 * 1024 bytes
21+
// GB: 1024 * 1024 * 1024 bytes
1422
const KB: f64 = 1024.0;
1523
const MB: f64 = 1024.0 * 1024.0;
1624
const GB: f64 = 1024.0 * 1024.0 * 1024.0;
25+
// Convert to f64 for division; u64→f64 preserves precision up to 2^53 (plenty for file sizes).
1726
let b = bytes as f64;
27+
// Check largest unit first (descending order) to pick appropriate suffix.
1828
if b >= GB {
29+
// One decimal place via {:.1}; division produces value in GB units.
1930
format!("{:.1} GB", b / GB)
2031
} else if b >= MB {
2132
format!("{:.1} MB", b / MB)
2233
} else if b >= KB {
2334
format!("{:.1} KB", b / KB)
2435
} else {
36+
// Bytes: no decimal, use original u64 value (integer bytes).
2537
format!("{} B", bytes)
2638
}
2739
}
2840

2941
/// Format milliseconds as HH:MM:SS or MM:SS if under one hour.
3042
pub fn fmt_hms(ms: u64) -> String {
43+
// Integer division truncates fractional seconds (e.g., 1500ms → 1s, not 1.5s).
3144
let total_secs = ms / 1000;
45+
// Extract hours: total_secs / 3600 (integer division).
3246
let hours = total_secs / 3600;
47+
// Extract minutes: remainder after removing hours, then divide by 60.
48+
// Modulo 3600 gives seconds in current hour, divide by 60 gives minutes.
3349
let mins = (total_secs % 3600) / 60;
50+
// Extract seconds: remainder after removing hours and minutes.
3451
let secs = total_secs % 60;
52+
// Conditional formatting: show hours only if > 0 (compact display for short durations).
3553
if hours > 0 {
54+
// HH:MM:SS format; {:02} zero-pads minutes/seconds to 2 digits.
3655
format!("{}:{:02}:{:02}", hours, mins, secs)
3756
} else {
57+
// MM:SS format (no hours) for durations under 1 hour.
3858
format!("{:02}:{:02}", mins, secs)
3959
}
4060
}

0 commit comments

Comments
 (0)