This will be needed to test functionality for showing shortest
unique prefix for commit and change ids. As a bonus, this also
allows us to test log output with change ids.
As another bonus, this will prevent occasional CI failures like
https://github.com/martinvonz/jj/actions/runs/3817554687/jobs/6493881468.
Suggested by @arxanas.
Actually, it's easier to support these infix ops than erroring out, but I
don't want to make revset syntax more cryptic. "x- y" can't be handled by
this rule because "x-" is parsed as a parents expression.
Since CR+LF vs LF things shouldn't matter in commit description, it's probably
better to normalize newline characters.
In Mercurial, ui.edit() and changelog.stripdesc() handle line normalization,
and trailing newlines are stripped. In Git, cleanup_message() handles that,
and the last newline is added after stripping trailing newlines.
Otherwise the description set by -m would differ from the one set by editor.
This fixes test_describe() which says "make no changes", but previously "\n"
would be added by the second "jj describe".
As you can see, almost all hashes change in CLI tests. This means in-flight
PRs will need to be rebased to update insta snapshots.
Description text could be normalized by CommitBuilder, but the caller would
have to normalize it beforehand to compare with the current description, so
we would need an explicit function anyway. Another idea is to add a newtype
that represents a normalized description, and make CommitBuilder require it.
Commit::description() will return &Description in place of &str to ensure
that commit.description() == raw_str wouldn't compile.
Git CLI provides --cleanup=<mode> option to switch normalization rules, but
I don't think we'll need such feature.
I ran an upgraded Clippy on the codebase. All the changes seem to be
about using variables directly in format strings instead of passing
them as separate arguments.
"jj log -p --summary" now shows summary and color-words diff, like
"hg log -p --stat".
Handling of "-p" is tricky. I first considered "-p" would turn on the default
diff output, but I found it would be confusing if "jj log -p --git" showed
both color-words and git diffs. So the default format is inserted only if
no --git nor --color-words is explicitly specified.
The author timestamp is rarely useful (in my experience). The
committer timestamp, on the other hand, can be useful for
understanding when a change was most recently modified. IIRC, I
originally picked the author timestamp to match the email (which is
the author's), but it's probably not confusing to use the author email
and the committer timestamp. I suspect few users will even reflect on
it.
Clap bails parsing when an "error" is encountered, e.g. a subcommand is missing,
"--help" is passed, or the "help" subcommand is invoked. This means that the
current approach of parsing args does not handle flags like `--no-pager` or
`--color` when an error is encountered.
Fix this by separating early args into their own struct and preprocessing them
using `ignore_errors` (per https://github.com/clap-rs/clap/issues/1880).
The early args are in a new `EarlyArgs` struct because of a known bug where
`ignore_errors` causes default values not to be respected
(https://github.com/clap-rs/clap/issues/4391 specifically calls out bool, but
strings may also be ignored), so when `ignore_errors` is given, the default
values will be missing and parsing will fail unless the right arg types are used
(e.g`. Option`). By parsing only early args (using the new struct) we only need
to adjust `no_pager`, instead of adjusting all args with a default value.
The number of lines in the diff output is unchanged.
This makes diffs a little more readable when the "..." would otherwise hide a
single line of code that helps in understanding the surrounding context lines.
This change mostly rearranges the loop that consumes the diff lines, so it can
buffer up to num_context_lines*2+1 lines instead of just num_context_lines.
There's a bit of extra code to handle times when a "..." replaces the last line
of a diff.
Note that `jj diff --git` is unchanged, and will still output `@@` lines that
replace a single line of context.
This fixes the bug described in the previous commit.
Because we now print the message about failed exports also while
snapshotting, we may end up reporting it twice on one command. I'm not
sure it's worth worrying about that. We can deal with that later if it
turns out to be a common complaint.
If a branch points to the working-copy commit, it will automatically
get updated when the working copy is snapshotted. However, it turns
out that we don't automatically export the change to Git when running
in a colocated working copy (nor in a non-colocated working copy, but
that shouldn't be surprising). The change will not get exported until
a non-snapshot operation runs. We noticed this bug in @hooper's repo
today.
Unfortunately, config::Value is lax and '[7]' could be parsed as '["7"]'.
I don't like it, but I think that's actually better for consistency as we
use config.get_string() in various places.
When a workspace's working-copy commit is updated from another
workspace, the workspace becomes "stale". That means that the working
copy on disk doesn't represent the commit that the repo's view says it
should. In this state, we currently automatically it to the desired
commit next time the user runs any command in the workspace. That can
be undesirable e.g. if the user had a slow build or test run started
in the working copy. It can also be surprising that a checkout happens
when the user ran a seemingly readonly command like `jj status`.
This patch makes most commands instead error out if the working copy
is stale, and adds a `jj workspace update-stale` to update it. The
user can still run commands with `--no-commit-working-copy` in this
state (doing e.g. `jj --no-commit-working-copy rebase -r @ -d @--` is
another way of getting into the stale-working-copy state, by the way).
Also allows several paths to be specified. By default, `jj resolve`
will find the first conflict that matches provided paths (if any)
and try to resolve it.
It seems like I forgot to update the `jj status` output when I decided
(years ago?) that the changes in a commit should always be compared to
the auto-merged parents. I was very confused before I realized that
`jj status` was showing the diff summary against the first parent. I
suppose the fact that `jj status` lists only one parent should have
been a hint. Thanks to ilyagr@ for finding this odd behavior. This
patch fixes it by making the command list all parents, and changes the
diff summary to be against the auto-merged parents.
If this new option is not specified, we start with empty output
file and trust the merge tool did a complete merge no matter
what the file contains.
Includes tests.
This command uses an external merge tool to resolve conflicts
simple enough that they can be resolved with a 3-way merge.
This commit provides a very basic version of `jj resolve` that
is hardcoded to use vimdiff.
This also slightly changes the errors of the Diff Editor, so that
both the diff editor and `jj resolve can share an error type.
Because a unary negation node '~y' is more primitive than the corresponding
difference node 'x~y', '~y' is easier to deal with while rewriting the tree.
That's the main reason to add RevsetExpression::NotIn node.
As we have a NotIn node, it makes sense to add an operator for that. This
patch reuses '~' token, which I feel intuitive since the other set operators
looks like bitwise ops. Another option is '!'.
The unary '~' operator has the highest precedence among the set operators,
but they are lower than the ranges. This might be counter intuitive, but
useful because a prefix range ':x' can be negated without parens.
Maybe we can remove the redundant infix operator 'x ~ y', but it isn't
decided yet.
I can't see any reason the user would want to specify revisions
matching the empty string, so let's disallow it. I created a custom
type for revision arguments instead of repeating `value_parser =
NonEmptyStringValueParser::new()`.
If the user creates a branch with an empty name, it seems very likely
to be an accident. Let's help them realize that by erroring out.
I didn't add the same checks to `jj branch delete`, since that would
make it hard to delete a branch with an empty name from existing
repos.
Function parameters are processed as local symbols while substituting
alias expression. This isn't as efficient as Mercurial which caches
a tree of fully-expanded function template, but that wouldn't matter in
practice.
Let's acknowledge everyone's contributions by replacing "Google LLC"
in the copyright header by "The Jujutsu Authors". If I understand
correctly, it won't have any legal effect, but maybe it still helps
reduce concerns from contributors (though I haven't heard any
concerns).
Google employees can read about Google's policy at
go/releasing/contributions#copyright.
Aliases are loaded at WorkspaceCommandHelper::new() as it's easier to warn
invalid declarations there. Not all commands use revsets, but many do, so
I think it's okay to always pay the loading cost. Parsing the declaration
part (i.e. a symbol) should be fast anyway.
The nested error message isn't super readable, but seems good enough.
Config syntax to bikeshed:
- naming: [revset-alias] vs [revset-aliases] ?
- function alias will need quotes: 'f(x)' = 'x'
This adds a warning whenever export to the backing Git repo fails,
whether it's by an explicit `jj git export` or an automatic export. It
might be too spammy to print the message after every failed command in
the colocated case, but let's try it and see.
I'm thinking of adding alias expansion at this stage, and it would be a bit
tedious to pass around mutable context by function parameter. So let's reduce
the number of the intermediate functions.
This also produces a better error message.
It would be nice to be able to use snapshot testing and not have to
parse the output of `jj op log`. This patch lets us do that by
providing a new environment variable and config for overriding the
timestamps. Unlike `operation.hostname` and `operation.username`,
these are only meant for tests.
This makes the tests more hermetic, even though I don't think the
default values (taken from `whoami`) can break any tests (then we
would have already seen them break). Now we just need to make the
operation log's timestamps predictable and then we can start using
operation IDs in snapshot tests.
In the test case `test_branch_mutually_exclusive_actions`, we weren't actually testing anything useful, because the interface has since changed to use subcommands instead of options. The test has been deleted in this commit, and `TestEnvironment::jj_cmd_cli_error` has been changed to return a `#[must_use]` `String` representing stderr. I also added `#[must_use]` to `TestEnvironment::jj_cmd_failure` while I was here.
We currently get the hostname and username from the `whoami` crate. We
do that in lib crate, without giving the caller a way to override
them. That seems wrong since it might be used in a server and
performing operations on behalf of some other user. This commit makes
the hostname and username configurable, so the calling crate can pass
them in. If they have not been passed in, we still default to the
values from the `whoami` crate.
We have talked about showing the commit ID only for divergent changes
because it's generally easier to work with the change ID, and it's
less likely to result in a divergent change. However, it's useful to
have the commit ID available for pasting into e.g. a commit message or
the GitHub UI. To try to steer users towards using the change ID, this
commit moves the commit ID off to the right in the log output.
I put it just after the "divergent" field, because that makes it close
to how I imagine it would look if we decided to hide the commit ID
except for divergent changes. I was thinking that could be rendered as
"divergent (abc123)". So if we add config to hide the commit ID, then
it would be rendered almost the same for divergent commits (just with
the added parentheses). It would also make sense to replace the
"divergent" field by a question mark on the change ID, since change
IDs basically behave like branches. If we do that, then the placement
of the commit ID I picked in this commit does not make sense.
Given how easy this was, I can't believe I didn't make the change
sooner.
I haven't updated the screenshots in the readme because I plan to make
some further changes to the default template. I'll update them after
those changes.
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:
1. Import a branch pointing to commit A from Git
2. Modify the branch in jj to point to commit B
3. Export the branch to Git
4. Update the branch in Git to point to commit C
5. Import refs from Git
In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.
This commit fixes the bug by updating the refs in the `MutableRepo`.
Closes#463.
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463). It also lets us remove a `jj export` call
from `test_templater.rs`.
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
Several lines of red text can be overwhelming, and makes it harder to
tell the hint from the error. Let's separate the hint from the error
instead. This matches what hg does. Having the hints separated out
also means that we could have a single config to turn them off.
It's useful to know when you've modified a branch that exists on a
remote. A typical case is when you have pushed a branch to a remote
and then rewritten it. This commit adds an indication in the
`branches` template keyword. A branch that needs to be pushed to a
remote now has a `*` at the end (similar to how conflicted branches
have a `?` at the end). Note that the indication only considers
remotes where the branch currently exists, so there won't be an
indication that the branch has not been pushed to a remote.
Closes#254
The `testutils` module should ideally not be part of the library
dependencies. Since they're used by the integration tests (and the CLI
tests), we need to move them to a separate crate to achieve that.
Unfortunately, TOML requires quotes around the argument. So, the
usage is `jj --config-toml ui.color=\"always\"` in bash. The plan is
to eventually have a `--config` option with simpler syntax for
simple cases.
As discussed in https://github.com/martinvonz/jj/discussions/688.
It seems very likely that we're going to remove support for open
commits, but it's still useful to have a `commit` command that lets
the user enter a description and starts a new change. Calling it
`commit` seems good to make the transition from other VCSs simpler.
The whole file is about the colocated case, where we automatically
import and export on every command, so there should be no reason to
call `git import` here.
