write!(ui.hint(), ..) error is suppressed because it seemed weird if the
configuration error had io::Error variant. The write error isn't important
anyway.
This moves the config loading closer to CLI args where --tool=<name> option
will be processed. The factory function are proxied through the command helper
so that the base_ignores can be attached there later.
I'm going to make them be loaded by caller, and these newtypes will provide
extra compile-time safety (plus nicer API to be added later.) The error types
will be cleaned up later patches.
This gets rid of the last UserSettings dependency from edit_diff_external().
I'm going to remove it from edit_diff() too, and let callers pass a
preconfigured MergeTool struct instead.
These changes will make it easier to add --tool=<name> argument #2575.
Because the snapshot directory is removed at the end of the function, it doesn't
make sense to enable watchman in it. The max_new_file_size parameter might be
somewhat useful, but it's unlikely that the temporary directory contains
gigantic node_modules tree for example. OTOH, the base_ignores matters since it
may contain common ignore patterns like *~.
This eliminates most of the UserSettings dependencies from this function.
I'm going to add string.len() method which will return a length in bytes. The
number of the UTF-8 code points is useless metrics, and strings here are often
ASCII bytes, so let's simply use byte indices in substr().
If the given index is not at a char boundary, it will be rounded. I considered
making it an error, but that would be annoying. I would want to see something
printed by author.name().substr() even if it contained latin characters.
I've extracted index normalization function which might be used by other string
methods. The remaining part of substr() is trivial, so inlined it.
Before, --tool=:builtin argument was ignored and the tool was loaded from
"ui.diff.tool" option. Since there is no single builtin diff format, :builtin
doesn't make sense here. Maybe we can translate ":<format>" to the internal
diff format instead, but that will also mean "ui.diff.tool" and ".format" can
be merged.
This partially reverts 409356fa5b "merge_tools: enable `:builtin` as default
diff/merge editor."
I'm going to split get_tool_config() to fix "diff --tool=:builtin", and it
doesn't make sense to duplicate get_tool_config_from_args() per backing
get_tool_config() functions.
This make :builtin render conflicts as conflict markers instead of
panicking. To support conflicts properly, we also need to parse the
conflict markers (calling `update_from_content()`) after the user
closes the editor.
A runtime error will be printed inline. This simplifies error handling, and I
think it's better behavior overall. "jj log" won't be terminated just because
"gpg" crashed during signature verification for example. When property
evaluation failed, the error propagates to the closest template expression, and
the error message is printed there. Then, template output continues as long as
the output stream is open.
If we add revset() function for example, dynamic revset evaluation error will be
displayed inline. Static revset expression will still be processed at parsing
phase (to cache the evaluation result), and the error will be reported early.
One caveat: a string argument passed to e.g. .contains(needle) can be a
template, so the evaluation error would be swallowed there.
It was moved to CLI at 42252a2f00 "cli: on `jj init --git-repo=.`, use
relative path to `.git/`." As far as I can tell, .canonicalize() is needed
to calculate relative path, which is now processed differently in
Workspace::init_external_git() and GitBackend::init_external().
This reverts dc074363d1 "no-op: Move external git repo canonicalization into
Workspace::init_git_external." As I said in the PR comment, appending ".git"
is normalization of the user input, which is IMHO more appropriate to be done
in the CLI layer.
The translation from method error to keyword error can go wrong if the context
object had n-ary methods (n > 0), which isn't the case as of now. For
simplicity, arguments error is mapped to "self.<name>(..)" suggestion.
Local variables and "self" could be merged without using extra method, but
we'll need extend_*_candidates() to merge in symbol/function aliases anyway.
This seems more useful if aliases are nested. The innermost error usually
contains the problem, and the outer errors are contexts where aliases are
expanded.
Except for the generic list and template methods. We'll need a bit more
refactoring to migrate List<T> method builders to be compatible with
non-capturing fn() type.
The recovery commit we create when we run into a stale working copy
with a missing operation currently has an empty tree. Our commit
backend at Google creates an index of which files changed in each
commit. That gets really expensive when a commit deletes all files in
the repo, as these recovery commits do. So for our backend, it is much
better to make the recovery commit empty instead. That's what this
patch does.
It almost doesn't matter functionally what tree we use for it since we
don't care much about the current tree when snapshotting the working
copy. It does matter in a few cases, however. One case is for
conflicts. In that case, it's likely better to use the recovery
commit's parent as base tree (as we do by making the recovery commit
empty) than to use an empty tree, as that would guarantee that all
conflicts would be considered resolved. (Side note: perhaps we should
start looking at the current commit's parent instead of looking at the
current commit when snapshotting, but that's a topic for another day.)
When we abandon a working-copy commit, we create a new working-copy
commit on top. This behave is very useful, but it's not obvious. Let's
document it.
Thankfully, 2bbefcc338 (rewrite: default to not simplifying ancestor
merges) means that there are much fewer commands where we need to
document this behavior.
There are two major goals:
* provide typo hints in a similar way to revset
* make methods extensible
The created method table is bound to the 'repo lifetime because of the problem
described in the inline comment. It would be nice if we can build cachable
core method table for<'repo> CommitTemplateLanguage<'repo, '_>, but I couldn't
figure out how.
Add an option to list tracked branches only
This option keeps most of the current `--all` printing logic, but:
- Omits local Git-tracking branches by default (can be extended to
support filtering by remote).
- Skip over the branch altogether if it doesn't contain tracked remotes
- Don't print the untracked_remote_refs at the end
Usage:
`jj branch list -t`
`jj branch list --tracked`
`jj branch list --tracked <branch name>`
I think the user usually wants to abandon only newly empty commits. I
think they should use `jj abandon` if they want to get rid of already
empty commits. By keeping already empty commits, we don't need to
special-case the working copy and merge commits.
This allows us to call alias function with the top-level object.
For convenience, all self.<method>()s are available as keywords. I don't think
we'll want to deprecate them. It would be tedious if we had to specify
-T'self.commit_id()' instead of -Tcommit_id.
If I remember correctly, wrap_fn() was added to help type inference. It no
longer makes sense because the type is coerced by TemplateFunction::new()
and language.wrap_*().
The description of `jj diff` was lost in commit b5e4e670. We later got
a short description for it in b5e4e670. This patch restores the
original description.
This eliminates the separate keywords table. All keywords are resolved through
the pseudo "self" property. Maybe we'll add "self" keyword/variable later.
This is copied from the commit templater. I'm going to extract the "self"
property handling to the core template builder, and build_keyword() methods
will be replaced with that.
The default immutable_heads() includes tags(), which makes sense, but computing
heads(tags()) can be expensive because the tags() set is usually sparse. For
example, "jj bench revset 'heads(tags())'" took 157ms in my linux stable
mirror. We can of course optimize the heads evaluation by using bit set or
segmented index, but the query includes many historical heads if the repository
has per-release branches, which are uninteresting anyway. So, this patch
replaces heads(immutable_heads()) with trunk().
The reason we include heads(immutable_heads()) is to mitigate the following
problem. Suppose trunk() is the branch to be based off, I think using trunk()
here is pretty good.
```
A B
*---*----* trunk() ⊆ immutable_heads()
\
* C
```
https://github.com/martinvonz/jj/pull/2247#discussion_r1335078879
In my linux stable mirror, this makes the default log revset evaluation super
fast. immutable_heads(), if configured properly, includes many historical
branch heads which are also the visible heads.
revsets/immutable_heads()..
---------------------------
0 12.27 117.1±0.77m
3 1.00 9.5±0.08m
It should be useful at least in the presentation layer to know which
operations correspond to working-copy snapshots. They might be
rendered differently in the graph, for example. Or maybe an undo
command wants to warn if you just undid a snapshot operation. This
patch just introduces a field in the metadata to store the
information.
I think I prefer this behavior because it's less lossy. The user can
manually simplify the history with `jj rebase -s <merge commit> -d
<one of the parents>` afterwards. We can roll this change back later
if we find it annoying.
We now have lots of tests of ancestor merges in `test_bug_2600()`, so
we don't need the ones in `test_basics()`. Since it doesn't have the
"nottherootcommit" commit, it would break when we change the default
to preserve ancestor merges.
I think the conclusion from #2600 is that at least auto-rebasing
should not simplify merge commits that merge a commit with its
ancestor. Let's start by adding an option for that in the library.
The shortest change id prefix will become a few digits longer, but I think
that's acceptable. Entries included in the "revsets.short-prefixes" set are
unaffected.
The reachable set is calculated eagerly, but this is still faster as we no
longer need to sort the reachable entries by change id. The lazy version will
save another ~100ms in mid-size repos.
"jj log" without working copy snapshot:
```
% hyperfine --sort command --warmup 3 --runs 20 -L bin jj-0,jj-1,jj-2 \
-s "target/release-with-debug/{bin} -R ~/mirrors/linux debug reindex" \
"target/release-with-debug/{bin} -R ~/mirrors/linux \
--ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=\"\"'"
Benchmark 1: target/release-with-debug/jj-0 -R ~/mirrors/linux --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=""'
Time (mean ± σ): 353.6 ms ± 11.9 ms [User: 266.7 ms, System: 87.0 ms]
Range (min … max): 329.0 ms … 365.6 ms 20 runs
Benchmark 2: target/release-with-debug/jj-1 -R ~/mirrors/linux --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=""'
Time (mean ± σ): 271.3 ms ± 9.9 ms [User: 183.8 ms, System: 87.7 ms]
Range (min … max): 250.5 ms … 282.7 ms 20 runs
Relative speed comparison
1.99 ± 0.16 target/release-with-debug/jj-0 -R ~/mirrors/linux --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=""'
1.53 ± 0.12 target/release-with-debug/jj-1 -R ~/mirrors/linux --ignore-working-copy log -r.. -l100 --config-toml='revsets.short-prefixes=""'
```
"jj status" with working copy snapshot (watchman enabled):
```
% hyperfine --sort command --warmup 3 --runs 20 -L bin jj-0,jj-1,jj-2 \
-s "target/release-with-debug/{bin} -R ~/mirrors/linux debug reindex" \
"target/release-with-debug/{bin} -R ~/mirrors/linux \
status --config-toml='revsets.short-prefixes=\"\"'"
Benchmark 1: target/release-with-debug/jj-0 -R ~/mirrors/linux status --config-toml='revsets.short-prefixes=""'
Time (mean ± σ): 396.6 ms ± 10.1 ms [User: 300.7 ms, System: 94.0 ms]
Range (min … max): 373.6 ms … 408.0 ms 20 runs
Benchmark 2: target/release-with-debug/jj-1 -R ~/mirrors/linux status --config-toml='revsets.short-prefixes=""'
Time (mean ± σ): 318.6 ms ± 12.6 ms [User: 219.1 ms, System: 94.1 ms]
Range (min … max): 294.2 ms … 333.0 ms 20 runs
Relative speed comparison
1.85 ± 0.14 target/release-with-debug/jj-0 -R ~/mirrors/linux status --config-toml='revsets.short-prefixes=""'
1.48 ± 0.12 target/release-with-debug/jj-1 -R ~/mirrors/linux status --config-toml='revsets.short-prefixes=""'
```
When adding a new workspace, I would expect that it inherits the
patterns from the workspace I ran the command in. We currently don't
do that. That's quite annoying when your repo has very many files
(like at Google).
`graphlog::Edge` is used somewhat inconsistently. I've replaced `Edge::Present`
with two distinct `Edge::Direct` and `Edge::Indirect` which simplifies the
construction of the enum.
The `ContentHash` documentation specifies that implementations for enums should
hash the ordinal number of the variant contained in the enum as a 32-bit
little-endian number and then hash the contents of the variant, if any.
The current implementations for `std::Option`, `MergedTreeId`, and
`RemoteRefState` are non-conformant since they hash the ordinal number as a u8
with platform specific endianness.
Fixes#3051
The legacy parsing rules are turned into compatibility errors. The x:y rule
is temporarily enabled when parsing string patterns. It's weird, but we can't
isolate the parsing function because a string pattern may be defined in an
alias.
This adds a config to render a synthetic node with a "(elided
revisions)" description for elided segments of the graph.
I didn't add any templating support for the elided nodes because I'm
not sure how we would want that to work. In particular, I don't know
what `commit_id` and most other keywords should return for elided
revisions.
Users who edit non-head commits usually expect `jj next/prev` to
continue to edit the next/previous commit, so let's make that the
default behavior. This should not confuse users who don't edit
non-head commits since they will simply not be in this state. My main
concern is that doing `jj next; jj prev` will now usually take you
back to the previous commit, but not if you started on the parent of a
head commit.
The main goal is to avoid having a symlink in our source tree. Currently, there
is no good way to work with the `jj` repo with `jj` on Windows. Currently `jj`
just crashes with symlinks. This is being worked on, see e.g. #2939, but it will
always depend on whether Developer Mode is enabled in Windows or whether
symlinks are materialized as text files with symlinks. Finally, MkDocs has
trouble following symlinks on Windows, so building docs wouldn't work there.
Another advantage is that, previously, we were lucky that MkDocs treats `insta`
header in `cli-reference@.md.snap` as a Markdown header and follows symlinks at
all. Now, we no longer depend on that.
Follows up 9702a425e5 "Allow negative numbers in the template grammar."
Since we've added parsing rules for operator expressions, it makes sense to
parse unary '-' as operator.
If the operation corresponding to a workspace is missing for some reason
(the specific situation in the test in this commit is that an operation
was abandoned and garbage-collected from another workspace), currently,
jj fails with a 255 error code. Teach jj a way to recover from this
situation.
When jj detects such a situation, it prints a message and stops
operation, similar to when a workspace is stale. The message tells the
user what command to run.
When that command is run, jj loads the repo at the @ operation (instead
of the operation of the workspace), creates a new commit on the @
commit with an empty tree, and then proceeds as usual - in particular,
including the auto-snapshotting of the working tree, which creates
another commit that obsoletes the newly created commit.
There are several design points I considered.
1) Whether the recovery should be automatic, or (as in this commit)
manual in that the user should be prompted to run a command. The user
might prefer to recover in another way (e.g. by simply deleting the
workspace) and this situation is (hopefully) rare enough that I think
it's better to prompt the user.
2) Which command the user should be prompted to run (and thus, which
command should be taught to perform the recovery). I chose "workspace
update-stale" because the circumstances are very similar to it: it's
symptom is that the regular jj operation is blocked somewhere at the
beginning, and "workspace update-stale" already does some special work
before the blockage (this commit adds more of such special work). But it
might be better for something more explicitly named, or even a sequence
of commands (e.g. "create a new operation that becomes @ that no
workspace points to", "low-level command that makes a workspace point to
the operation @") but I can see how this can be unnecessarily confusing
for the user.
3) How we recover. I can think of several ways:
a) Always create a commit, and allow the automatic snapshotting to
create another commit that obsoletes this commit.
b) Create a commit but somehow teach the automatic snapshotting to
replace the created commit in-place (so it has no predecessor, as viewed
in "obslog").
c) Do either a) or b), with the added improvement that if there is no
diff between the newly created commit and the former @, to behave as if
no new commit was created (@ remains as the former @).
I chose a) since it was the simplest and most easily reasoned about,
which I think is the best way to go when recovering from a rare
situation.
A subsequent commit will need to handle the return value of
check_stale_working_copy() in 3 different ways, so a boolean will soon
not be sufficient. In preparation for that, inline is_stale() into its
caller, converting it into a "match".
These operator symbols are different from the ones in the revset language. I
have no idea if we need bitwise operators, but we'll probably add comparison
operators. It would look weird if 'x == y & z' were parsed as '(x == y) & z'.
