Skip to content

fix(config): redirect misplaced nested keys to the other config with a scope note#3473

Open
worktrunk-bot wants to merge 5 commits into
mainfrom
fix/issue-3469-wrong-config-note-29387712890
Open

fix(config): redirect misplaced nested keys to the other config with a scope note#3473
worktrunk-bot wants to merge 5 commits into
mainfrom
fix/issue-3469-wrong-config-note-29387712890

Conversation

@worktrunk-bot

Copy link
Copy Markdown
Collaborator

Addresses @max-sixty's request in #3469: surface a note when a config key only lives in the other config file, and reference project-specific user config when the destination is user config.

Problem

When @KieranP put [list] columns / [list.custom-columns] in project config (.config/wt.toml), the warning read a bare unknown field list.columns, giving no hint that these are user-config display settings. Nested-key redirection only fired for the hard-coded commit.generation leaves, so every other misplaced nested key fell through to "unknown field".

Change

  • Generalize nested-key redirection. A nested key that's schema-unknown in the loaded config but valid in the other config type now reports belongs in <other> config instead of unknown field. This is decided by walking the other config's own unknown tree for the same content — a key valid there simply never appears, so its absence is the signal. The existing hard-coded commit.generation redirect stays as a fast-path that still fires even when the other-config analysis is unreliable.
  • Reference project-specific user config. When the destination is user config, the warning appends ; to scope it to this repo, add it under [projects."<id>"] in user config — the [projects."<id>"] table is exactly how you keep a personal setting on one repo, which is what reaching for project config usually means.

Both the load-time warning and wt config show share the classification and the note.

Before / after

# Before
▲ Project config has unknown field list.columns (will be ignored)

# After
▲ Project config has key list.columns which belongs in user config (will be ignored); to scope it to this repo, add it under [projects."<id>"] in user config

A key that's genuinely unknown in both configs (a typo like list.nonsense-typo) still reads unknown field, and the reverse direction (list.url — project-only — placed in user config) redirects to project config with no scope note.

Tests

  • New unit tests in src/config/deprecation.rs: generalized nested redirect for list.columns/list.full, typo-stays-unknown, reverse-direction redirect, and the scope note fires for user-config destinations only.
  • Updated the config_show_suggests_user_config_for_commit_generation snapshot — the commit.generation.command redirect now consistently carries the note.
  • cargo clippy --all-targets clean; full lib + integration suites pass (two unrelated failures locally are network-egress / file-permission sandbox artifacts, not touched by this change).

…a scope note

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 <other> 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."<id>"]` table, which scopes a personal setting to one repo — the
usual reason a user reaches for project config in the first place.

Closes #3469.

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the generalized nested-key redirection. The OtherStatus lockstep walk is a clean way to answer "valid in the other config?" — traced it through the shared-section (list.columns → user), typo-stays-unknown, whole-section-unknown, and reverse-direction cases and it classifies each correctly. Unit tests pass locally (config::unknown_tree + config::deprecation, 174 passed). One accuracy gap and one minor cleanup:

The scope note is misdirecting for two top-level user-only keys. with_scope_note is now applied unconditionally to TopLevelWrongConfig as well as NestedWrongConfig (format_load_warning in src/config/deprecation.rs, format_show_warning in src/commands/config/show.rs), and scope_to_repo_note fires for any "user config" destination. But the note tells the user to add the key under [projects."<id>"], and UserProjectOverrides (the schema for that table, in src/config/user/sections.rs) only carries worktree-path, list, commit, merge, remove, switch, step, aliases, approved-commands, and hooks. The two top-level scalar user keys — skip-shell-integration-prompt and skip-commit-generation-prompt — aren't fields there. So putting either in project config now yields:

▲ Project config has key skip-shell-integration-prompt which belongs in user config (will be ignored); to scope it to this repo, add it under [projects."<id>"] in user config

…and following that advice produces a fresh unknown field skip-shell-integration-prompt under [projects."<id>"] — plus these two are global first-run flags with no per-repo semantics to begin with. Every section-based redirect (list.*, commit.generation.*, merge.*, …) and worktree-path is genuinely per-project-overridable, so the note is correct for those; the gap is just these two scalars. Since the note only carries other_description, gating it correctly means threading through whether the specific key is a UserProjectOverrides field (or, more cheaply, only appending it on the NestedWrongConfig arm, since the two problem keys are top-level — at the cost of dropping the correct note for a misplaced top-level worktree-path). Flagging for your call rather than pushing, since it's a design tradeoff, not a mechanical fix.

Minor: with_scope_note is duplicated verbatim. The helper is byte-identical in src/config/deprecation.rs and src/commands/config/show.rs (only the path to scope_to_repo_note differs). Since scope_to_repo_note was already made pub and re-exported through src/config/mod.rs for exactly this sharing, the wrapper could be exported alongside it and called from show.rs, keeping the color-print caveat ("note is plain text so [projects."<id>"] isn't parsed as markup") attached to a single definition.

The scope note advised placing a misplaced user-config key under
[projects."<id>"], 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.
@worktrunk-bot

Copy link
Copy Markdown
Collaborator Author

Addressed both points in 088eb45.

Scope-note accuracy gap. Took the accurate option rather than the cheaper NestedWrongConfig-only gate — the note now fires only when the misplaced key's top-level segment is an actual [projects."<id>"] field. scope_to_repo_note(other_description, key) gains the key and checks is_user_project_override_key(top_level) (derived from the UserProjectOverrides schema via schema_top_level_keys). This keeps the note for every genuinely per-project-overridable redirect — list.*, commit.generation.*, merge.*, and top-level worktree-path — while dropping it for the two root-only first-run scalars.

Verified end-to-end against a project .config/wt.toml:

▲ Key skip-shell-integration-prompt belongs in user config (will be ignored)
▲ Key list.columns belongs in user config (will be ignored); to scope it to this repo, add it under [projects."<id>"] in user config

Duplicated with_scope_note. Consolidated into one exported definition in src/config/deprecation.rs (re-exported through src/config/mod.rs); show.rs now calls worktrunk::config::with_scope_note, so the color-print caveat lives in a single place.

Also corrected the skip-shell-integration-prompt inline snapshot in src/commands/config/mod.rs — a concurrent commit (cacbefc) had updated it to carry the note; with the gap fixed it correctly reads without one. The commit.generation.command snapshots keep the note (commit is a valid override). New unit coverage asserts the note fires for list.columns/worktree-path and stays absent for both skip-*-prompt scalars.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant