This will simplify path comparison in config resolver. <cwd>/.. shouldn't match
path prefix <cwd>. Maybe we should do path canonicalization globally (or never
do canonicalization), but I'm not sure where that should be made.
We can usually omit quotes in --config=NAME=VALUE, but an RFC3339 string is a
valid TOML date-time expression. It's weird that quoting is required to specify
a date-time value.
This will simplify handling of multiple ConfigArg items of the same type.
Consecutive --config NAME=VALUE arguments will be inserted to the same
ConfigLayer.
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.
The `Signature.email()` method is also updated to return the new Email
type. The `Signature.username()` method is deprecated for
`Signature.email().local()`.
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.
They are short, and it wouldn't make much sense to do load, mutate one entry,
and save in one function.
In later patches, "jj config set"/"unset" will be changed to reuse the loaded
ConfigLayer if the layer can be unambiguously selected.
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.
I followed the recommendation in the `jj new` doc to use `jj new main @`
to make a merge commit and ended up with a merge commit that GitHub did
not like. The PR diff included both the relevant changes from that
branch plus everything I merged in. @papertigers pointed out that
swapping the two args produces a merge commit GitHub understands better.
Happy to add a line explaining that the order matters, but it might be
too much detail for this spot. The linked doc
https://martinvonz.github.io/jj/latest/working-copy/ also does not
explain this.
This would be useful for scripting purpose. Maybe we can also replace the
current --config-toml=<TOML> use cases by --config-file=<PATH> and simpler
--config=<KEY>=<VALUE>.
https://github.com/martinvonz/jj/issues/4926#issuecomment-2506672165
If we want to add more source variants (such as fd number), it might be better
to add --config-from=<type>:<path|fd|..>. In any case, we'll probably want
--config=<KEY>=<VALUE>, and therefore, we'll need to merge more than one
--config* arguments.
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.
As these tests show, we sometimes print an error when trying to rebase
an empty set, and sometimes we don't say anything at all. It seems to
me like we should say "Nothing changed" in all of these cases.
As Martin spotted, the original code can't prevent "1.0GiB, maximum size allowed
is ~1.0GiB." I personally don't mind if the error message contained the exact
size, so I simply let it print both exact and human byte sizes unconditionally.
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
It wasn't obvious which io::Error was mapped to a "file creation" error.
Perhaps, file creation will be moved to caller, but let's make the error
handling explicit so we'll remove the unused error variant later.
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!().
I think the idea of the code was try do 30 updates per second even if
events arrive at, say, every 20 milliseconds. If we had reset the
timer every time we printed, we would otherwise reset the timer every
40 milliseconds and end up with 25 updates per second. However, a bug
in the code caused it to print every update because it always set the
threshold to print the next update to `now`. I tried to keep what I
think was the intent of the original code while fixing the bug.
The progress bar is supposed to update only 30 times per second, but
as was reported in #5057, that doesn't seem to be what's
happening. This patch adds tests showing how we update the progress
bar for every event.
Both Mercurial and Git (xdiff) have a special case for empty hunks.
https://repo.mercurial-scm.org/hg/rev/2b1ec74c961f
I also changed the internal line numbers to start from 0 so we wouldn't have
to think about whether "N - 1" would underflow.
Fixes#5049
Appears that "cargo test" parses indented text as a code block, and fails to
run doc tests. Spotted by running "cargo insta test". This doc comment is a CLI
help which is usually rendered to console, so I think markdown annotation should
be minimal.
This backs out commit ed84468cb8, "docs: in `jj help util exec`, use Markdown
`warning` admonition."
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.
We're scraping the CLI help text and rendering it as markdown, so we can use an "admonition" to have this warning text render nicer in the web documentation.
You could argue that `!!! warning` is a little weird to see on the CLI. Some alternatives:
- We could opt to not design the CLI help text around markdown and skip the change to the `jj util exec` help in this commit.
- We could adopt some kind of format that can be rendered well in both contexts.
- Could sticking to specific formatting constructs by convention.
- Could use/create an actual translation tool from CLI format to Markdwon.
- We could keep separate versions of web and CLI documentation. (Seems like a bad idea for the foreseeable future, because we don't have the resources to constantly keep both up-to-date and sync.)
I'm in favor of just writing Markdown in the CLI help text for now.
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.
This patch removes pre-merge steps which depend on the ConfigBuilder API.
Some of Vec<ConfigLayer> could be changed to Vec<config::Config> (or
Vec<ConfigTable>) to drop the layer parameter, but I'm not sure which is
better. parse_config_args() will have to construct ConfigLayer (or ConfigTable +
Option<PathBuf>) if we add support for --config-file=PATH argument.
The `NO_COLOR` spec says that user-specified config is supposed to
override the `$NO_COLOR` environment variable, and we do correctly use
the `ColorFormatter` when `ui.color= "always"` is set in the user's
config. However, it turns out that `NO_COLOR=1` still resulted in no
color because `crossterm` also respects the variable, so the color
codes the `ColorFormatter` requested had no effect. Since `crossterm`
doesn't know about our user configs, it cannot decide whether to
respect `$NO_COLOR`, so let's tell `crossterm` to always use the
colors we tell it to use.
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.
We currently ignore lines prefixed with "JJ: " (including the space)
in commit messages and in the list of sparse paths from `jj sparse
edit`. I think I included the trailing space in the prefix simply
because that's how we render comments line we insert before we ask the
user to edit the file. However, as #5004 says, Git doesn't require a
space after their "#" prefix. Neither does Mercurial after their "HG:"
prefix. So let's follow their lead and not require the trailing
space. Seems useful especially for people who have their editor
configured to strip trailing spaces.
Some Git merge drivers can partially resolve conflicts, leaving some
conflict markers in the output file. In that case, they exit with a code
between 1 and 127 to indicate that there was a conflict remaining in the
file (since Git uses a shell to run the merge drivers, and shells often
use exit codes above 127 for special meanings such as exiting due to a
signal):
https://git-scm.com/docs/gitattributes#_defining_a_custom_merge_driver
We should support this convention as well, since it will allow many Git
merge drivers to be used with Jujutsu, but we don't run our merge tools
through a shell, so there is no reason to treat exit codes 1 through 127
specially. Instead, we let the user specify which exact exit codes
should indicate conflicts. This is also better for cross-platform
support, since Windows may use different exit codes than Linux for
instance.
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().
I would like to delete this alias altogether. By moving it to the
config to start with, users can start overriding it themselves, as
`commit` or whatever they prefer.
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 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.
If there are multiple files in a subdirectory that are candidates for
completion, only complete the common directory prefix to reduce the number of
completion candidates shown at once.
This matches the normal shell completion of file paths.
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.
This is heavily based on Benjamin Tan's fish completions:
https://gist.github.com/bnjmnt4n/9f47082b8b6e6ed2b2a805a1516090c8
Some differences include:
- The end of a `--from`, `--to` ranges is also considered.
- `jj log` is not completed (yet). It has a different `--revisions` argument
that requires some special handling.
I left the "merge-tool-edits-conflict-markers" option unchanged,
since removing it would likely break some existing configurations. It
also seems like it could be useful to have a merge tool use the default
conflict markers instead of requiring the conflict marker style to
always be set for the merge tool (e.g. if a merge tool allows the user
to manually edit the conflicts).
This will help replace cli LayeredConfigs with new StackedConfig. I originally
considered moving this function to jj-lib, but the current API is very specific
to the CLI use case, and wouldn't be reused for building a merged sub table. We
might instead want to extract some bits as a library function.
This adds a new `revsets.simplify-parents` configuration option (similar
to `revsets.fix`) which serves as the default revset for `jj
simplify-parents` if no `--source` or `--revisions` arguments are
provided.
Layers are now constructed per file, not per source type. This will allow us
to report precise position where bad configuration variable is set. Because
layers are now created per file, it makes sense to require existence of the
file, instead of ignoring missing files which would leave an empty layer in the
stack. The path existence is tested by ConfigEnv::existing_config_path(), so I
simply made the new load_file/dir() methods stricter. However, we still need
config::File::required(false) flag in order to accept /dev/null as an empty
TOML file.
The lib type is renamed to StackedConfig to avoid name conflicts. The cli
LayeredConfigs will probably be reorganized as an environment object that
builds a StackedConfig.
Adds a new "git" conflict marker style option. This option matches Git's
"diff3" conflict style, allowing these conflicts to be parsed by some
external tools that don't support JJ-style conflicts. If a conflict has
more than 2 sides, then it falls back to the similar "snapshot" conflict
marker style.
The conflict parsing code now supports parsing Git-style conflict
markers in addition to the normal JJ-style conflict markers, regardless
of the conflict marker style setting. This has the benefit of allowing
the user to switch the conflict marker style while they already have
conflicts checked out, and their old conflicts will still be parsed
correctly.
Example of "git" conflict markers:
```
<<<<<<< Side #1 (Conflict 1 of 1)
fn example(word: String) {
println!("word is {word}");
||||||| Base
fn example(w: String) {
println!("word is {w}");
=======
fn example(w: &str) {
println!("word is {w}");
>>>>>>> Side #2 (Conflict 1 of 1 ends)
}
```
Adds a new "snapshot" conflict marker style which returns a series of
snapshots, similar to Git's "diff3" conflict style. The "snapshot"
option uses a subset of the conflict hunk headers as the "diff" option
(it just doesn't use "%%%%%%%"), meaning that the two options are
trivially compatible with each other (i.e. a file materialized with
"snapshot" can be parsed with "diff" and vice versa).
Example of "snapshot" conflict markers:
```
<<<<<<< Conflict 1 of 1
+++++++ Contents of side #1
fn example(word: String) {
println!("word is {word}");
------- Contents of base
fn example(w: String) {
println!("word is {w}");
+++++++ Contents of side #2
fn example(w: &str) {
println!("word is {w}");
>>>>>>> Conflict 1 of 1 ends
}
```
Adds a new "ui.conflict-marker-style" config option. The "diff" option
is the default jj-style conflict markers with a snapshot and a series of
diffs to apply to the snapshot. New conflict marker style options will
be added in later commits.
The majority of the changes in this commit are from passing the config
option down to the code that materializes the conflicts.
Example of "diff" conflict markers:
```
<<<<<<< Conflict 1 of 1
+++++++ Contents of side #1
fn example(word: String) {
println!("word is {word}");
%%%%%%% Changes from base to side #2
-fn example(w: String) {
+fn example(w: &str) {
println!("word is {w}");
>>>>>>> Conflict 1 of 1 ends
}
```
There's a subtle difference in error message, but the conversion function to be
called is the same. The error message now includes "for key <key>", which is
nice.
.get_table() isn't implemented because it isn't cheap to build a HashMap,
and a table of an abstract Value type wouldn't be useful. Maybe we'll
instead provide an iterator of table keys.
.config() is renamed to .raw_config() to break existing callers.
Ui::with_config() is unchanged because I'm not sure if UserSettings should be
constructed earlier. I assume UserSettings will hold an immutable copy of
LayerdConfigs object, whereas Ui has to be initialized before all config layers
get loaded.
I'm planning to rewrite config store layer by leveraging toml_edit instead of
the config crate. It will allow us to merge config overlays in a way that
deprecated keys are resolved within a layer prior to merging, for example.
This patch moves ConfigNamePathBuf to jj-lib where new config API will be
hosted. We'll probably extract LayeredConfigs to this module, but we'll first
need to split environment dependencies from it.
The previous iteration of `jj simplify-parents` would only reparent the
commits in the target set in the `MutableRepo::transform_descendants`
callback. The subsequent `MutableRepo::rebase_descendants` call invoked
when the transaction is committed would then rebase all descendants of
the target set, which includes the commits in the target set again.
This commit updates the `MutableRepo::transform_descendants` callback to
perform rebasing of all descendants within the callback. All descendants
of the target set will only be rebased at most once.
I believe this was an oversight. "jj duplicate" should duplicate commits (=
patches), not trees.
This patch adds a separate test file because test_rewrite.rs is pretty big, and
we'll probably want to migrate CLI tests to jj-lib.
The working-copy revision is usually the latest commit, but it's not always
true. This patch ensures that the wc branch is emitted first so the graph node
order is less dependent on rewrites.
If you have multiple remotes to push to, you might want to keep some changes
(such as security patches) in your private fork. Git CLI has one upstream remote
per branch, but jj supports multiple tracking remotes, and therefore "jj git
push" can start tracking new remotes automatically.
This patch makes new bookmarks not eligible for push by default. I considered
adding a warning, but it's not always possible to interrupt the push shortly
after a warning is emitted.
--all implies --allow-new because otherwise it's equivalent to --tracked. It's
also easier to write a conflict rule with --all/--deleted/--tracked than with
two of them.
-c/--change doesn't require --allow-new because it is the flag to create new
tracking bookmark.
#1278
