From 50940a1e7fc5c3eba25ae51341668d1b07cc237b Mon Sep 17 00:00:00 2001 From: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:14:50 +0000 Subject: [PATCH 1/5] fix(config): redirect misplaced nested keys to the other config with a scope note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A nested key that's valid only in the other config type (e.g. `list.columns` or `list.custom-columns` — user-config display settings — placed in project config) previously read "unknown field", because nested-key redirection only covered the hard-coded `commit.generation` leaves. Generalize the check: a nested key that's schema-unknown in this config but valid in the other now reports "belongs in config" instead, determined by walking the other config's own unknown tree for the same content. When the destination is user config, append a note pointing at the `[projects.""]` table, which scopes a personal setting to one repo — the usual reason a user reaches for project config in the first place. Closes #3469. --- src/commands/config/show.rs | 20 ++- src/config/deprecation.rs | 116 +++++++++++++++++- src/config/mod.rs | 2 +- src/config/unknown_tree.rs | 71 ++++++++++- ...sts_user_config_for_commit_generation.snap | 2 +- 5 files changed, 200 insertions(+), 11 deletions(-) diff --git a/src/commands/config/show.rs b/src/commands/config/show.rs index f2f62de1a8..16e8735e2a 100644 --- a/src/commands/config/show.rs +++ b/src/commands/config/show.rs @@ -766,7 +766,10 @@ fn format_show_warning(warning: &worktrunk::config::UnknownWarning) -> String { UnknownWarning::TopLevelWrongConfig { key, other_description, - } => cformat!("Key {key} belongs in {other_description} (will be ignored)"), + } => with_scope_note( + cformat!("Key {key} belongs in {other_description} (will be ignored)"), + other_description, + ), UnknownWarning::TopLevelDeprecatedWrongConfig { key, other_description, @@ -775,13 +778,26 @@ fn format_show_warning(warning: &worktrunk::config::UnknownWarning) -> String { UnknownWarning::NestedWrongConfig { path, other_description, - } => cformat!("Key {path} belongs in {other_description} (will be ignored)"), + } => with_scope_note( + cformat!("Key {path} belongs in {other_description} (will be ignored)"), + other_description, + ), UnknownWarning::NestedUnknown { path } => { cformat!("Unknown key {path} will be ignored") } } } +/// Append the project-scoped-user-config note when the key belongs in user +/// config, mirroring the load-time warning path. See +/// [`scope_to_repo_note`](worktrunk::config::scope_to_repo_note). +fn with_scope_note(message: String, other_description: &str) -> String { + match worktrunk::config::scope_to_repo_note(other_description) { + Some(note) => format!("{message}; {note}"), + None => message, + } +} + fn render_project_config(out: &mut String) -> anyhow::Result<()> { // Try to get current repository root let repo = match Repository::current() { diff --git a/src/config/deprecation.rs b/src/config/deprecation.rs index 4f76fcc31b..4872c268e0 100644 --- a/src/config/deprecation.rs +++ b/src/config/deprecation.rs @@ -1787,6 +1787,19 @@ pub fn nested_key_belongs_in(path: &str) -> Option<&'static .then(C::Other::description) } +/// Note appended to a "belongs in user config" warning: a key placed in +/// project config that really lives in user config is usually an attempt to +/// scope a personal setting to one repo, which the `[projects.""]` table +/// in user config does directly. Returns `None` for any other destination. +/// +/// Keyed off the destination *description* rather than a config-type gate so +/// both warning formatters (load-time and `config show`) can share it — they +/// hold only the `other_description` string, not the config type. +pub fn scope_to_repo_note(other_description: &str) -> Option<&'static str> { + (other_description == crate::config::UserConfig::description()) + .then_some(r#"to scope it to this repo, add it under [projects.""] in user config"#) +} + /// Classification of an unknown config key for warning purposes. pub enum UnknownKeyKind { /// Deprecated key in its correct config type — deprecation system handles it @@ -1871,8 +1884,11 @@ fn format_load_warning(label: &str, warning: &crate::config::UnknownWarning) -> UnknownWarning::TopLevelWrongConfig { key, other_description, - } => cformat!( - "{label} has key {key} which belongs in {other_description} (will be ignored)" + } => with_scope_note( + cformat!( + "{label} has key {key} which belongs in {other_description} (will be ignored)" + ), + other_description, ), UnknownWarning::TopLevelDeprecatedWrongConfig { key, @@ -1884,8 +1900,11 @@ fn format_load_warning(label: &str, warning: &crate::config::UnknownWarning) -> UnknownWarning::NestedWrongConfig { path, other_description, - } => cformat!( - "{label} has key {path} which belongs in {other_description} (will be ignored)" + } => with_scope_note( + cformat!( + "{label} has key {path} which belongs in {other_description} (will be ignored)" + ), + other_description, ), UnknownWarning::NestedUnknown { path } => { cformat!("{label} has unknown field {path} (will be ignored)") @@ -1893,6 +1912,17 @@ fn format_load_warning(label: &str, warning: &crate::config::UnknownWarning) -> } } +/// Append the project-scoped-user-config note to `message` when the key's +/// destination is user config (see [`scope_to_repo_note`]). Joined with a +/// semicolon per the house style for related clauses. The note is plain text +/// so its `[projects.""]` placeholder isn't parsed as color-print markup. +fn with_scope_note(message: String, other_description: &str) -> String { + match scope_to_repo_note(other_description) { + Some(note) => format!("{message}; {note}"), + None => message, + } +} + #[cfg(test)] mod tests { use super::*; @@ -4491,6 +4521,84 @@ ff = true ); } + #[test] + fn test_nested_user_only_key_redirects_generally() { + use crate::config::{ProjectConfig, UnknownWarning, UserConfig, collect_unknown_warnings}; + + // `[list]` is a valid *shared* section (project config accepts `url`), + // but `columns` / `full` are user-config display settings. Placing them + // in project config redirects to user config rather than reading + // "unknown field" — the general "valid in the other config" check, not + // the hard-coded commit.generation list (#3469). + let warnings = collect_unknown_warnings::( + "[list]\ncolumns = [\"branch\"]\nfull = true\n", + ); + assert!( + warnings.iter().all(|w| matches!( + w, + UnknownWarning::NestedWrongConfig { path, other_description } + if (path == "list.columns" || path == "list.full") + && *other_description == "user config" + )) && warnings.len() == 2, + "expected list.columns/list.full → user config, got {warnings:?}" + ); + + // A key unknown in *both* configs stays "unknown field". + let warnings = + collect_unknown_warnings::("[list]\nnonsense-typo = true\n"); + assert!( + matches!( + warnings.as_slice(), + [UnknownWarning::NestedUnknown { path }] if path == "list.nonsense-typo" + ), + "expected list.nonsense-typo → unknown, got {warnings:?}" + ); + + // The reverse direction: `url` is project-only, so it redirects to + // project config when found in user config — no scope-to-repo note + // there (that only applies to user-config destinations). + let warnings = collect_unknown_warnings::("[list]\nurl = \"x\"\n"); + assert!( + matches!( + warnings.as_slice(), + [UnknownWarning::NestedWrongConfig { path, other_description }] + if path == "list.url" && *other_description == "project config" + ), + "expected list.url → project config, got {warnings:?}" + ); + } + + #[test] + fn test_scope_to_repo_note_only_for_user_config() { + // The note fires for user-config destinations and nothing else. + assert!(scope_to_repo_note("user config").is_some()); + assert!(scope_to_repo_note("project config").is_none()); + + // It reaches the rendered load-warning for a user-config redirect... + let note = "to scope it to this repo"; + let msg = format_load_warning( + "Project config", + &crate::config::UnknownWarning::NestedWrongConfig { + path: "list.columns".to_string(), + other_description: "user config", + }, + ); + assert!(msg.contains(note), "user-config redirect should carry note: {msg}"); + + // ...but not a project-config redirect. + let msg = format_load_warning( + "User config", + &crate::config::UnknownWarning::NestedWrongConfig { + path: "list.url".to_string(), + other_description: "project config", + }, + ); + assert!( + !msg.contains(note), + "project-config redirect should not carry note: {msg}" + ); + } + /// Every `DEPRECATION_RULES` row's migration fires on one config, pinning /// cross-rule interactions the per-rule tests can't see — in particular /// the inserted `[forge]` staying in the mid-file spot where the user diff --git a/src/config/mod.rs b/src/config/mod.rs index 6c8c45944a..9ffc6f27d6 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -142,7 +142,7 @@ pub use deprecation::suppress_warnings; pub use deprecation::warnings_suppressed; pub use deprecation::{ DEPRECATED_SECTION_KEYS, DeprecatedSection, UnknownKeyKind, classify_unknown_key, - key_belongs_in, nested_key_belongs_in, warn_unknown_fields, + key_belongs_in, nested_key_belongs_in, scope_to_repo_note, warn_unknown_fields, }; pub use deprecation::{DeprecationKind, Deprecations}; pub use expansion::{ diff --git a/src/config/unknown_tree.rs b/src/config/unknown_tree.rs index 1389b8babd..228125f81f 100644 --- a/src/config/unknown_tree.rs +++ b/src/config/unknown_tree.rs @@ -177,6 +177,51 @@ pub enum UnknownWarning { NestedUnknown { path: String }, } +/// Position within `C::Other`'s unknown tree while walking `C`'s nested +/// unknowns. A nested key that's schema-unknown in `C` but *valid* in +/// `C::Other` (e.g. `list.columns` — a user-config display setting — placed in +/// project config) should redirect there rather than read "unknown field". We +/// answer "is this leaf valid in the other config?" by walking the other +/// config's unknown tree in lockstep: a key valid there never appears in that +/// tree, so its absence is the signal. +#[derive(Clone, Copy)] +enum OtherStatus<'a> { + /// An ancestor section is absent or wholly unknown in `C::Other`, so + /// nothing at or below this point is valid there. + UnknownSection, + /// Within a section that exists in `C::Other`. Carries the corresponding + /// node in the other tree, or `None` once no further unknowns are recorded + /// — i.e. every key at or below here is valid in the other config. + Known(Option<&'a UnknownTree>), +} + +impl<'a> OtherStatus<'a> { + /// Whether `key` at the current level is a valid key in `C::Other`. + fn key_is_valid_in_other(self, key: &str) -> bool { + match self { + OtherStatus::UnknownSection => false, + OtherStatus::Known(None) => true, + OtherStatus::Known(Some(node)) => !node.keys.contains(key), + } + } + + /// Descend into `key`, returning the status for its children. + fn descend(self, key: &str) -> OtherStatus<'a> { + match self { + OtherStatus::UnknownSection => OtherStatus::UnknownSection, + OtherStatus::Known(None) => OtherStatus::Known(None), + OtherStatus::Known(Some(node)) => { + if node.keys.contains(key) { + // Whole subtree is unknown in the other config too. + OtherStatus::UnknownSection + } else { + OtherStatus::Known(node.nested.get(key)) + } + } + } + } +} + /// Collect structured warnings for `raw_contents` under config type `C`. /// /// Top-level classification reads the *raw* tree (so deprecated top-level @@ -185,6 +230,13 @@ pub enum UnknownWarning { /// so patterns the deprecation system already warns about (e.g., /// `switch.no-cd`, `merge.no-ff`) don't double-warn here. /// +/// A misplaced *nested* key is redirected to `C::Other` when it's valid there +/// — determined by walking `C::Other`'s own unknown tree for the same content +/// (see [`OtherStatus`]). If that other-config analysis is unreliable, the +/// walk falls back to treating nested keys as unknown, so only the hard-coded +/// [`nested_key_belongs_in`](crate::config::nested_key_belongs_in) redirects +/// still fire. +/// /// Returns an empty vec if either analysis is unreliable — the load path /// surfaces parse/type errors elsewhere. pub fn collect_unknown_warnings(raw_contents: &str) -> Vec { @@ -197,6 +249,13 @@ pub fn collect_unknown_warnings(raw_contents: &str) -> Vec t, UnknownAnalysis::Unreliable(_) => return Vec::new(), }; + // The same content viewed as the *other* config type: a nested key absent + // from this tree is valid there. Unreliable → no generalized redirect. + let other_analysis = compute_unknown_tree::(&migrated); + let other_root = match other_analysis.warn_tree() { + Some(t) => OtherStatus::Known(Some(t)), + None => OtherStatus::UnknownSection, + }; let mut out = Vec::new(); for key in &raw_tree.keys { @@ -225,7 +284,7 @@ pub fn collect_unknown_warnings(raw_contents: &str) -> Vec(sub, key, &mut out); + walk_nested::(sub, key, other_root.descend(key), &mut out); } out } @@ -233,11 +292,17 @@ pub fn collect_unknown_warnings(raw_contents: &str) -> Vec( tree: &UnknownTree, prefix: &str, + other: OtherStatus, out: &mut Vec, ) { for key in &tree.keys { let path = format!("{prefix}.{key}"); - out.push(match crate::config::nested_key_belongs_in::(&path) { + // Hard-coded redirects (commit.generation leaves) take precedence and + // fire even when the other-config analysis is unreliable; otherwise + // fall back to the general "valid in the other config" check. + let belongs = crate::config::nested_key_belongs_in::(&path) + .or_else(|| other.key_is_valid_in_other(key).then(C::Other::description)); + out.push(match belongs { Some(other_description) => UnknownWarning::NestedWrongConfig { path, other_description, @@ -250,7 +315,7 @@ fn walk_nested( continue; } let path = format!("{prefix}.{key}"); - walk_nested::(sub, &path, out); + walk_nested::(sub, &path, other.descend(key), out); } } diff --git a/tests/snapshots/integration__integration_tests__config_show__config_show_suggests_user_config_for_commit_generation.snap b/tests/snapshots/integration__integration_tests__config_show__config_show_suggests_user_config_for_commit_generation.snap index 2f8f4d8881..a37dc5ee28 100644 --- a/tests/snapshots/integration__integration_tests__config_show__config_show_suggests_user_config_for_commit_generation.snap +++ b/tests/snapshots/integration__integration_tests__config_show__config_show_suggests_user_config_for_commit_generation.snap @@ -79,7 +79,7 @@ exit_code: 0   \ No newline at end of file   +[commit.generation]   +command = "claude" -▲ Key commit.generation.command belongs in user config (will be ignored) +▲ Key commit.generation.command belongs in user config (will be ignored); to scope it to this repo, add it under [projects.""] in user config SHELL INTEGRATION ▲ Shell integration not configured From 78b5bc37d55c000299aaa3505f5f47dad31b8df3 Mon Sep 17 00:00:00 2001 From: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:26:14 +0000 Subject: [PATCH 2/5] style: rustfmt new config warning tests --- src/config/deprecation.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/config/deprecation.rs b/src/config/deprecation.rs index 4872c268e0..c4f71d2c71 100644 --- a/src/config/deprecation.rs +++ b/src/config/deprecation.rs @@ -4544,8 +4544,7 @@ ff = true ); // A key unknown in *both* configs stays "unknown field". - let warnings = - collect_unknown_warnings::("[list]\nnonsense-typo = true\n"); + let warnings = collect_unknown_warnings::("[list]\nnonsense-typo = true\n"); assert!( matches!( warnings.as_slice(), @@ -4583,7 +4582,10 @@ ff = true other_description: "user config", }, ); - assert!(msg.contains(note), "user-config redirect should carry note: {msg}"); + assert!( + msg.contains(note), + "user-config redirect should carry note: {msg}" + ); // ...but not a project-config redirect. let msg = format_load_warning( From cde2eb2bb140ecb6d8a855550421930154f08959 Mon Sep 17 00:00:00 2001 From: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:37:17 +0000 Subject: [PATCH 3/5] docs(config): drop private intra-doc link to OtherStatus --- src/config/unknown_tree.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/config/unknown_tree.rs b/src/config/unknown_tree.rs index 228125f81f..5f3f06ad7f 100644 --- a/src/config/unknown_tree.rs +++ b/src/config/unknown_tree.rs @@ -232,8 +232,9 @@ impl<'a> OtherStatus<'a> { /// /// A misplaced *nested* key is redirected to `C::Other` when it's valid there /// — determined by walking `C::Other`'s own unknown tree for the same content -/// (see [`OtherStatus`]). If that other-config analysis is unreliable, the -/// walk falls back to treating nested keys as unknown, so only the hard-coded +/// (see the private `OtherStatus` helper). If that other-config analysis is +/// unreliable, the walk falls back to treating nested keys as unknown, so only +/// the hard-coded /// [`nested_key_belongs_in`](crate::config::nested_key_belongs_in) redirects /// still fire. /// From cacbefc3dc8d26f519ff0765fa9e319ef3014afd Mon Sep 17 00:00:00 2001 From: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:49:10 +0000 Subject: [PATCH 4/5] test(config): update inline warning snapshots for the scope note --- src/commands/config/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/commands/config/mod.rs b/src/commands/config/mod.rs index 18ad243499..4140fbadc3 100644 --- a/src/commands/config/mod.rs +++ b/src/commands/config/mod.rs @@ -147,7 +147,7 @@ mod tests { // skip-shell-integration-prompt in project config should suggest user config assert_snapshot!( warn_unknown_keys::("skip-shell-integration-prompt = true\n"), - @"▲ Key skip-shell-integration-prompt belongs in user config (will be ignored)"); + @"▲ Key skip-shell-integration-prompt belongs in user config (will be ignored); to scope it to this repo, add it under [projects.\"\"] in user config"); // forge in user config should suggest project config assert_snapshot!(warn_unknown_keys::("[forge]\nplatform = \"github\"\n"), @"▲ Key forge belongs in project config (will be ignored)"); @@ -162,10 +162,10 @@ mod tests { // `template-append`) so the offending key surfaces nested. assert_snapshot!( warn_unknown_keys::("[commit-generation]\ncommand = \"llm\"\n"), - @"▲ Key commit.generation.command belongs in user config (will be ignored)"); + @"▲ Key commit.generation.command belongs in user config (will be ignored); to scope it to this repo, add it under [projects.\"\"] in user config"); assert_snapshot!( warn_unknown_keys::("[commit.generation]\ncommand = \"llm\"\n"), - @"▲ Key commit.generation.command belongs in user config (will be ignored)"); + @"▲ Key commit.generation.command belongs in user config (will be ignored); to scope it to this repo, add it under [projects.\"\"] in user config"); // The one project-valid key in the section is unaffected. assert!( From 088eb4529aa290c8d033990349256da9bc59aec6 Mon Sep 17 00:00:00 2001 From: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Date: Wed, 15 Jul 2026 04:58:04 +0000 Subject: [PATCH 5/5] fix(config): only add scope note for keys with a [projects.""] form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scope note advised placing a misplaced user-config key under [projects.""], but that table (UserProjectOverrides) has no skip-shell-integration-prompt / skip-commit-generation-prompt fields — following the note yielded a fresh "unknown field", and these are global first-run flags with no per-repo semantics anyway. Gate the note on is_user_project_override_key(top_level). Also consolidate the duplicated with_scope_note helper into one exported definition. --- src/commands/config/mod.rs | 2 +- src/commands/config/show.rs | 16 +++------- src/config/deprecation.rs | 59 +++++++++++++++++++++++++++++-------- src/config/mod.rs | 14 +++++++++ 4 files changed, 66 insertions(+), 25 deletions(-) diff --git a/src/commands/config/mod.rs b/src/commands/config/mod.rs index 4140fbadc3..be50e1ba4d 100644 --- a/src/commands/config/mod.rs +++ b/src/commands/config/mod.rs @@ -147,7 +147,7 @@ mod tests { // skip-shell-integration-prompt in project config should suggest user config assert_snapshot!( warn_unknown_keys::("skip-shell-integration-prompt = true\n"), - @"▲ Key skip-shell-integration-prompt belongs in user config (will be ignored); to scope it to this repo, add it under [projects.\"\"] in user config"); + @"▲ Key skip-shell-integration-prompt belongs in user config (will be ignored)"); // forge in user config should suggest project config assert_snapshot!(warn_unknown_keys::("[forge]\nplatform = \"github\"\n"), @"▲ Key forge belongs in project config (will be ignored)"); diff --git a/src/commands/config/show.rs b/src/commands/config/show.rs index 16e8735e2a..831451e06a 100644 --- a/src/commands/config/show.rs +++ b/src/commands/config/show.rs @@ -766,9 +766,10 @@ fn format_show_warning(warning: &worktrunk::config::UnknownWarning) -> String { UnknownWarning::TopLevelWrongConfig { key, other_description, - } => with_scope_note( + } => worktrunk::config::with_scope_note( cformat!("Key {key} belongs in {other_description} (will be ignored)"), other_description, + key, ), UnknownWarning::TopLevelDeprecatedWrongConfig { key, @@ -778,9 +779,10 @@ fn format_show_warning(warning: &worktrunk::config::UnknownWarning) -> String { UnknownWarning::NestedWrongConfig { path, other_description, - } => with_scope_note( + } => worktrunk::config::with_scope_note( cformat!("Key {path} belongs in {other_description} (will be ignored)"), other_description, + path, ), UnknownWarning::NestedUnknown { path } => { cformat!("Unknown key {path} will be ignored") @@ -788,16 +790,6 @@ fn format_show_warning(warning: &worktrunk::config::UnknownWarning) -> String { } } -/// Append the project-scoped-user-config note when the key belongs in user -/// config, mirroring the load-time warning path. See -/// [`scope_to_repo_note`](worktrunk::config::scope_to_repo_note). -fn with_scope_note(message: String, other_description: &str) -> String { - match worktrunk::config::scope_to_repo_note(other_description) { - Some(note) => format!("{message}; {note}"), - None => message, - } -} - fn render_project_config(out: &mut String) -> anyhow::Result<()> { // Try to get current repository root let repo = match Repository::current() { diff --git a/src/config/deprecation.rs b/src/config/deprecation.rs index c4f71d2c71..8e2ff3d75f 100644 --- a/src/config/deprecation.rs +++ b/src/config/deprecation.rs @@ -1792,12 +1792,22 @@ pub fn nested_key_belongs_in(path: &str) -> Option<&'static /// scope a personal setting to one repo, which the `[projects.""]` table /// in user config does directly. Returns `None` for any other destination. /// +/// `key` is the misplaced key (`worktree-path`, or a dotted path like +/// `list.columns`); the note only fires when its top-level segment is a field +/// of that table (see [`is_user_project_override_key`](crate::config::is_user_project_override_key)). +/// Root-only user settings — `skip-shell-integration-prompt`, +/// `skip-commit-generation-prompt` — have no `[projects.""]` form and no +/// per-repo semantics, so following the note would just produce a fresh +/// "unknown field"; they get no note. +/// /// Keyed off the destination *description* rather than a config-type gate so /// both warning formatters (load-time and `config show`) can share it — they /// hold only the `other_description` string, not the config type. -pub fn scope_to_repo_note(other_description: &str) -> Option<&'static str> { - (other_description == crate::config::UserConfig::description()) - .then_some(r#"to scope it to this repo, add it under [projects.""] in user config"#) +pub fn scope_to_repo_note(other_description: &str, key: &str) -> Option<&'static str> { + let top_level = key.split('.').next().unwrap_or(key); + (other_description == crate::config::UserConfig::description() + && crate::config::is_user_project_override_key(top_level)) + .then_some(r#"to scope it to this repo, add it under [projects.""] in user config"#) } /// Classification of an unknown config key for warning purposes. @@ -1889,6 +1899,7 @@ fn format_load_warning(label: &str, warning: &crate::config::UnknownWarning) -> "{label} has key {key} which belongs in {other_description} (will be ignored)" ), other_description, + key, ), UnknownWarning::TopLevelDeprecatedWrongConfig { key, @@ -1905,6 +1916,7 @@ fn format_load_warning(label: &str, warning: &crate::config::UnknownWarning) -> "{label} has key {path} which belongs in {other_description} (will be ignored)" ), other_description, + path, ), UnknownWarning::NestedUnknown { path } => { cformat!("{label} has unknown field {path} (will be ignored)") @@ -1912,12 +1924,14 @@ fn format_load_warning(label: &str, warning: &crate::config::UnknownWarning) -> } } -/// Append the project-scoped-user-config note to `message` when the key's -/// destination is user config (see [`scope_to_repo_note`]). Joined with a -/// semicolon per the house style for related clauses. The note is plain text -/// so its `[projects.""]` placeholder isn't parsed as color-print markup. -fn with_scope_note(message: String, other_description: &str) -> String { - match scope_to_repo_note(other_description) { +/// Append the project-scoped-user-config note to `message` when the misplaced +/// `key`'s destination is user config and it's a `[projects.""]` field +/// (see [`scope_to_repo_note`]). Joined with a semicolon per the house style +/// for related clauses. The note is plain text so its `[projects.""]` +/// placeholder isn't parsed as color-print markup. Shared with `config show` +/// via [`crate::config::with_scope_note`] so the caveat lives in one place. +pub fn with_scope_note(message: String, other_description: &str, key: &str) -> String { + match scope_to_repo_note(other_description, key) { Some(note) => format!("{message}; {note}"), None => message, } @@ -4569,9 +4583,16 @@ ff = true #[test] fn test_scope_to_repo_note_only_for_user_config() { - // The note fires for user-config destinations and nothing else. - assert!(scope_to_repo_note("user config").is_some()); - assert!(scope_to_repo_note("project config").is_none()); + // The note fires for user-config destinations whose top-level key is a + // `[projects.""]` field, and nothing else. + assert!(scope_to_repo_note("user config", "list.columns").is_some()); + assert!(scope_to_repo_note("user config", "worktree-path").is_some()); + assert!(scope_to_repo_note("project config", "list.url").is_none()); + + // Root-only user scalars have no `[projects.""]` form, so following + // the note would just yield a fresh "unknown field" — no note. + assert!(scope_to_repo_note("user config", "skip-shell-integration-prompt").is_none()); + assert!(scope_to_repo_note("user config", "skip-commit-generation-prompt").is_none()); // It reaches the rendered load-warning for a user-config redirect... let note = "to scope it to this repo"; @@ -4599,6 +4620,20 @@ ff = true !msg.contains(note), "project-config redirect should not carry note: {msg}" ); + + // ...and not a misplaced root-only user scalar, even though its + // destination is user config. + let msg = format_load_warning( + "Project config", + &crate::config::UnknownWarning::TopLevelWrongConfig { + key: "skip-shell-integration-prompt".to_string(), + other_description: "user config", + }, + ); + assert!( + !msg.contains(note), + "root-only user scalar should not carry note: {msg}" + ); } /// Every `DEPRECATION_RULES` row's migration fires on one config, pinning diff --git a/src/config/mod.rs b/src/config/mod.rs index 9ffc6f27d6..3d5de9ef41 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -123,6 +123,19 @@ pub(crate) fn schema_top_level_keys() -> Vec { keys } +/// Whether `key` is a top-level field of a `[projects.""]` table (the +/// [`UserProjectOverrides`] schema). Used to gate the "scope it to this repo" +/// note: a misplaced user-config key only earns that advice when it can +/// actually be placed under `[projects.""]`. Root-only user settings such +/// as `skip-shell-integration-prompt` are absent here and get no note. +pub fn is_user_project_override_key(key: &str) -> bool { + use std::sync::OnceLock; + static KEYS: OnceLock> = OnceLock::new(); + KEYS.get_or_init(schema_top_level_keys::) + .iter() + .any(|k| k == key) +} + // Re-export public types pub use approvals::{Approvals, approvals_path, require_approvals_path}; pub use commands::{Command, CommandConfig, HookStep, append_aliases}; @@ -143,6 +156,7 @@ pub use deprecation::warnings_suppressed; pub use deprecation::{ DEPRECATED_SECTION_KEYS, DeprecatedSection, UnknownKeyKind, classify_unknown_key, key_belongs_in, nested_key_belongs_in, scope_to_repo_note, warn_unknown_fields, + with_scope_note, }; pub use deprecation::{DeprecationKind, Deprecations}; pub use expansion::{