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.
I think this provides a better UX than refusing any operation due to large
files. Because untracked files won't be overwritten, it's usually safe to
continue operation ignoring the untracked files. One caveat is that new large
files can become tracked if files of the same name checked out. (see the test
case)
FWIW, the warning will be printed only once if watchman is enabled. If we use
the snapshot stats to print untracked paths in "jj status", this will be a
problem.
Closes#3616, #3912
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!().
It's annoying that rustfmt indents a large code block depending on the length
of parameters and method calls needed to configure the parallel iterator.
.get_table() callers will be migrated to .table_keys() + .get() to attach source
indication to error message. It might be a bit more expensive, but we can get by
without introducing table wrapper that tracks source config layers.
Since config::Value type was lax about data types, an integer value could be
deserialized through a string. This won't apply to new toml_edit-based value
types.
I'm going to rewrite the deserialization constructor to accept either integer
or string, but the from_str() API is generally useful.
This patch also fixes lifetime of the parse error message.
The serde::Deserialize interface isn't always useful. Suppose we're going to
make parsing of color tables stricter, we would have to process a string|table
value. It could be encoded as an untagged enum in serde, but that means the
error message has to be static str (e.g. "expected string or color table") and a
detailed error (e.g. "invalid color name: xxx") would be lost. It's way easier
to dispatch based on the ConfigValue type, than on serde data model.
The current implementation is silly, which will be reimplemented to be using
toml_edit::DocumentMut. "jj config set" will probably be ported to this API.
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 added function is not "get_table(name) -> Result<Table, Error>" because
callers need to know whether the value was missing or shadowed by non-table
value. We just don't have this problem in template/revset-aliases because these
tables are top-level items.
When I wrote the original lookup function, I didn't notice that the root config
value could be accessed as &config.cache without cloning. That's the only reason
I added split_safe_prefix().
While this is a debug option, it doesn't make sense to store an integer value
as a string. We can parse the environment variable instead. The variable is
temporarily parsed into i64 because i64 is the integer type of TOML.
toml_edit::Value doesn't implement any other integer conversion functions.
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.
This patch adds separate .get<T>(), .get_table(), and .get_value() methods
because generic value types in toml_edit don't implement Deserialize.
There's a bit of naming inconsistency right now. Low-level generic object is
called an "item", whereas mid-level generic object is called a "value". These
objects will become different types when we migrate to toml_edit.
We usually look up config objects with a static name, but commands like "jj
config get" has to parse a user input as a config key. We could consolidate the
name type to &ConfigNamePathBuf, but it would be inconvenient if all callers had
to do config.get(&"..".parse()?). It's also undesirable to pass a raw user input
in to .get() as a string.
I originally considered making it support fallible conversion, but doing that
would complicate error handling path. Not all functions should return an error.
It's better to do fallible parsing at the call sites instead.
A string config name is parsed into a temporary ConfigNamePathBuf object. If the
allocation cost matters, the output path type can be abstracted.
This will help migrate away from config::Config. We'll probably need a better
abstraction to preserve error source indication, but let's leave that for now.
.get_table() isn't forwarded to .get::<T>() because ConfigTable won't implement
Deserialize if we migrate to toml_edit.
* GitFetch{} separates `fetch()` from `import_refs()`.
* This allows more control over the stages of the fetch so multiple fetches
can happen before `import_refs` resolves the changes in the local jj repo.
* Implement `git::fetch` in terms of the new api.
* Add a test case for initial fetch from a repo without `HEAD` set. This tests
the default branch retrieving behaviour.
Issue: #4923
UserSettings::get_*() will be changed to look up a merged value from
StackedConfig, not from a merged config::Value. This will help migrate away
from the config crate.
Not all tests are ported to ConfigLayer::parse() because it seemed a bit odd
to format!() a TOML document and parse it to build a table of configuration
variables.
