fix(edit): add --editor flag and validate editor before launching (#7)

Add a `-e`/`--editor` CLI flag to override the editor used by the edit
command. Editor resolution now prefers the flag, falls back to $EDITOR,
and fails explicitly when neither is set (instead of silently defaulting
to vim). The resolved editor is validated via `which` before launch to
give a clear error on missing executables.

Add unit tests for resolve_editor, try_generate_edit_diffs, and display
helpers, and add tests for PrintableActivityLog row rendering.

Author: coko7

Cargo.lock

@@ -30,6 +30,7 @@ boat-lib = "0.5.0"
 tabular = { version = "0.2.0", features = ["ansi-cell"] }
 yansi = "1.0.1"
 dialoguer = "0.12.0"
+which = "8.0.2"
 
 [dev-dependencies]
 assert_cmd = "2.0"

Cargo.toml

@@ -287,6 +287,10 @@ pub struct EditLogsArgs {
     )]
     pub period: Option<PeriodInput>,
 
+    /// Specify the editor program to use
+    #[arg(short = 'e', long = "editor")]
+    pub editor: Option<String>,
+
     /// Include instruction comments in the editable file
     #[arg(short = 'i', long = "with-instructions", alias = "with-instr", action = ArgAction::SetTrue, conflicts_with = "hide_instructions")]
     pub show_instructions: bool,

src/cli/args.rs

@@ -56,7 +56,14 @@ pub fn edit(
     let default_content = boat_data.to_csv_str(include_instructions, include_activity_definitions);
     let edit_file_path = create_tmp_edit_file(&default_content)?;
 
-    let editor = env::var("EDITOR").unwrap_or_else(|_| "vim".to_string());
+    let editor = args
+        .editor
+        .clone()
+        .or_else(|| env::var("EDITOR").ok())
+        .context("No editor specified and EDITOR is not set")?;
+
+    resolve_editor(&editor)?;
+    info!("launching editor: {editor}");
     let status = Command::new(editor).arg(&edit_file_path).status()?;
 
     ensure!(
@@ -137,6 +144,17 @@ struct EditLogDiff {
     ends_at_new: Option<Option<DateTime<Utc>>>,
 }
 
+fn resolve_editor(editor: &str) -> Result<()> {
+    if editor.is_empty() {
+        bail!(
+            "Default text editor not found. Please refer to your shell documentation to set your EDITOR environment variable."
+        );
+    }
+
+    which::which(editor).with_context(|| format!("EDITOR command not found: {editor}"))?;
+    Ok(())
+}
+
 fn pretty_print_edit_diffs(diffs: &[EditLogDiff]) {
     println!("Detected changes:");
     for diff in diffs {
@@ -391,10 +409,13 @@ fn convert_modified_content_to_log_lines(content: &str) -> Result<Vec<DatabaseLo
 }
 
 fn create_tmp_edit_file(content: &str) -> Result<PathBuf> {
-    info!("creating temporary file for editing");
     let tmp_dir = env::temp_dir();
     let file_path = tmp_dir.join("boat_edit_logs_tmp.csv");
     fs::write(&file_path, content)?;
+    info!(
+        "created temporary file for editing: {}",
+        file_path.display()
+    );
     Ok(file_path)
 }
 
@@ -478,4 +499,120 @@ mod tests {
         let csv = "1, 10, not-a-date, 2024-06-01 11:00";
         assert!(convert_modified_content_to_log_lines(csv).is_err());
     }
+
+    // --- resolve_editor ---
+
+    #[test]
+    fn resolve_editor_empty_string_fails() {
+        assert!(resolve_editor("").is_err());
+    }
+
+    #[test]
+    fn resolve_editor_nonexistent_command_fails() {
+        assert!(resolve_editor("definitely-not-a-real-editor-xyz").is_err());
+    }
+
+    // --- try_generate_edit_diffs error paths ---
+
+    #[test]
+    fn try_generate_edit_diffs_length_mismatch_fails() {
+        let orig = vec![make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 11:00"))];
+        let edited = vec![
+            make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 11:00")),
+            make_log(2, 1, "2024-06-01 12:00", Some("2024-06-01 13:00")),
+        ];
+        assert!(try_generate_edit_diffs(&edited, &orig).is_err());
+    }
+
+    #[test]
+    fn try_generate_edit_diffs_multiple_open_ended_logs_fails() {
+        let orig = vec![
+            make_log(1, 1, "2024-06-01 10:00", None),
+            make_log(2, 1, "2024-06-01 12:00", None),
+        ];
+        assert!(try_generate_edit_diffs(&orig, &orig).is_err());
+    }
+
+    #[test]
+    fn try_generate_edit_diffs_unknown_log_id_fails() {
+        let orig = vec![make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 11:00"))];
+        let edited = vec![make_log(
+            99,
+            1,
+            "2024-06-01 10:00",
+            Some("2024-06-01 11:00"),
+        )];
+        assert!(try_generate_edit_diffs(&edited, &orig).is_err());
+    }
+
+    #[test]
+    fn try_generate_edit_diffs_activity_id_changed_fails() {
+        let orig = vec![make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 11:00"))];
+        let edited = vec![make_log(
+            1,
+            999,
+            "2024-06-01 10:00",
+            Some("2024-06-01 11:00"),
+        )];
+        assert!(try_generate_edit_diffs(&edited, &orig).is_err());
+    }
+
+    #[test]
+    fn try_generate_edit_diffs_starts_at_after_ends_at_fails() {
+        let orig = vec![make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 11:00"))];
+        let edited = vec![make_log(1, 1, "2024-06-01 12:00", Some("2024-06-01 11:00"))];
+        assert!(try_generate_edit_diffs(&edited, &orig).is_err());
+    }
+
+    // --- try_generate_edit_diffs happy paths ---
+
+    #[test]
+    fn try_generate_edit_diffs_no_change_returns_empty() {
+        let logs = vec![make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 11:00"))];
+        let diffs = try_generate_edit_diffs(&logs, &logs).unwrap();
+        assert!(diffs.is_empty());
+    }
+
+    #[test]
+    fn try_generate_edit_diffs_detects_ends_at_change() {
+        let orig = vec![make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 11:00"))];
+        let edited = vec![make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 12:00"))];
+        let diffs = try_generate_edit_diffs(&edited, &orig).unwrap();
+        assert_eq!(diffs.len(), 1);
+        assert!(diffs[0].ends_at_new.is_some());
+        assert!(diffs[0].starts_at_new.is_none());
+    }
+
+    #[test]
+    fn try_generate_edit_diffs_detects_log_closed_to_open() {
+        let orig = vec![make_log(1, 1, "2024-06-01 10:00", Some("2024-06-01 11:00"))];
+        let edited = vec![make_log(1, 1, "2024-06-01 10:00", None)];
+        let diffs = try_generate_edit_diffs(&edited, &orig).unwrap();
+        assert_eq!(diffs.len(), 1);
+        // ends_at_new is Some(None) — the log was re-opened
+        assert_eq!(diffs[0].ends_at_new, Some(None));
+    }
+
+    // --- date_time_opt_loose_eq ---
+
+    #[test]
+    fn date_time_opt_loose_eq_both_none_are_equal() {
+        let none: Option<DateTime<Utc>> = None;
+        assert!(date_time_opt_loose_eq(&none, &none).unwrap());
+    }
+
+    #[test]
+    fn date_time_opt_loose_eq_some_and_none_are_not_equal() {
+        let some = Some(Utc.with_ymd_and_hms(2024, 6, 1, 10, 0, 0).unwrap());
+        let none = None;
+        assert!(!date_time_opt_loose_eq(&some, &none).unwrap());
+        assert!(!date_time_opt_loose_eq(&none, &some).unwrap());
+    }
+
+    #[test]
+    fn date_time_opt_loose_eq_different_minutes_are_not_equal() {
+        let dt1 = Some(Utc.with_ymd_and_hms(2024, 6, 1, 10, 0, 0).unwrap());
+        let dt2 = Some(Utc.with_ymd_and_hms(2024, 6, 1, 10, 1, 0).unwrap());
+        assert!(!date_time_opt_loose_eq(&dt1, &dt2).unwrap());
+    }
 }

src/commands/edit.rs

@@ -68,3 +68,69 @@ impl RowPrintable for PrintableActivityLog {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use boat_lib::models::log::Log as DatabaseLog;
+    use chrono::{TimeZone, Utc};
+    use std::collections::HashSet;
+
+    fn make_activity() -> SimpleActivity {
+        SimpleActivity {
+            id: 7,
+            name: "testing".to_string(),
+            description: Some("a desc".to_string()),
+            tags: HashSet::new(),
+        }
+    }
+
+    fn make_log(ends_after_secs: Option<i64>) -> DatabaseLog {
+        let start = Utc.with_ymd_and_hms(2024, 4, 15, 10, 0, 0).unwrap();
+        DatabaseLog {
+            id: 42,
+            activity_id: 7,
+            starts_at: start,
+            ends_at: ends_after_secs.map(|s| start + chrono::Duration::seconds(s)),
+        }
+    }
+
+    #[test]
+    fn row_values_open_log_shows_dash_for_end() {
+        let pal = PrintableActivityLog::from_activity_and_log(&make_activity(), &make_log(None));
+        assert_eq!(pal.row_values()[5], "-");
+    }
+
+    #[test]
+    fn row_values_closed_log_has_non_empty_end() {
+        let pal =
+            PrintableActivityLog::from_activity_and_log(&make_activity(), &make_log(Some(3600)));
+        let end_col = &pal.row_values()[5];
+        assert_ne!(end_col, "-");
+        assert!(!end_col.is_empty());
+    }
+
+    #[test]
+    fn row_values_id_and_name_match_activity() {
+        let pal =
+            PrintableActivityLog::from_activity_and_log(&make_activity(), &make_log(Some(3600)));
+        let values = pal.row_values();
+        assert_eq!(values[0], "7");
+        assert_eq!(values[1], "testing");
+    }
+
+    #[test]
+    fn style_cell_closed_log_returns_value_unchanged() {
+        let pal =
+            PrintableActivityLog::from_activity_and_log(&make_activity(), &make_log(Some(3600)));
+        let val = "unchanged".to_string();
+        assert_eq!(pal.style_cell(val.clone()), val);
+    }
+
+    #[test]
+    fn style_cell_open_log_result_contains_original_value() {
+        let pal = PrintableActivityLog::from_activity_and_log(&make_activity(), &make_log(None));
+        let val = "styled".to_string();
+        assert!(pal.style_cell(val.clone()).contains(&val));
+    }
+}

src/models/activity_log.rs

@@ -190,3 +190,49 @@ pub fn current_activity_msg(activity: &DatabaseActivity) -> Result<String> {
 pub fn no_current_act_msg() -> String {
     "no current activity".to_string()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn get_group_by_display_values_none_returns_all() {
+        let (label, tooltip) = get_group_by_display_values(GroupBy::None, "anything").unwrap();
+        assert_eq!(label, "ALL");
+        assert!(tooltip.is_none());
+    }
+
+    #[test]
+    fn get_group_by_display_values_year_returns_key() {
+        let (label, tooltip) = get_group_by_display_values(GroupBy::Year, "2024").unwrap();
+        assert_eq!(label, "2024");
+        assert!(tooltip.is_none());
+    }
+
+    #[test]
+    fn get_group_by_display_values_month_formats_name() {
+        let (label, tooltip) = get_group_by_display_values(GroupBy::Month, "2024-04").unwrap();
+        assert_eq!(label, "April 2024");
+        assert!(tooltip.is_none());
+    }
+
+    #[test]
+    fn get_group_by_display_values_month_invalid_key_fails() {
+        assert!(get_group_by_display_values(GroupBy::Month, "not-a-month").is_err());
+    }
+
+    #[test]
+    fn get_group_by_display_values_week_label_and_date_range() {
+        let (label, tooltip) = get_group_by_display_values(GroupBy::Week, "2024-W10").unwrap();
+        assert_eq!(label, "Week 10");
+        // 2024 starts on a Monday, so week 10 runs Mar 04–Mar 10
+        let tooltip = tooltip.unwrap();
+        assert!(tooltip.contains("Mar 04, 2024"), "got: {tooltip}");
+        assert!(tooltip.contains("Mar 10, 2024"), "got: {tooltip}");
+    }
+
+    #[test]
+    fn get_group_by_display_values_week_invalid_key_fails() {
+        assert!(get_group_by_display_values(GroupBy::Week, "not-a-week").is_err());
+    }
+}

src/utils/display.rs

@@ -49,3 +49,18 @@ fn delete_with_missing_id_arg_fails() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn delete_running_activity_succeeds() -> Result<()> {
+    let (_tmp, config_path) = cli_args_for_temp()?;
+
+    run_boat(["new", "ActiveTask", "--start-now"], &config_path).success();
+
+    run_boat(["delete", "1", "--no-confirm"], &config_path).success();
+
+    run_boat(["list", "--json"], &config_path)
+        .success()
+        .stdout(predicates::str::contains("ActiveTask").not());
+
+    Ok(())
+}

tests/cli_delete.rs

@@ -41,3 +41,17 @@ fn get_when_no_current_activity_fails() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn get_after_pause_fails() -> Result<()> {
+    let (_tmp, config_path) = cli_args_for_temp()?;
+
+    run_boat(["new", "WasRunning", "--start-now"], &config_path).success();
+    run_boat(["pause"], &config_path).success();
+
+    run_boat(["get", "--json"], &config_path)
+        .failure()
+        .stderr(predicates::str::contains("no current activity"));
+
+    Ok(())
+}