Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion crates/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,23 @@ pub fn default_hotbar_bindings() -> Vec<HotbarBinding> {
.collect()
}

/// The default hotbar slots in on-disk (`[[hotbar]]`) form. Since #3807 an
/// absent `hotbar` key means "hidden", so `/hotbar on` persists these explicit
/// bindings rather than deleting the key. Kept in terms of
/// [`default_hotbar_bindings`] so `DEFAULT_HOTBAR_ACTIONS` stays the single
/// source of truth.
#[must_use]
pub fn default_hotbar_bindings_toml() -> Vec<HotbarBindingToml> {
default_hotbar_bindings()
.into_iter()
.map(|binding| HotbarBindingToml {
slot: binding.slot,
action: binding.action,
label: binding.label,
})
.collect()
}

#[must_use]
pub fn resolve_hotbar_bindings(
configured: Option<&[HotbarBindingToml]>,
Expand All @@ -797,7 +814,10 @@ pub fn resolve_hotbar_bindings(
label: binding.label.clone(),
})
.collect::<Vec<_>>(),
None => default_hotbar_bindings(),
// #3807: an absent `hotbar` key means the Hotbar is hidden until the
// user opts in (via the setup wizard or `/hotbar on`). Only an explicit
// `[[hotbar]]` config produces bindings. `Some([])` stays "disabled".
None => Vec::new(),
};

let mut by_slot: BTreeMap<u8, HotbarBinding> = BTreeMap::new();
Expand Down
35 changes: 18 additions & 17 deletions crates/config/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,29 +335,30 @@ fn provider_auth_source_rejects_empty_command() {
}

#[test]
fn hotbar_defaults_when_config_is_absent() {
fn hotbar_hidden_when_config_is_absent() {
// #3807: an absent `hotbar` key resolves to no bindings, so the Hotbar is
// hidden until the user opts in. The default slots are still available
// explicitly via `default_hotbar_bindings_toml()` (what `/hotbar on` writes).
let config = ConfigToml::default();

let resolved = config.resolve_hotbar_bindings(&DEFAULT_HOTBAR_ACTIONS);

assert_eq!(resolved.warnings, Vec::new());
assert_eq!(resolved.bindings, default_hotbar_bindings());
assert!(
resolved.bindings.is_empty(),
"fresh config must resolve to no hotbar bindings: {:?}",
resolved.bindings
);

// The explicit default set still expands to the eight recommended slots.
let mut explicit = ConfigToml::default();
explicit.hotbar = Some(default_hotbar_bindings_toml());
assert_eq!(
resolved
.bindings
.iter()
.map(|binding| (binding.slot, binding.action.as_str()))
.collect::<Vec<_>>(),
vec![
(1, "voice.toggle"),
(2, "session.compact"),
(3, "mode.plan"),
(4, "mode.agent"),
(5, "mode.yolo"),
(6, "palette.open"),
(7, "sidebar.toggle"),
(8, "trust.toggle"),
]
explicit
.resolve_hotbar_bindings(&DEFAULT_HOTBAR_ACTIONS)
.bindings,
default_hotbar_bindings(),
"an explicit default-bindings config still shows all eight slots"
);
}

Expand Down
123 changes: 27 additions & 96 deletions crates/tui/src/config_persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,41 +381,6 @@ pub(crate) fn persist_hotbar_bindings(
Ok(path)
}

/// Remove the `hotbar` key from the resolved config so the resolver falls back
/// to the built-in default slots (`/hotbar on` / `/hotbar reset`). Unlike
/// `persist_hotbar_bindings(&[])` — which writes an explicit `hotbar = []` and
/// therefore *disables* the hotbar — this deletes the key entirely, restoring
/// defaults. If the file or key is absent there is nothing to restore; we still
/// return the resolved path. Surrounding keys are preserved via `toml_edit`;
/// a comment used as the removed key's own leading decor is removed with it
/// (standard `toml_edit` key-prefix behavior).
pub(crate) fn remove_hotbar_from_config(config_path: Option<&Path>) -> anyhow::Result<PathBuf> {
use anyhow::Context;
use std::fs;

let path = config_toml_path(config_path)?;
if !path.exists() {
// No config file ⇒ no `hotbar` key ⇒ defaults already apply.
return Ok(path);
}
let raw = fs::read_to_string(&path)
.with_context(|| format!("failed to read config at {}", path.display()))?;
if raw.trim().is_empty() {
return Ok(path);
}
let mut document = raw
.parse::<toml_edit::DocumentMut>()
.with_context(|| format!("failed to edit config at {}", path.display()))?;
let table = document.as_table_mut();
if table.remove("hotbar").is_none() {
// Key wasn't present ⇒ defaults already apply; avoid a needless rewrite.
return Ok(path);
}
fs::write(&path, document.to_string())
.with_context(|| format!("failed to write config at {}", path.display()))?;
Ok(path)
}

pub(crate) fn config_toml_path(config_path: Option<&Path>) -> anyhow::Result<PathBuf> {
use anyhow::Context;

Expand Down Expand Up @@ -726,6 +691,33 @@ mod tests {
assert_eq!(parsed.hotbar, Some(bindings));
}

#[test]
fn persist_default_hotbar_bindings_round_trips_for_hotbar_on() {
// #3807: `/hotbar on` persists the explicit default slots (an absent key
// now means hidden), and they read back as the eight recommended slots.
let temp_root = temp_root("codewhale-hotbar-on-defaults");
fs::create_dir_all(&temp_root).unwrap();
let _guard = EnvGuard::new(&temp_root);

let defaults = codewhale_config::default_hotbar_bindings_toml();
assert_eq!(defaults.len(), codewhale_config::HOTBAR_SLOT_COUNT as usize);

let path = persist_hotbar_bindings(None, &defaults).expect("persist should succeed");
let body = fs::read_to_string(&path).expect("written file should be readable");
assert!(body.contains("[[hotbar]]"), "hotbar table missing: {body}");

let parsed: codewhale_config::ConfigToml =
toml::from_str(&body).expect("written hotbar config should parse");
assert_eq!(parsed.hotbar, Some(defaults));

// The persisted defaults resolve back to all eight recommended slots.
let resolved = parsed.resolve_hotbar_bindings(&codewhale_config::DEFAULT_HOTBAR_ACTIONS);
assert_eq!(
resolved.bindings,
codewhale_config::default_hotbar_bindings()
);
}

#[test]
fn persist_hotbar_bindings_preserves_comments_and_replaces_existing_tables() {
let temp_root = temp_root("codewhale-hotbar-persist-comments");
Expand Down Expand Up @@ -796,65 +788,4 @@ enabled = true
toml::from_str(&body).expect("written hotbar config should parse");
assert_eq!(parsed.hotbar, Some(Vec::new()));
}

#[test]
fn remove_hotbar_from_config_deletes_key_to_restore_defaults() {
let temp_root = temp_root("codewhale-hotbar-remove");
fs::create_dir_all(&temp_root).unwrap();
let _guard = EnvGuard::new(&temp_root);

let path = temp_root.join(".codewhale").join("config.toml");
fs::create_dir_all(path.parent().unwrap()).unwrap();
// Seed a config with a hotbar binding and an unrelated key whose own
// trailing comment is NOT the hotbar key's decor (so it must survive).
fs::write(
&path,
"hotbar = [{ slot = 1, action = \"mode.plan\" }]\nother = true # keep me\n",
)
.unwrap();

let returned = remove_hotbar_from_config(Some(&path)).expect("remove should succeed");
assert_eq!(returned, path);

let body = fs::read_to_string(&path).expect("written file should be readable");
assert!(
!body.contains("hotbar"),
"hotbar key should be gone: {body}"
);
// An unrelated key and its own trailing comment must survive the rewrite.
assert!(
body.contains("other = true"),
"unrelated key should survive: {body}"
);
assert!(
body.contains("keep me"),
"comment that is not the hotbar key's decor should survive: {body}"
);

let parsed: codewhale_config::ConfigToml =
toml::from_str(&body).expect("restored config should parse");
assert_eq!(
parsed.hotbar, None,
"removing the key reads back as None so the resolver falls back to defaults"
);
}

#[test]
fn remove_hotbar_from_config_is_a_noop_when_key_absent() {
let temp_root = temp_root("codewhale-hotbar-remove-absent");
fs::create_dir_all(&temp_root).unwrap();
let _guard = EnvGuard::new(&temp_root);

let path = temp_root.join(".codewhale").join("config.toml");
fs::create_dir_all(path.parent().unwrap()).unwrap();
fs::write(&path, "other = true\n").unwrap();

let before = fs::read_to_string(&path).unwrap();
remove_hotbar_from_config(Some(&path)).expect("noop should succeed");
let after = fs::read_to_string(&path).unwrap();
assert_eq!(
before, after,
"an absent hotbar key should not trigger a rewrite"
);
}
}
4 changes: 3 additions & 1 deletion crates/tui/src/tui/hotbar/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,9 @@ mod tests {
);
assert_eq!(view.selected_source(), Some(HotbarActionCategory::App));
assert!(view.recommended_action_ids().contains("mode.agent"));
assert!(view.checked_action_ids().contains("mode.plan"));
// #3807: a fresh config seeds no bindings, so the wizard opens with
// nothing checked until the user opts in.
assert!(view.checked_action_ids().is_empty());
}

#[test]
Expand Down
43 changes: 24 additions & 19 deletions crates/tui/src/tui/sidebar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,12 @@ fn split_sidebar_hotbar_area(area: Rect, show_hotbar: bool) -> (Rect, Option<Rec
(sections[0], Some(sections[1]))
}

/// The Hotbar is "disabled" only when the user persisted an explicit empty
/// `hotbar = []`. A missing `hotbar` key (`None`) means "use built-in defaults",
/// so it is *not* disabled. This keeps `/hotbar on` (remove key → defaults) and
/// `/hotbar off` (empty array → hidden) distinct.
/// The Hotbar is "disabled" when the user persisted an explicit empty
/// `hotbar = []`. Since #3807 a missing `hotbar` key (`None`) also renders no
/// panel — the Hotbar is hidden until the user opts in — but it resolves to
/// zero bindings via [`hotbar_panel_enabled`] rather than the explicit-disabled
/// state, which keeps `/hotbar on` (write default bindings) and `/hotbar off`
/// (write `[]`) distinct on disk.
fn is_hotbar_disabled(config: &Config) -> bool {
config.hotbar.as_deref().is_some_and(<[_]>::is_empty)
}
Expand Down Expand Up @@ -3480,25 +3482,23 @@ mod tests {
}

#[test]
fn hotbar_panel_slots_resolve_default_bindings_and_active_state() {
fn hotbar_panel_hidden_for_fresh_default_config() {
// #3807: a fresh config has no `hotbar` key, so the panel is hidden
// until the user opts in. Slot resolution + active state are covered by
// `hotbar_panel_slots_resolve_configured_bindings_and_active_state`.
let mut app = create_test_app();
app.mode = AppMode::Agent;
app.sidebar_focus = SidebarFocus::Pinned;

assert!(hotbar_panel_enabled(&app, &Config::default()));

let slots = hotbar_panel_slots(&app, &Config::default());

assert_eq!(slots.len(), 8);
assert_eq!(slots[0].slot, 1);
assert_eq!(slots[0].label, "voice");
assert_eq!(slots[0].state, HotbarSlotState::Inactive);
assert_eq!(slots[3].label, "agent");
assert_eq!(slots[3].state, HotbarSlotState::Active);
let slot_4_chord = format!("{}4", crate::tui::widgets::key_hint::alt_prefix());
assert!(
slots[3].full_text.contains(&slot_4_chord),
"hover text should expose the dispatch chord: {slots:?}"
!hotbar_panel_enabled(&app, &Config::default()),
"fresh config must not enable the Hotbar panel"
);
assert!(
hotbar_panel_slots(&app, &Config::default())
.iter()
.all(|slot| slot.state == HotbarSlotState::Empty),
"fresh config resolves to no bound slots"
);
}

Expand Down Expand Up @@ -3677,7 +3677,12 @@ mod tests {
let mut app = create_test_app();
app.sidebar_focus = SidebarFocus::Pinned;
app.mode = AppMode::Agent;
let config = Config::default();
// #3807: the panel is hidden on a fresh config, so opt in explicitly
// with the default bindings to smoke-test the rendered panel.
let config = Config {
hotbar: Some(codewhale_config::default_hotbar_bindings_toml()),
..Config::default()
};

let backend = TestBackend::new(44, 12);
let mut terminal = Terminal::new(backend).expect("terminal");
Expand Down
19 changes: 11 additions & 8 deletions crates/tui/src/tui/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9092,22 +9092,25 @@ fn disable_hotbar(app: &mut App, config: &mut Config) {
app.needs_redraw = true;
}

/// Restore the default recommended Hotbar slots: remove the `hotbar` key so the
/// resolver falls back to the built-in defaults. This is an explicit reset, so
/// any custom bindings are replaced with the recommended set.
/// Show the default recommended Hotbar slots. Since #3807 an absent `hotbar`
/// key means "hidden", so `/hotbar on` persists the explicit default bindings
/// rather than deleting the key. This is an explicit reset, so any custom
/// bindings are replaced with the recommended set.
fn restore_hotbar_defaults(app: &mut App, config: &mut Config) {
match crate::config_persistence::remove_hotbar_from_config(app.config_path.as_deref()) {
let defaults = codewhale_config::default_hotbar_bindings_toml();
match crate::config_persistence::persist_hotbar_bindings(app.config_path.as_deref(), &defaults)
{
Ok(path) => {
config.hotbar = None;
config.hotbar = Some(defaults);
app.status_message = Some(format!(
"Hotbar restored to default slots ({}). Customize with `/hotbar`.",
"Hotbar enabled with the default slots ({}). Customize with `/hotbar`.",
path.display()
));
}
Err(err) => {
app.status_message = Some(format!("Failed to restore Hotbar defaults: {err}"));
app.status_message = Some(format!("Failed to enable the Hotbar: {err}"));
app.add_message(HistoryCell::System {
content: format!("Failed to restore Hotbar defaults: {err}"),
content: format!("Failed to enable the Hotbar: {err}"),
});
}
}
Expand Down
7 changes: 6 additions & 1 deletion crates/tui/src/tui/ui/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4186,7 +4186,12 @@ fn hotbar_alt_digit_is_blocked_while_decision_card_is_active() {
#[test]
fn hotbar_dispatches_bound_slot_and_ignores_empty_slot() {
let mut app = create_test_app();
let config = Config::default();
// #3807: a fresh config has no bindings, so opt in with the default slots
// (slot 4 = mode.agent) to exercise dispatch of a bound slot.
let config = Config {
hotbar: Some(codewhale_config::default_hotbar_bindings_toml()),
..Config::default()
};
app.onboarding = OnboardingState::None;
app.mode = AppMode::Plan;
app.needs_redraw = false;
Expand Down
Loading