`jj` should warn about unrecognized configuration settings

[arxanas]

Description

See https://discord.com/channels/968932220549103686/969829516539228222/1069406648600375347 for an example, where the user set ui.graph.format but actually wanted ui.graph.style. jj should preemptively warn about configuration settings that don't exist, or perhaps at least have a command to lint your configuration files.

Steps to Reproduce the Problem

1. Set a value for a configuration setting that doesn't exist.

Expected Behavior

• jj should warn about unused configuration settings. (cargo does this, which is helpful.)

Actual Behavior

• Configuration setting is unused without any diagnostic.

[arxanas]

Possibly interesting, although I don't know if we use serde: https://github.com/dtolnay/serde-ignored

[Ralith]

This seems very difficult/fragile so long as jj is using ad-hoc strings to identify configuration options, especially across the lib/cli boundary. Moving towards a statically defined struct of all options used in the CLI, combined with serde-ignored seems like a good approach.

[ilyagr]

In case we stick with strings, there are some imperfect but possibly good enough solutions.

Thinking out loud, https://github.com/martinvonz/jj/blob/main/src/commands/config-schema.json could be helpful here.

Or, we could have a (possibly imperfect) whitelist of acceptable settings and warn when we see a setting outside the whitelist. We could rely on tests and the default config for this whitelist not to become stale.

The whitelist would have to recognize, for example, that [merge-tools] can contain a key with any name.

If this whitelist can be expressed as a Rust type, we're back to the statically defined struct idea and should stick with that. A whitelist is more flexible in some ways, though. For example, in tests you could expand the whitelist to include test-only options.

[samueltardieu]

The other difficulty in using serde, even with serde-ignored, is that in the future we might want to be able to write the configuration file (think jj config --global user.email foo@example.com). We would need to write back the unknown keys and values because they might be used by external tools used along with jj.

[dbarnett]

@martinvonz have you done any thinking on how to extend the config with custom company-internal config options, similar to #1069 for custom global flags? Would that involve patching/linking different rust code, or something else?

I'm not sure if serde would be flexible or pluggable enough, but I'd imagined encapsulating all the config data access into some DAO or loader type that could (a) centralize which config values are "known" or "unknown" and (b) provide some higher-level abstraction over the more complex structures and relationships (like ui.merge-editor + corresponding merge-tools entries). As long as that could be extended to be aware of custom config options early in runtime, that could help offer warnings for unrecognized settings.

[martinvonz]

@martinvonz have you done any thinking on how to extend the config with custom company-internal config options, similar to #1069 for custom global flags? Would that involve patching/linking different rust code, or something else?

Yes, what we do internally is to simply build a custom jj binary similar to how the example applications work. I haven't thought much about how it should work. It sounds like you have thought at least as much about it as I have :) What you suggested above sounds like what I had imagined.

We should keep the separation between library and application in mind when doing that refactoring. The library crate currently cares only about the [user], [operation], and [debug] sections, IIRC. Still, the UserSettings object, which is defined in the library, has accessors for many things that are clearly about the UI. It's possible that we should even pass in the user/operation/debug data to the library in a specialized struct, or maybe via a trait. We can of course still provide a utility for reading those values from configs.