Include change id as well as (empty) and (conflict) markers in short commit
description.
Unlike `jj log`,
* (conflict) is put in parentheses to distinguish it from the commit
description when the output is not colored.
* the change id is placed right before the commit id to put it into a
predicteble place, even if the short commit description takes up more than one
line. The commit id can be important for commit descriptions in commands like
`abandon`.
The `--allow-large-revset` option for `jj rebase` and `jj new` is used
for allowing a single revset to resolve to more than one destination
commit. It also means that duplicate commits between individual
revsets are allowed (e.g. `jj rebase -d x -d 'x|y'`). I'm about to
replace the first meaning of the flag by a revset function. I don't
think it's worth keeping the flag only for the second meaning, so I'm
just removing the feature instead. We can add it back under a
different name (`--allow-duplicate-destinations`?) if people care
about it.
It seemed awkward if merged PR is sometimes rendered as a first branch.
Instead of sorting edges in index order, let's build a HashSet only when
deduplication is needed.
The original idea was similar to Mercurial's "topo" sorting, but it was bad
at handling merge-heavy history. In order to render merges of topic branches
nicely, we need to prioritize branches at merge point, not at fork point.
OTOH, we do also want to place unmerged branches as close to the fork point
as possible. This commit implements the former requirement, and the latter
will be addressed by the next commit.
I think this is similar to Git's sorting logic described in the following blog
post. In our case, the in-degree walk can be dumb since topological order is
guaranteed by the index. We keep HashSet<CommitId> instead of an in-degree
integer value, which will be used in the next commit to resolve new heads as
late as possible.
https://github.blog/2022-08-30-gits-database-internals-ii-commit-history-queries/#topological-sorting
Compared to Sapling's beautify_graph(), this is lazy, and can roughly preserve
the index (or chronological) order. I tried beautify_graph() with prioritizing
the @ commit, but the result seemed too aggressively reordered. Perhaps, for
more complex history, beautify_graph() would produce a better result. For my
wip branches (~30 branches, a couple of commits per branch), this works pretty
well.
#242
On "jj checkout", description of the working-copy commit is empty, and the
working-copy parent provides more information. It might be a bit verbose to
print parent summary on every history rewriting, but I think that's okay.
@joyously found `o` confusing because it's a valid change id prefix. I
don't have much preference, but `●` seems fine. The "ascii",
"ascii-large", and "legacy" graph styles still use "o".
I didn't change `@` since it seems useful to have that match the
symbol used on the CLI. I don't think we want to have users do
something like `jj co ◎-`.
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.
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.
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.
Previously, using `rebase -r` on the parent of a merge commit
turned it into a non-merge commit. In other words, starting
with
```
o d
|\
o | c
o | b
| o a
|/
o
```
and doing `rebase -r c -d a` resulted in
```
o d
o b
| o c
| o a
|/
o
```
where `d` is no longer a merge commit.
For reference, here's a complete test that passed before this commit (but should NOT pass; see the diff for a test that should pass):
```
#[test]
fn test_rebase_single_revision_merge_parent() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
create_commit(&test_env, &repo_path, "a", &[]);
create_commit(&test_env, &repo_path, "b", &[]);
create_commit(&test_env, &repo_path, "c", &["b"]);
create_commit(&test_env, &repo_path, "d", &["a", "c"]);
// Test the setup
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@
o d
|\
o | c
o | b
| o a
|/
o
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["rebase", "-r", "c", "-d", "a"]);
insta::assert_snapshot!(stdout, @r###"
Also rebased 2 descendant commits onto parent of rebased commit
Working copy now at: 3e176b54d680 (no description set)
Added 0 files, modified 0 files, removed 2 files
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@
o d
| o c
o | b
| o a
|/
o
"###);
}
```
It can be confusing that some commits (typically the working copy)
don't have a description. Let's show a placeholder text in such cases.
I chose the format to match the "(no email configured)" message we
already have.
We didn't have any testing of exit codes on failure, other than
checking that they were not 0. This patch changes that so we always
check. Since we have the special exit code 2 (set by `clap`) for
incorrect command line, I've replaced some testing of error messages
by testing of just the exit code.
As part of this, I also fixed `jj branch --allow-backwards` to
actually require `-r` (it didn't before because having a default value
means the argument is considered always provided).
