The same parsing function will be used for --config NAME=VALUE.
I don't think we'll add schema-based type inference anytime soon, so I moved
the value parsing to clap layer.
I don't think the new behavior is strictly better, but it's more consistent
with the other "jj config" commands so we can remove the special case for
"jj config edit".
If the user config path was an empty directory, these commands would fail with
EISDIR instead of "can't set config in path (dirs not supported)". I think this
can be changed later to create new "$dir/config.toml" file. Maybe we can also
change the default path to ~/.config/jj directory, and load all *.toml files
from there?
This patch adds simpler user/repo_config_path() accessors. existing_*_path()
are kinda implementation details (for testing), so they are now private methods.
new_user_config_path() will be removed later.
Since .get("path.to.non-table.children") returns NotFound, I made
.delete_value() not fail in that case. The path doesn't exist, so
.delete_value() should be noop.
remove_config_value_from_file() will be inlined to callers.
Before, "jj config get"/"list" and .get() functions processed inline tables as
tables (or directories in filesystem analogy), whereas "set"/"unset" processed
ones as values (or files.) This patch makes all commands and functions process
inline tables as values. We rarely use the inline table syntax, and it's very
hard to pack many (unrelated) values into an inline table. TOML doesn't allow
newlines between { .. }. Our common use case is to define color styles, which
wouldn't be meant to inherit attributes from the default settings.
The default pager setting is flattened in case user overrides pager.env without
changing the command args.
This patch does not change the handling of inline tables yet. Both inline and
non-inline tables are merged as before. OTOH, .set_value() is strict about table
types because it should refuse to overwrite a table whereas an inline table
should be overwritten as a value. This matches "jj config set"/"unset"
semantics. rules_from_config() in formatter.rs uses .as_inline_table(), which is
valid because toml_edit::Value type never contains non-inline table.
Since toml_edit::Value doesn't implement PartialEq, stacking tests now use
insta::assert_snapshot!().
Since most callers don't need to handle loading/parsing errors, it's probably
better to add a separate error type for "get" operations. The other uses of
ConfigError will be migrated later.
Since ConfigGetError can preserve the source name/path information, this patch
also removes some ad-hock error handling codes.
The goal is to remove dependency on config::Config and replace the underlying
table type to toml_edit::Table. Other StackedConfig::merge() users will be
migrated in the next batch.
Allow unsetting config values similar to `git config unset`.
```bash
$ jj config set --user some-key some-val
$ jj config get some-key
some-val
$ jj config unset --user some-key
$ jj config get some-key
Config error: configuration property "some-key" not found
For help, see https://martinvonz.github.io/jj/latest/config/.
```
Unsetting a key, which is part of a table, might leave that table empty.
For now we do not delete such empty tables, as there may be cases where
an empty table is semantically meaningful
(https://github.com/martinvonz/jj/issues/4458#issuecomment-2407109269).
For example:
```toml
[table]
key = "value"
[another-table]
key = "value"
```
Running `jj config unset --user table.key` will leave us with `table`
being empty:
```toml
[table]
[another-table]
key = "value"
```
This addresses issue #3317, where as discussed we want to show the paths to
configuration files if they contain errors, to make it easier for the user to
locate them.
"config list NAME" argument is now parsed as TOML key, but it's still broken
since config.get() expects a query expression in different syntax.
The other config commands will be migrated later.
It's inconsistent that some warnings have headings and some don't, and it seems
the choice is arbitrary. Let's unify the style. There are two exceptions:
1. continued line following labeled message,
2. "unrecognized response" followed by prompt.
There's a caveat: "jj config list -Tname" will concatenate all names in a
single line. That's correct but useless. We might want some option or config
knob to complete missing "\n". This also applies to "log --no-graph".
this greatly speeds up the time to run all tests, at the cost of slightly larger recompile times for individual tests.
this unfortunately adds the requirement that all tests are listed in `runner.rs` for the crate.
to avoid forgetting, i've added a new test that ensures the directory is in sync with the file.
## benchmarks
before this change, recompiling all tests took 32-50 seconds and running a single test took 3.5 seconds:
```
; hyperfine 'touch lib/src/lib.rs && cargo t --test test_working_copy'
Time (mean ± σ): 3.543 s ± 0.168 s [User: 2.597 s, System: 1.262 s]
Range (min … max): 3.400 s … 3.847 s 10 runs
```
after this change, recompiling all tests take 4 seconds:
```
; hyperfine 'touch lib/src/lib.rs ; cargo t --test runner --no-run'
Time (mean ± σ): 4.055 s ± 0.123 s [User: 3.591 s, System: 1.593 s]
Range (min … max): 3.804 s … 4.159 s 10 runs
```
and running a single test takes about the same:
```
; hyperfine 'touch lib/src/lib.rs && cargo t --test runner -- test_working_copy'
Time (mean ± σ): 4.129 s ± 0.120 s [User: 3.636 s, System: 1.593 s]
Range (min … max): 3.933 s … 4.346 s 10 runs
```
about 1.4 seconds of that is the time for the runner, of which .4 is the time for the linker. so
there may be room for further improving the times.
Before this patch, it was an error to run `jj config set --user foo
'[1]'` twice. But it's only been broken since the previous commit
because '[1]' was interpreted as a string before then.
