This basically supersedes the current "branch set" command. The plan is to turn
"branch set" into an "upsert" command, and deprecate "branch create". (#3584)
Maybe we can also add "branch set --new" flag to only allow creation of new
branches. One reason behind this proposed change is that "set" usually allows
both "creation" and "update". However, we also need a typo-safe version of
"set" to not create new branches by accident.
"jj branch move" is useful when advancing ancestor branches. Let's say you've
added a couple of commits on top of an existing PR branch, you can advance the
branch by "jj branch move --from 'heads(::@- & branches())' --to @-". If this
pattern is super common, maybe we can add --advance flag for short.
One drawback of this change is that "git branch --move" is equivalent to
"jj branch rename". I personally don't find this is confusing, but it's true
that "move" sometimes means "rename".
This basically backs out 8706fadca1 "cli: inline check for
non-fast-forwardable branch move." I'm going to add another subcommand that
moves existing branches.
Otherwise they wouldn't be sorted in help. I also reordered the match statement.
Since subcommands are split to per-file modules, there's no point to keep some
logical ordering.
- make an internal set of watchman extensions until the client api gets
updates with triggers
- add a config option to enable using triggers in watchman
Co-authored-by: Waleed Khan <me@waleedkhan.name>
If I can't tell what this comment means, probably nobody else can
either.
I think it might be a copy-paste error from whatever docs I copied the
initial version of the config from.
Still alias function shadows builtin function (of any arity) by name. This
allows to detect argument error as such, but might be a bit inconvenient if
user wants to overload heads() for example. If needed, maybe we can add some
config/revset syntax to import builtin function to alias namespace.
The functions table is keyed by name, not by (name, arity) pair. That's mainly
because std collections require keys to be Borrow, and a pair of borrowed
values is incompatible with owned pair. Another reason is it makes easy to look
up overloads by name.
Alias overloading could also be achieved by adding default parameters, but that
will complicate the implementation a bit more, and can't prevent shadowing of
0-ary immutable_heads().
Closes#2966
I'm going to add arity-based alias overloading, and we'll need function
(name, arity) pair to identify it in alias expansion stack. The exact parameter
names aren't necessary, but they can be embedded in error messages.
Most of the value of `jj fix` over a shell script is in formatting commits
other than `@`. `@::` often doesn't contain those other commits, so `-s @` is a
bad default.
We could get the same effect from `-s 'mutable() & ::@'`, but `reachable()` is
a bit more explicit and simple to read.
We could also base this on excluding `trunk()`, but that just seems like an
indirection for `mutable()` that might ignore the user's intent if they have
configured part of trunk to be mutable.
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.
As we discovered in the `jj fix` tests,
`MergedTreeBuilder::write_tree()` doesn't try to resolve conflicts,
not even trivial ones. This patch fixes that.
The goal is to remove special case from parsing functions and provide slightly
better error message. I don't know if we'd want to use "all:" in aliases, but
there are no strong reasons to disable it.
This commit adds an optional flag to be able to push commits with an
empty description to a remote git repo. While the default behavior is
ideal we might need to interact with a repo that has an empty commit
description in it. I ran into this issue a few weeks ago pushing commits
from an open source repo to an empty repo and had to go back to using
git for that push as I would not want to rewrite the history which was
many many years long just for that.
This flag allows users an escape hatch for pushing empty descriptions
for commits and they're sure that they want that behavior.
This commit adds the flag to the `git push` command and updates the docs
for the command. It also updates the original test to make sure that the
flag works as intended to reject the commit when not set and to allow
the commit when the flag is set.
Closes#2633
This simplifies the interface of helper functions. While revset doesn't have
top-level string pattern or integer literal, these parsing helpers could be
used to parse array subscript or n-th parent operator if any.
Previously, it sounded like `jj git` might only include highly-technical
commands, while IMO the most important commands in here are `jj git
fetch` and `jj git push`.
This will allows us to parse "file(..)" arguments as fileset expression by
transforming AST for example. I'm not sure if that's good or bad, but we'll
probably want to embed fileset expressions without quoting.
parse_expression_rule() is split to the first str->ExpressionNode stage and
the second ExpressionNode->RevsetExpression stage. The latter is called
"resolve_*()" in fileset, but we have another "symbol" resolution stage in
revset. So I choose "lower_*()" instead.
Otherwise, newly created default branch would be re-imported as a new Git HEAD.
This could be addressed by cmd_git_init(), but the same situation can be
crafted by using "git checkout -b".
It's copied from test_git_colocated.rs, and switched to commit_id.short()
because full-length commit_id looked too verbose. "all()" history isn't needed,
but it's easier to follow.
Some backends, like the one we have at Google, can restrict access to
certain files. For such files, if they return a regular
`BackendError::ReadObject`, then that will terminate iteration in many
cases (e.g. when diffing or listing files). This patch adds a new
error variant for them to return instead, plus handling of such errors
in diff output and in the working copy.
In order to test the feature, I added a new commit backend that
returns the new `ReadAccessDenied` error when the caller tries to read
certain objects.
I'm going to add a similar message for access denied. That will want
an error message printed at the end. For consistency, let's do the
same for non-file paths.
Clear the rest of the cursor line (from the cursor to the end of the
row) after drawing the progress bar rather than clearing the entire line
before drawing. This reduces flickering on terminal emulators which are
able to redraw rapidly.
I recently needed to test something on top of a two branches at the
same time, so I created a new commit on top of both of them (i.e. a
merge commit). I then ran tests and made some adjustments to the
code. These adjustments belonged in one of the parent branches, so I
used `jj squash --into` to squash it in there. Unfortunately, that
meant that my working copy became a single-parent commit based on one
of the branches only. We already had #2859 for tracking this issue.
This patch changes the behavior so we create a new working-copy commit
with all of the previous parents.
This should be a no-op, though that is not necessarily obvious in corner
cases.
Note that libgit2 already performs the push negotiation even when
pushing without force (without `+` in the refspec).
As explained in the commit, our logic is a bit more complicated than
that of `git push --force-with-lease`. This is to match the behavior of
`jj git fetch` and branch conflict resolution rules.
This tests `git push` attempting to create a branch when the branch
already unexpectedly exists on the remote. This should (and does)
fail.
Also changes another test to use `jj_cmd_failure`.
We want to move UI-independent logic that's currently in `jj-cli` into
`jj-lib`. `WorkspaceCommandHelper` is perhaps the most important part
to start moving. As I was looking into what to move from
`WorkspaceCommandHelper`, the first thing I saw there was the
`cwd`. It might seem like a good candidate to start moving. However,
when running a server, you might be running operations on repos stored
in database, so `cwd` and the workspace root don't make sense then
(because the repo is not stored at a particular path).
So, instead, this patch starts abstracting out our uses of those two
paths by adding an enum for converting between `RepoPath` and paths as
they are presented in the UI. I added a variant for repos stored in a
file system, and made `WorkspaceCommandHelper` use that to show that
it works. We'll probably add a server variant later.
I put the new type in `repo_path.rs` because at least the
file-system-based implementation is closely related to
`RepoPath::parse_fs_path()`.
It's been about six months since we started using tree-level conflicts
by default. I can't imagine we would switch back. So let's continue
the migration by always using tree-level conflicts when merging trees,
even if all inputs were legacy trees.
The original expand_node() body is migrated as follows:
- Identifier -> fold_identifier()
- FunctionCall -> fold_function_call()
expand_defn() now manages states stack by itself, which simplifies lifetime
parameters.
The templater implementation of FoldableExpression is a stripped-down version
of expand_node(). It's visitor-like because I'm going to write generic alias
substitution rules over abstract expression types (template, revset, fileset.)
Naming comes from rustc.
https://rust-unofficial.github.io/patterns/patterns/creational/fold.html
This is basically the same as the previous patch, but for error types. Some
of these functions could be encoded as "E: From<AliasExpandError<'i>>", but
alias substitution logic is recursive, so it would have to convert E back and
force.
Perhaps, the original intent was to abandon non-empty working-copy commit
assuming it was rewritten by git (therefore it should be superseded by the
current working-copy content.) However, there are other reasons that could
make the HEAD out of sync (including concurrent jj operations), and abandoning
non-empty commit can be a disaster. This patch turns it to safer side, and let
user abandon non-empty commit manually.
Fixes#3747
I'm going to extract generic alias substitution functions, and these AST types
will be accessed there. Revset parsing will also be migrated to the generic
functions.
This will help extract common FunctionCallNode<'i, T> type. We don't need
freedom of arbitrary error type choices, but implementing From<_> is the
easiest option I can think of. Another option is to constrain error type by
the expression type T through "T::ParseError: ArgumentsParseError" or
something, but it seemed a bit weird that we have to use trait just for that.
"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.
I add them as aliases, since a user may instead choose to define `immutable_heads()`, for example, as `heads(immutable())`, and the define `immutable()` instead.
This revset correctly implements "reachability" from a set of source commits following both parent and child edges as far as they can go within a domain set. This type of 'bfs' query is currently impossible to express with existing revset functions.
This follows up on https://github.com/martinvonz/jj/pull/3459 and adds a
label to the closing delimeter of each conflict, e.g. "Conflict 1 of 3
ends".
I didn't initially put any label at the ending delimeter since the
starting delimeter is already marked with "Conflict 1 of 3". However,
I'm now realizing that when I resolve conflicts, I usually go from top
to bottom. The first thing I do is delete the starting conflict
delimeter. It is when I get to the *end* of the conflict that I wonder
whether there are any more conflicts left in the file.
I'll probably rewrite expand_aliases() in visitor-like interface, and tree
traversal logic will be implemented on ExpressionKind. That's why I made
expand_node() destructure ExpressionNode first.
This consolidates the type of substitution results. Before, symbol substitution
can return inner ExpressionKind internally, but function-parameter substitution
couldn't.
This should avoid regression caused by upcoming changes. An alias function
parameter will be wrapped with AliasExpanded, and type errors in it should
be reported with its alias expansion stack.
I'm trying to extract generic alias substitution functions, and some of them
will take ExpressionKind or Box<FunctionCallNode> by value, then return it or
substituted value of the same type. The cost of moving values wouldn't matter
in practice, but I think it's better to keep the value types small.
Now ExpressionKind is 4-word long.
previously, aliases to built-in commands were silently ignored. this matches git's behavior, but seems unhelpful, especially if the user doesn't know that a command with that name already exists.
give a warning rather than silently ignoring it.
I've added a wrapper struct in order to get around too many arguments warning.
It captures &dyn Repo as CommitTemplateLanguage would do. OTOH, &Ui is passed
by argument because the caller might need &mut Ui after the renderer object was
configured.
I've added a struct similar to RevsetWorkspaceContext. It can be a closure,
but we'll need to duplicate format_file_path() function anyway if we add
commit.diff() template.
The function now returns an iterator over `Result`s, matching
`Operation::parents()`.
I updated callers to also propagate the error where it was trivial.
When using `ui.color = "debug"`, changes in the output style
additionally include delimiters << and >>, as well as all active labels
at this point separated by ::. The output is otherwise unformatted and
the delimiters and labels inherit the style of the content they apply
to.
Not all "branch list --all-remotes" callers are replaced because I'm going to
make get_branch_output() suppress hints by default, and there should be tests
for the hints.
This also means "all:" is allowed in default revsets (such as "revsets.log"),
but that seems okay. In revset aliases, "all:" isn't allowed because aliases
may be expanded to sub-expression position.
Closes#3654
Perhaps, this can be used to generate parsable branches list.
The hint for deleted branches isn't migrated to the template. I'm thinking of
moving it out of the loop and printed once at the end. If we want to generate
a hint in template, we'll probably need local_ref.tracking_remote_refs(), etc.
that return a list of RefNames.
Perhaps, this make it a bit clearer when "deleted" local branches should be
displayed. In practice, this change is noop since remote_ref.target should
never be absent.
More tests will be added later as "branch list" templates.
In "log" template, we might want to see the number of "local" commits ahead
of any tracked remotes. It can be implemented later in a similar way (or as a
nested remote_refs list.)
Because template is declarative language, and is evaluated as a tree, there
will be multiple copies of the same RefName object. This patch allows us to
cache ahead/behind counts which will be lazily calculated.
We'll probably add binary comparison operators at some point, but this patch
also adds size_hint.zero() method. Otherwise, we'll have to write
"if(x.upper() && x.upper() == 0, ..)" to deal with None.
The resulting "branch list" template will look like:
```
separate(", ",
if(!ref.tracking_ahead_count().zero(),
if(ref.tracking_ahead_count().exact(),
"ahead by " ++ ref.tracking_ahead_count().exact() ++ " commits",
"ahead by at least " ++ ref.tracking_ahead_count().lower() ++ " commits")),
if(!ref.tracking_behind_count().zero(),
if(ref.tracking_behind_count().exact(),
"behind by " ++ ref.tracking_behind_count().exact() ++ " commits",
"behind by at least " ++ ref.tracking_behind_count().lower() ++ " commits")),
)
```
In order to port "branch list" to template, we need to somehow represent
revset.count_estimate() result as a template property. I'm going to add
SizeHint template type for that, and its .upper() and .exact() methods will
have to return optional integers.
Fortunately, the Integer type has no implicit conversion to bool, so
"if(optional_integer, ..)" is not ambiguous.
This is a remainder of the previous refactoring series. into_template() could be
implemented as a non-extension method, which allows us to get rid of .clone()
from Literal property extraction. However, there wasn't measurable difference.
Let's not try to overly optimize things. It's probably simpler to switch to
Rc<str> if .clone() really matters.
This is instead of https://github.com/martinvonz/jj/pull/3292, which would make
`diffedit3` built into `jj`. I still have some hope of eventually making
`diffedit3` into the default diff editor that is available without any
configuration, which probably requires building it into `jj`, but this may not
happen, and it wouldn't hurt to test `diffedit3` first. Some examples of
concerns (see also the discussion in that PR):
- It is only a guess on my part that this would make a good default. The editor
might not be polished enough, and most users are not used to 3-pane diff
editing. I think most users would like it if they tried it, but this might be
plain wrong.
- There are concerns about adding a heavyweight dependency on `jj`. While I
tried to make it as lightweight as possible, it still unavoidably includes a web
server.
- There may be ways to bundle `diffedit3` with `jj` without combining them in a
single binary.
For example,
```
<<<<<<< Conflict 1 of 3
+++++++ Contents of side #1
left 3.1
left 3.2
left 3.3
%%%%%%% Changes from base to side #2
-line 3
+right 3.1
>>>>>>>
```
or
```
<<<<<<< Conflict 1 of 1
%%%%%%% Changes from base to side #1
-line 3
+right 3.1
+++++++ Contents of side #2
left 3.1
left 3.2
left 3.3
>>>>>>>
```
Currently, there is no way to disable these, this is TODO for a future
PR. Other TODOs for future PRs: make these labels configurable. After
that, we could support a `diff3/git`-like conflict format as well, in
principle.
Counting conflicts helps with knowing whether you fixed all the
conflicts while you are in the editor.
While labeling "side #1", etc, does not tell you the commit id or
description as requested in #1176, I still think it's an improvement.
Most importantly, I hope this will make `jj`'s conflict format less
scary-looking for new users.
I've used this for a bit, and I like it. Without the labels, I would see
that the two conflicts have a different order of conflict markers, but I
wouldn't be able to remember what that means. For longer diffs, it can
be tricky for me to quickly tell that it's a diff as opposed to one of
the sides. This also creates some hope of being able to navigate a
conflict with more than 2 sides.
Another not-so-secret goal for this is explained in
https://github.com/martinvonz/jj/pull/3109#issuecomment-2014140627. The
idea is a little weird, but I *think* it could be helpful, and I'd like
to experiment with it.
I've heard of one instance of a person being confused by the error.
Previously, the error was:
```
Error: Failed to load tool configuration
Caused by: To use `diffedit3` as a merge tool, the config `merge-tools.diffedit3.merge-args` must be defined (see docs for details)
```
Now, it is:
```
Error: The tool `diffedit3` cannot be used as a merge tool with `jj resolve`.
Hint: To use `diffedit3` as a merge tool, the config `merge-tools.diffedit3.merge-args` must be defined (see docs for details)
```
TODO for future PR: allow setting `merge-tools.TOOL.edit-args = false` so that
attempting to use TOOL as a diff editor fails. This would be helpful, for
example, for the `vscode` tool.
I considered adding RefTarget template type, but some of the methods naturally
fit to RefName. For example, a conflicted branch name is decorated as "??", so
it makes sense to add branch.conflict() instead of branch.target().conflict().
I'm not pretty sure how many RefName methods we'll need to add to port the
current branch listing, but there will be .tracked(), .tracking_local_present(),
.ahead_by(), and .behind_by().
I'm going to add more detailed output there. This is a step towards "branch
list" template. "tag list -T" wouldn't be that useful, but it shares primitives
with "branch list -T".
I'm going to add ref_name.target*() template methods so the commit templater
can be reused for branches/tags templates. RefTarget could be looked up by
(name, kind) pair, but it's simpler to store it in RefName.
Before this patch, we would abandon the source commit if it became
empty after applying the reverse diff. This changes that condition to
the equivalent condition of the selected tree being the source
commit's original tree. This will help us rewrite the code to use
`transform_descendants()`.
The `move_commits` function accepts a set of target commits to shift to
a new location given by `new_parents` and `new_children`. The roots of
the target set will be reparented onto `new_parents`. `new_children`
will then be reparented onto the heads of the target set.
The commits will be rebased in reverse topological order based on the
new set of parents of each commit, which avoids the need for multiple
sets of rebase operations.
Spotted while experimenting with "jj tag list -T". The description_placeholder
alias could be changed to function taking a Commit object, but I feel it's odd.
Conceptually, the placeholder could also be used in "op log" templates.
Since fileset/revset/template expressions are specified as command-line
arguments, it's sometimes convenient to use single quotes instead of double
quotes. Various scripting languages parse single-quoted strings in various ways,
but I choose the TOML rule because it's simple and practically useful. TOML is
our config language, so copying the TOML syntax would be less surprising than
borrowing it from another language.
https://github.com/toml-lang/toml/issues/188
While I like strict parsing, it's not uncommon that we have to deal with file
names containing spaces, and doubly-quoted strings such as '"Foo Bar"' look
ugly. So, this patch adds an exception that accepts top-level bare strings.
This parsing rule is specific to command arguments, and won't be enabled when
loading fileset aliases.
When you use e.g. `git switch` to check out a conflicted commit,
you're going to end up with the `.jjconflicts-*` directories in your
working copy. It's probably not obvious what those mean. This patch
adds a README file to the root tree to try to explain to users what's
going on and how to recover.
The authoritative information about conflicts is stored in the
`jj:trees` commit header. The contents of conflicted commits is only
used for preventing GC. We can therefore add contents to the tree
without much consequence.
I will be updating `rebase -r` to avoid simplifying ancestor merges in a
subsequent commit, which will cause existing tests to fail for the Git
backend due to ancestor merges with the root commit.
I'm not sure if this was an intentional omission, but I think it would be
useful to have `-e` as a short flag for `--edit`. I don't usually edit commits,
but I do use `prev` and `next` with edit to navigate to a commit that I want to
squash. Often this is easier than typing `--from` and `--into` plus the change
IDs.
If people want to edit commits we shouldn't stand in their way.
This is following on the rewrite for `parallelize`.
- https://github.com/martinvonz/jj/pull/3521
Since rebase_descendants from rebase.rs is no longer used outside of that file,
it can be made private again.
In a future commit these tests will run with both `jj commit` and `jj new` since
both will have the same semantics for advancing the branch pointer.
Parameterizing the tests allows us to run both variants without duplicating the
test bodies. Since the commit IDs are different when `jj describe` + `jj new`
is used instead of `jj commit`, this commit also drops the commit IDs from the
test snapshots. This is fine because the commit IDs are not important for these
tests.
## Feature Description
If enabled in the user or repository settings, the local branches pointing to the
parents of the revision targeted by `jj commit` will be advanced to the newly
created commit. Support for `jj new` will be added in a future change.
This behavior can be enabled by default for all branches by setting
the following in the config.toml:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
```
Specific branches can also be disabled:
```
[experimental-advance-branches]
enabled-branches = ["glob:*"]
disabled-branches = ["main"]
```
Branches that match a disabled pattern will not be advanced, even if they also
match an enabled pattern.
This implements feature request #2338.
Maybe it's personal preference, but the hash sign looks bigger compared to
the normal "o" nodes, and is slanted. This makes immutable commits stand out
too much. I think "+" is closer to the diamond character used in the unicode
template.
Since we have two separate "immutable" calls in the builtin node template, and
user might add a few more to their text template, it seems reasonable to cache
the containing_fn globally.
It's reasonable for a `WorkingCopy` implementation to want to return
an error. `LocalWorkingCopyFactory` doesn't because it loads all data
lazily. The VFS-based one at Google wants to be able to return an
error, however.
This one just tests with a larger value and a human-readable string (10KB).
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: If9e5d62146b369d3a1b7efe4e56a1b6b4338c720
For new users this results in a significantly better error output, that
actually shows them how to solve the problem, and why it happened.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Ide0c86fdfb40d66f970ceaef7b60a71392d2cd4b
Previously, this command would work:
jj --config-toml='snapshot.max-new-file-size="1"' st
And is equivalent to this:
jj --config-toml='snapshot.max-new-file-size="1B"' st
But this would not work, despite looking like it should:
jj --config-toml='snapshot.max-new-file-size=1' st
This is extremely confusing for users.
This config value is deserialized via serde; and while the `HumanByteSize`
struct allegedly implemented Serde's `visit_u64` method, it was not called by
the deserialize visitor. Strangely, adding an `visit_i64` method *did* work, but
then requires handling of overflow, etc. This is likely because TOML integers
are naturally specified in `i64`.
Instead, just don't bother with any of that; implement a `TryFrom<String>`
instance for `HumanByteSize` that uses `u64::from_str` to try parsing the string
immediately; *then* fall back to `parse_human_byte_size` if that doesn't work.
This not only fixes the behavior but, IMO, is much simpler to reason about; we
get our `Deserialize` instance for free from the `TryFrom` instance.
Finally, this adjusts the test for `max-new-file-size` to now use a raw integer
literal, to ensure it doesn't regress. (There are already in-crate tests for
parsing the human readable strings.)
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I8dafa2358d039ad1c07e9a512c1d10fed5845738
This replaces `.map(|c| c.id().clone())` with `.ids().cloned()` to use nicer
syntax for getting `CommitId`s from an iterator of commits using the
`CommitIteratorExt` trait.
In one case we can actually call `.parent_ids()` directly. I also pluralized a
variable to make it clearer that it's a vec of IDs and not a single ID.
The rewritten code is already a no-op when there's a single input. I
don't think the case is common enough to warrant having a special case
for performance reasons either. Also, by not having the special case,
`jj parallelize <immutable commit>` fails consistently with the
non-singleton case.
I'm going to make some `jj parallelize` cases that currently error out
instead be successful. Some of the will result in ancestor merges with
the root commit. This patch updates those tests to avoid that.
`jj parallelize` was a good example of a command that can be
simplified by the new API, so I decided to rewrite it as an example.
The rewritten version is more flexible and doesn't actually need the
restrictions from the old version (such as checking that the commits
are connected). I still left the check for now to keep this patch
somewhat small. A subsequent commit will remove the restrictions.
`CommitRewriter` wraps 3 of the arguments, so I think it makes sense
to pass it instead. More importantly, I hope to continue refactoring
so many of the callers already have a `CommitRewriter`.
It was removed at 522025e091 "log: remove unused and inconsistent `log`
label", but obslog had the same inconsistency. Since it's now easy to label
the template output, let's re-add the "log" label.
The change in test_templater_upper_lower() is noop. Formatter no longer
emits reset sequence in the middle because the template is still labeled.
These labels could be renamed to "log_node"/"op_log_node" for consistency, but
I'm not sure if that's a good idea. A single "node" namespace is practically
more convenient.
It's not uncommon to label the whole template output with command or template
name. If the output doesn't have to be captured, we can simply push the label
to the formatter. cmd_config_list() is an example of such cases, but it's also
migrated for consistency.
Mercurial appears to resolve cwd-relative path first, so "glob:*.c" could be
parsed as "**/*.c" if cwd was literally "**". It wouldn't practically matter,
but isn't correct. Instead, jj's parser first splits glob into literal part
and pattern. That's mainly because we want to parse the user input texts into
type-safe objects, and (RepoPathBuf, glob::Pattern) pairs are the simplest
ones. The current parser can't handle patterns like "foo/*/.." (= "foo" ?),
and errors out. I believe this restriction is acceptable.
Unlike literal paths, the 'glob:' pattern anchors to the whole file path. I
don't think "prefix"-matching glob is useful, and making it the default would
be rather confusing.
It's cheap to look up commits again from the cache in `Store` but it
can be expensive to look up commits we didn't end up needing. This
will make it easier to refactor further and be able to cheaply set
preliminary parents for a rewritten commits and then let the caller
update them.
I'm going to add a helper struct to help with rewriting commits. I
want to make that struct own the old commit and the new parents to
simplify lifetimes. This patch prepares for that by passing the
commits by value to `rebase_commit()`.
Running `cargo publish` from a non-colocated repo (such as my usual
repo) is currently quite scary because it uploads all non-hidden
files, even if they're ignored by `.gitignore`
(https://github.com/rust-lang/cargo/issues/2063). I noticed this a
while ago and have always run the command from a fresh clone since
then. To avoid the need for that, let's use the workaround mentioned
on the bug, which is to explicitly list patterns we want to publish.
This fixes several issues that made working with empty files difficult using
the builtin diff editor.
1. The scm-record library used for the editor expects each file to have at
least one section. For new empty files this should be a file mode section. jj
wasn't rendering this mode section, which prevented empty files from being
selected at all.
2. The scm-record library returns `SelectedContents::Absent` if the file has no
contents after the diff editor is invoked. This can be because of several
reasons: 1) the file is new and empty; 2) the file was empty before and is
still empty; 3) the file has been deleted. Perhaps this is a bug in scm-record
and it should return `SelectedContents::Unchanged` or
`SelectedContents::Present` if the file hasn't been deleted. Until this is
patched upstream, we can work around it by disambiguating these cases.
See https://github.com/arxanas/scm-record/issues/26 for the upstream bug.
Fixes#3016
This is the last non-debug command that doesn't support file patterns. It
wouldn't make much sense to "cat" multiple files (despite the command name),
but doing that should be harmless.
Prepares for migrating to the matcher API. "Path exists but is not a file"
error is turned into a warning because the loop shouldn't terminate there.
"No such path" error message is also updated for consistency.
if `--use-destination-message/-u` is passed to `jj squash`, the resulting
revision will use the description of the destination revision and the
description(s) of the source revision(s) will be discarded.
If we ever implement some sort of ABI for dynamic extension loading, we'll need these underlying APIs to support multiple extensions, so we might as well do that first.
If this doesn't work out, maybe we can try one of these:
a. fall back to bare file name if expression doesn't contain any operator-like
characters (e.g. "f(x" is an error, but "f x" can be parsed as bare string)
b. introduce command-line flag to opt in (e.g. -e FILESET)
c. introduce pattern prefix to opt in (e.g. set:FILESET)
Closes#3239, #2915, #2286
This is the same as the `test_split_siblings_with_merge_child` added in
https://github.com/martinvonz/jj/pull/3485, but without the siblings flag. I
forgot to add the non-siblings version in that PR.
#3485
This command checks not only whether Watchman works, but also whether
it's enabled in the config. Also, the output is easier to understand
than that of the other `jj debug watchman` commands.
It would be nice if `jj debug watchman` called `jj debug watchman
status`, but it's not trivial in `clap` to have a default subcommand.
Ilya reported this in https://github.com/martinvonz/jj/issues/3483.
The bug was introduced in 976320726d.
Before this fix, `jj split` dropped any parents what weren't involved in the
split when it rebased the children of the commit being split. This meant that
any children which were merge commits lost their other parents unintentionally.
Fixes#3483
Maybe we can optimize it to check paths during diffing, but I think it's okay
to add extra lookup cost at the end. The size of the path arguments is usually
small.
Closes#505
Path parsing will be migrated to parse_union_filesets(), but I haven't decided
how we'll go forward:
a. migrate everything to fileset
b. require flag like "-e FILESET" (note -p conflicts with log -p)
c. require flag like "-e FILESET" and deprecate positional PATHs #2554
d. require prefix like "set:FILESET" (not consistent with -r REVSET)
I'm currently dogfooding (a). It works for me, but I don't use exotic file
names that would require quoting in zsh.
#3239
Before this commit `jj prev` fails if the current working copy commit is a
merge commit. After this commit it will prompt the user to choose the ancestor
they want to select.
#2126
This commit adds commit graphs to most of the tests for `jj prev` to make it
clearer where `@` points before and after `prev` is run.
In addition, there were a couple of tests where the comments suggested the test
meant to have `@` pointing to a specific commit, but it actually pointed to an
empty child of that commit.
This sort of issue also exists in `test_prev_editing`. The test is supposed to
check that `--edit` is implied if you run `jj prev` on an interior commit, but
it actually caused a new empty commit to be created since `@` was sitting on a
tip commit.
Expose the information we now record, to allow changing the default "snapshot
working copy" message or to make snapshots more compact/less distracting in
the log
This will hopefully make it clear that `jj prev` does not
move by [OFFSET] relative to `@`, which is a misconception
that I had and I think others may also have.
I am suggesting this change as a result of the vigorous discussion in
these two issues:
- https://github.com/martinvonz/jj/issues/3426
- https://github.com/martinvonz/jj/pull/3445
We should make similar changes to `jj next` as well since
it follows similar rules.
This patch adds "string_" prefix to the related rules to discriminate them from
integer_literal. I also renamed "raw_literal" because it sounds like a raw
string literal that preserves backslash characters.
This is basically the same as string kind:pattern syntax in CLI. This will
hopefully be superseded by filesets, but I'm not sure if that will work out.
A file name is more likely to contain whitespaces, which will have to be
quoted as '"Documents and Settings"'.
There are no more callers of parse_function_argument_to_string(), so it's
removed. This function was a thin wrapper of literal parser, and can be
easily reintroduced if needed.
Parallelize revisions by making them siblings
Running `jj parallelize 1::2` will transform the history like this:
```text
3
| 3
2 / \
| -> 1 2
1 \ /
| 0
0
```
Each of the target revisions is rebased onto the parents of the root(s) of
the target revset (not to be confused with the repo root). The children of
the head(s) of the target revset are rebased onto the target revisions.
The target revset is the union of the REVISIONS arguments.
The target revset being parallelized must satisfy several conditions,
otherwise the command will fail.
1. The heads of the target revset must not have different children.
2. The roots of the target revset must not have different parents.
3. The parents of all target revisions except the roots must also be
parallelized. This means that the target revisions must be connected.
The nightly compiler has several clippy fix-its that, if applied, break the
build. There are various bugs about this, but there isn't enough space in the
margins to detail it all.
Just ignore these on a per-function basis; about 70% of them are just multiple
instances happening inside a single function.
This makes `cargo clippy --workspace --all-targets` run clean, even with the
nightly compiler.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Ic26a025d3c62b12fbf096171308b56e38f7d1bb9
This lets users use "large" revsets in commands such as `jj rebase`, without
needing the `all:` modifier.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Ica80927324f3d634413d3cc79fbc73057ccefd8a
After upgrading pest from 2.7.8 to 2.7.9, I noticed CLI tests got significantly
slow (something around 40sec -> 60sec on my laptop.) I suspect this would be
caused by detailed error state tracking introduced in 2.7.9, but it's also true
that our template grammar exercises such code path.
My understanding is that PEG is basically a top down parsing with unlimited
lookahead. Before this change, the default commit_summary template would be
parsed as follows:
1. parse the outermost separate(..) as "term"
2. "concat" rule can't continue, so
3. reparse the whole string as "expression"
Because this pattern is not uncommon, I think it's better to rewrite the
grammar to avoid large retry.
With this patch, our tests runs within ~50sec under debug build. It appears to
save a few milliseconds in release build, but my development environment isn't
quiet enough to say the difference is significant.
Offset is a more descriptive noun for this argument. This commit also tweaks
the help message for the argument.
This isn't an option/flag, so this change should be transparent to users.
This function doesn't actually need commits, it only needs their IDs. In some
contexts we may only have commit IDs, so there's no need to require an iterator
of Commits.
This commit also adds a `CommitIteratorExt` that makes it easy to convert an
iterator of `&Commit` to an iterator of `&CommitId`.
I'm going to make all WorkspaceCommandHelper::parse/resolve_revset functions
accept &RevisionArg, so I want a convenient way to unwrap Option<RevisionArg>.
Another option is to add an associated function that returns
RwvisionArg("@".to_owned()). As we wouldn't care for the allocation cost, either
approach should work fine.
I keep typing `--to` since I'm used to `jj move` interface. It is
also shorter.
Currently, if I type `--to`, clap unhelpfully suggests whether I
meant `--tool`.
Like -r/--revisions, it should be okay to filter synced/non-tracking remote
branches by name.
conflicts_with_all = "tracked" is redundant, so removed as well. The tracked
field declares that it conflicts with --all-remotes.
Since "all:" implies that the user doesn't care about the order of the
revisions within that argument, it should be okay if two "all:" sets overlapped.
resolve_revset_default_single() will be inlined instead. Since "default_single"
evaluation can return multiple revisions, the CLI interface usually accepts
multiple arguments. This suggests that there would be no external callers of
the singular resolve_revset_default_single().
I'm going to reorganize "single"/"default_single" revset functions in a way
that resolve_single_rev_with_hint_about_all_prefix() is inlined.
evaluate_revset_to_single_commit() could be a private method of
WorkspaceCommandHelper, but I want to minimize the code that has to be hosted
there.
When a commit is split, any branches pointing to it are moved to the second
commit created by the split. This is true even if the --siblings option is
used.
#3419
If the --siblings option is used, the target commit is split into two sibling
commits instead of parent and child commits. Any children of the original
commit will have both siblings as their new parents.
#2274
This refactor will allow us to reuse new `rebase_descendants` function for the
`jj split --siblings` feature (#2274) and later possibly for `jj parallelize`
(#1079).
Note that `jj resolve` already had its own `--quiet` flag. The output
with `--quiet` for that command got a lot quieter with the global
`--quiet` also taking effect. That seems reasonable to me.
When the caller needs a formatter, it's because they're doing
something non-trivial. When the user passed `--quiet` (see upcoming
patch), we should ideally skip doing related work for print the
formatting output. It helps if the `Ui` object doesn't even return a
`Formatter` then, so the caller is forced to handle the quiet case
differently.
Thanks to Yuya for the suggestion.
I'm about to make hints not get printed with `--quiet`, but error
hints are probably still useful to get. They shouldn't be a problem
for scripts since the script would have to deal with the error anyway.
evaluate_programmatic() should be allowed here. AFAIK, the contract is that
the expression should never contain any bare symbols and "fallible" symbol-like
expressions, which doesn't apply to branches() and remote_branches() functions.
evaluate_revset() is removed because there are no callers. However, it's simple
and basic function, so we might want to reintroduce it if needed.
Many callers of resolve_revset() and evaluate_revset() will be migrated to
this wrapper. "single" and "default_single" APIs won't be replaced because
they require more contexts to construct error messages.
id_prefix_context() now uses bare revset::parse() to avoid dependency cycle.
Templater doesn't have the one yet, but I think it belongs to the same
category.
For clap::Error, we could use clap's own mechanism to render suggestions as
"tip: ...", but I feel "Hint: ..." looks better because our error/hint message
is capitalized.
I'm going to add RevsetParseError constructor for InvalidFunctionArguments,
with/without a source error, and I don't want to duplicate code for all
combinations. The templater change is just for consistency.
I couldn't find a good naming convention for the builder-like API, so it's
called .with_source(mut self, _). Another option was .source_set(source).
Apparently, it's not uncommon to name consuming constructor as
with_<something>().
If "all:" is specified, an empty set should be allowed within that expression.
So the additional check we need here is to ensure that the resulting set is not
empty.
One less CLI revset helper. It might look odd that "jj rebase" says "Merge
failed" whereas "jj new" doesn't, but that depends on where the BackendError
is detected.
This commit moves the parse_string_pattern helper function into the
str_util module in jj lib and adds tests for it.
I'd like to reuse this code in a function defined by `UserSettings`, which is
part of the jj lib crate and cannot use functions from the cli crate.
The idea is that, if .extract() succeeded in static context, it means the
property can be evaluated as constant. This will potentially eliminate
expect_string_literal_with(), though I'm not too sure if it's a good idea.
If needed, maybe we can extend the idea to suppress type/name resolution errors
by "if(some_static_config_knob, x, y)".
This allows us to propagate property evaluation error to a string property. For
instance, "s.contains(x ++ y)" will be an error if "y" failed to evaluate,
whereas bare "x ++ y" shouldn't.
The other implementation ideas:
a. add Template::into_string_property() to enable strict evaluation
=> it's tedious to implement it for each printable type
b. pass (formatter, error_handler) arguments separately
=> works, but most implementors don't need error_handler argument
c. pass strict=bool flag around build_*() functions
=> didn't tried, but it would be more complicated than this patch
Because Template trait is now implementation detail of the templater, it
should be okay to use a non-standard formatter wrapper.
Commands like `new`, `duplicate`, and `abandon` can take multiple revset
arguments which results in their collective union. They take the revisions
directly as arguments. But for consistency with many other commands, they can
also take the `-r` argument, which is a no-op. However, due to the flag being
specified as a `bool`, the `-r` option can only be specified once, so e.g.
`abandon -r x -r y` often fails. I normally use `-r` for consistency and muscle
memory, so this bites me often.
Instead, use `clap::ArgAction::Count` in order to allow `-r` to be specified
multiple times. It remains unused, of course.
With this change, all the following invocations are equivalent. Before this
change, the second example would fail due to giving `-r` multiple times.
jj abandon x y
jj abandon -r x -r y
jj abandon -r 'x | y'
Note: `jj new` already supported this exact case actually, but it used an
awkward trick where it used `.overrides_with()` in order to override *itself* so
it could be specified multiple times. I believe this is a bit clearer.
Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: Ib36cf81d46dae4f698f06d0a32e8fd3120bfb4a4
This makes the summary line more informative. Even though it just duplicates
the message printed later, I think it's easier to follow.
This patch also adjusts some RevsetParseError messages because it seemed
redundant to repeat "revset function", "argument", etc.
Because the CLI error handler now prints error sources in multi-line format,
it doesn't make much sense to render Revset/TemplateParseError differently.
This patch also fixes the source() of the SyntaxError kind. It should be
self.pest_error.source() (= None), not self.pest_error.
I'm going to make TemplateParseError hold RevsetParseError as Box<dyn _>, but
Box<dyn std::error::Error ..> doesn't implement Eq. I could remove Eq from
ErrorKind enums, but it's handly if these enums remain as value types.
This change will also simplify fmt::Display and error::Error impls.
This can be used to flatten nested "if()"s. It's not exactly the same as "case"
or "switch" expression, but works reasonably well in template. It's not uncommon
to show placeholder text in place of an empty content, and a nullish value
(e.g. empty string, list, option) is usually rendered as an empty text.
A formatted error is not a string containing ANSI escape sequences because 1.
the output may be differently colored inside "hint", 2. the caller might not
be accessible to ui.new_formatter().
Highlighting "{n}: " will help to follow error sources containing multi-line
messages. I'm going to make revset/template alias errors be formatted as plain
error chain.
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.
The lowercase "warning: " is unified to "Warning: " as it is the jj's
convention afaik.
The _default() suffix could be dropped from these methods, but it's probably
better to break the existing codebase for the moment. Otherwise, the caller
might do writeln!(ui.warning(), "Warning: ..").
The existing .hint() method is renamed to .hint_no_heading() to clarify that
it's not the default choice to print a hint. I'll add .hint_default() later,
which will be the shorthand for .hint_with_heading("Hint: ").
This will be used to label "Error: " heading and content differently. I want
to see an error message in the default (white) color because it's easier to
read, but I still want to highlight the "Error: " heading.
We can achieve that without introducing new wrapper, but the resulting code
would look something like "writeln!(ui.error("Error: ")?, ..)?", and it would
get messier if the caller had to suppress io::Error.
I don't think we have any callers left that call
`record_rewritten_commit()` multiple times within a transaction and
expect it to result in divergence. I think we should consider it a bug
to do that.
I'm going to introduce two changes: 1. indent commit summary, 2. colorize
output. The former can be implemented without using the templater API, but the
latter can't.
IntoTemplate will be cleaned up later. Perhaps, the lifetime parameter can be
removed at this point, but I'm planning to remove the IntoTemplate trait at all.
The type parameter 'C' will be removed from the Template trait, making it
represent a printable type or compiled template.
TemplateRenderer now holds Box<dyn _> template because it's unlikely that the
inner template type can be statically determined.
This fixes --change/--branch conflicts by making --change precede --branch. I
don't think this is the most obvious behavior, but it's the easiest workaround.
It could be moved before set_local_branch_target() to not update the local
branch, but it seemed weird that --change is silently ignored. This
inconsistency will be addressed later.
This helps to implement CommandError::add_hint(). The inner errors could be
embedded in the enum as before, but they're mostly of the same type. And I think
it's okay to use downcast_ref() to deal with the clap::Error special case.
I'm going to reorganize CommandError as (kind, err, hints) tuple so that we
can add_hint() to the constructed error object.
Some config error messages are slightly adjusted because the inner error is
now printed in separate line.
I think Option<Commit> is the simplest encoding of the log node.
The behavior of an Option type is closer to nullable types rather than the
Option in Rust. I don't think we would want to write opt.map(|x| x.f()) or
opt.unwrap().f(). We can of course add opt?.f() syntax, but it will be a short
for "if(opt, opt.f())"?
As requested in #1471, I added a new flag for `jj branch list` to only show branches that are conflicted.
Adds a unit test to check for listing only conflicted branches and regenerates the cli output to incorporate the new flag.
Closes#1471
reformat
These two are common when implementing methods. Fortunately, the
"return-position impl Trait in traits" feature is stabilized in Rust 1.75.0,
so we don't need another variant of TemplateFunction.
Apart from (IMO) looking nicer, this will also sidestep the potential problem
that if the file contains actual jj conflict markers (`>>>>>>>` in the beginning
of a line, for example), jj would currently have trouble materializing and
subsequently parsing conflicts in the file if it actually became conflicted.
I'll demo this bug in either this or a subsequent PR. It's the kind of bug that
sounds serious in theory but might never cause a problem in practice.
After this PR, only `docs/tutorial.md` has a conflict marker that's not indented.
There's only one there, so hopefully it won't be too much of a pain to deal with.
I also indented other strings in `test_conflicts.rs`. IMO, this looks nice and
more consistent with the `insta::assert_snapshot` output. I didn't spend the
time to do the same for `test_resolve_command`.
Now a compiled template doesn't have a static Context type internally. A
property is basically of "Fn() -> Result<O, _>" type, and a type-erased "self"
variable will be injected as needed.
Template<C> types will be refactored separately.
In short, this enables compilation of template of e.g. Vec<Commit> type by
using CommitTemplateLanguage.
A Template<C> was originally compiled for a specific type C, and invoked as
"Fn(&C) -> _". It was simple and intuitive, but we had to define the context
type C statically. Things got even worse by extensions support because we had
to provide object-safe hook point for each context type C.
This patch basically removes the Context type from compiled templates. The
"self" variable is injected through RefCell<Option<C>> placeholder. A compiled
template knows its "self" type, but the type can be decided per instance, not
per TemplateLanguage type. A drawback is that the "self" variable will have to
be cloned one more time.
The Template<C> abstraction no longer makes sense, and will be split to inner
Template<()> implementations (which usually represent printable types) and the
outer Template<C>.
The idea is basically the same as list.map(|x| ...) template. It compiles the
inner template with a placeholder variable of type 'C', and evaluate it for
each instance variable by injecting the variable through RefCell.
Because GenericTemplateLanguage doesn't support any global resources, it no
longer makes sense to pass the language instance around to 0-ary keyword
functions.
These .wrap_<type>() functions aren't supposed to capture resources from the
language instance. It was convenient that wrap_() could be called without fully
spelling the language type, but doing that would introduce lifetime issue in
later patches.
I added type alias L to several places because the language type is usually
called L in generic code.
As discussed in #2900, the milliseconds are rarely useful, and it can
be confusing with different timezones because it makes harder to
compare timestamps.
I added an environment variable to control the timestamp in a
cross-platform way. I didn't document because it exists only for tests
(like `JJ_RANDOMNESS_SEED`).
Closes#2900
Changes the formatter to accept not only existing color names (such as "red" or
"green") but also those in the form #rrggbb, where rr, gg, and bb are two-digit
hexadecimal numbers. This allows much finer control over colors used.
"-r REVISIONS" here specifies the search space of the branches to push, and
warned if no branches are found in that space. I don't think an empty set
should be an error, but a warning for consistency. The warning message will be
improved by the subsequent patches.
This command belongs to the same category as "duplicate".
We might want a plural version of resolve_revset(), but I'm not sure whether
it should return Vec<Commit> or Revset. Let's revisit it later when we get
more callers.
Suppose we have an alias 'immutable()' = '::immutable_heads()', user can
express (visible) mutable set as '~immutable()'. 'immutable_heads()..' can
terminate early, but a generic difference 'all() & ~immutable()' can't.
Suppose the generation value is usually small, it should be faster to do
bounded range look up first 'y-', then walk ancestors with the unwanted set
'y-..x'.
There's a subtle behavior change that an empty revset is no longer rejected
individually, but I think that's good for "jj duplicate".
cmd_duplicate() was the last caller of index.topo_order().
When an operation is missing and we recover the workspace, we create a
new working-copy commit on top of the desired working-copy commit (per
the available head operation). We then reset the working copy to an
empty tree because it shouldn't really matter much which commit we
reset to. However, when the workspace is sparse, it does matter, as
the test case from the previous patch shows. This patch fixes it by
replacing the `reset_to_empty()` method by a new `recover(&Commit)`,
which effectively resets to the empty tree and then resets to the
commit. That way, any subsequent snapshotting will result keep the
paths from that tree for paths outside the sparse patterns.
As shown by the updated test case, when we recover from a working copy
pointing to a lost operation, the new working-copy commit after
snapshotting will have lost any files outside the sparse patterns.
AST substitution is technically closer to parsing, but the parsed expression
can be modified further by caller. So I think it's better to do optimize() in
later pass.
revset_util::parse() is inlined.
Spotted while moving revset::optimize() around. Since we don't include the
parsing cost of the target expression, we shouldn't include parsing/evaluation
cost of the short-prefixes either. The IdPrefixContext is currently populated
by WorkspaceCommandHelper::new(), but it's hard to tell.
The original plan was to extend the globals table to implement "revset(expr)".
I'm not sure if that's more discoverable than "self.contained_in(revset_expr)"
method, but we can decide that later. Anyways, this patch adds typo suggestion
for global functions.
Prepares for migrating to table-based lookup. It's unlikely that the
implementor handles global function calls differently, but the core doesn't
have an access to the customized symbol table.
This is preparation for #3292, which will use these functions. The main
goal is to merge the parts of #3292 that are likely to cause merge
conflicts with other PRs while I polish it up.
This will be reused for integration with the new `:builtin-web` diff editor in #3292.
`instructions-path_to_cleanup` is moved into DiffWorkingCopies.
DiffWorkingCopies: add instructions_path_to_cleanup
This change updates the language of `jj edit`'s help message to be
more clear as to the nature of the command. It also adds a
recommendation for a more idiomatic/safer workflow.
