There are two reasons for this:
* It helps understanding why divergence happened to see which
workspace an operation was run from. For example, perhaps an agent
is running in another workspace and it's not obvious from the
operation log which commands it ran without seeing the workspace
name.
* We may want to check that e.g. `jj undo` is run from the same
workspace as the operation it's undoing. We may want to print a
warning otherwise, or maybe we want to skip over operations create
from other workspaces. I'm not sure what's best but having the
workspace recorded at least enables such features.
Mostly done with OpenCode and Claude Sonnet 4.6 with the following prompt,
and reviewed manually:
Replace the Result.unwrap with the ? operator in test functions.
While doing that, make sure to:
* Add TestResult as return type in the test functions where you replace
unwrap by the ? operator, and Ok(()) at the end.
* Do the replacement only in method with the `#[test]` decorator.
* Don't replace unwraps in closures.
* Don't replace Option.unwrap.
* Be careful to not change the insta snapshot references.
* Run the tests each time you're done with a file.
* Keep the unwraps that you can't replace with the ? operator.
Note that:
* You can identify all the Result.unwrap locations with this command:
cargo clippy --tests --message-format=short -- -W clippy::unwrap_used 2>&1 | grep / | grep Result | grep <filename>
* Begin with the file with the most Result.unwrap in them as shown by
this command:
cargo clippy --tests --message-format=short -- -W clippy::unwrap_used 2>&1 | grep Result | cut -d: -f1 | sort | uniq -c | sort -nr
* Some non test files have inline tests.
Before this PR:
❯ cargo clippy --tests --message-format=short -- -W clippy::unwrap_used 2>&1 | grep Result | wc -l
3973
After this PR:
❯ cargo clippy --tests --message-format=short -- -W clippy::unwrap_used 2>&1 | grep Result | wc -l
1064
If the user does something like `jj split @` and the working copy is
not empty, they will get warnings like this:
```
Warning: No matching entries for paths: @
Warning: No changes have been selected, so the new revision will be empty
```
If they run the same command when the working copy is empty, they will
instead get this:
```
Error: Refusing to split empty commit e518dba7418a1b20a545c12ecb16cc870756efa3.
Hint: Use `jj new` if you want to create another empty commit.
```
That's probably less helpful because it's not clear that the `@`
argument was interpreted as a path.
The rationale for the error (in 07559f24ec) is that it's confusing
that an empty diff editor pops up if you accidentally try to split an
empty commit. This patch changes it so we print a warning when the
diff is empty instead of starting the diff editor.
Currently, checking out a commit with conflicts in a colocated workspace
causes many editors to show all existing files as added since they
aren't present in the Git HEAD commit. Using the tree of the first side
of the commit as a base tree and then adding the '.jjconflict-*' trees
on top of it would cause any files identical to the first tree to show
as unchanged, matching Git's conflict behavior.
One potential downside to this change is that it becomes harder for a
user to notice if they incorrectly check out a conflicted commit using
`git switch`, since previously all of their files would've disappeared
so it would be more difficult to ignore. However, I think this could be
solved a different way, such as by adding a warning when snapshotting if
'.jjconflict' files are detected. Keeping the files from the first
tree also has the benefit that '.gitignore' files will still be present,
meaning that when a user does `jj abandon` to recover, it won't delete
their ignored files.
Using 'JJ-CONFLICT-README' as the name for the readme file instead of
'README' makes it more clear that this file is related to the
'.jjconflict-*' trees, since now the repo's actual README file might be
present as well in the tree.
Example of conflict resulting from split:
```
<<<<<<< conflict 1 of 1
%%%%%%% diff from: selected changes for split (from rlvkpnrz 35382813)
\\\\\\\ to: split revision (rlvkpnrz 35382813)
line 2.1
+line 2.2
+++++++ qpvuntsm ee8e9376 (parents of split revision)
line 2
>>>>>>> conflict 1 of 1 ends
```
It would be good to include the word "divergent" in the log when a
change is divergent, since users are often unsure what's happening when
they see a divergent change, and giving them a term to search for would
be helpful. However, I don't think it looks good to put this label next
to the change ID itself if both are the same color, since it ends up
being hard to distinguish from the change offset at a glance. Also,
putting the label next to the change ID also messes up the alignment of
fields in the log. Therefore, I think it looks better to put the
"divergent" label at the end of the line.
Since divergence and hidden commits are similar, it makes sense for both
labels to be in the same place, so I also moved the hidden label to the
end for consistency.
One downside is that the labels are less obviously connected with the
change ID itself due to them being farther apart. I think this could be
fine, since they are still visually connected by being the same color.
Running "cargo insta test --workspace", results in numerous warnings:
Snapshot test passes but the existing value is in a legacy format.
Please run `cargo insta test --force-update-snapshots` to update to
a newer format.
This commit is the result of running the suggested command.
Deprecation warnings will be emitted for default "substring:" patterns. This
change will suppress them. Since "glob:" will be the new default, I made these
tests use "glob:" when both "exact:" and "glob:" work.
Tests for the revset filter functions aren't updated.
I'm going to fix parsing of CLI string patterns to use revset parser, and it
would be annoying if inner quotes were required in addition to shell quotes:
$ jj bookmark list 'glob:"push-*"'
There's also a plan to enable glob matching globally. This will mean that we'll
have to use either `subject(*foo*)` or `subject(substring:foo)` for substring
search.
https://github.com/jj-vcs/jj/issues/6971#issuecomment-3067038313
As we have discussed many times on Discord and GitHub, `--destination`
is not a great name because `--insert-before` and `--insert-after` are
also destinations. The most popular alternative seems to be
`--onto`. We already use that term in descriptions in several places,
such as in the help text for `rebase -d` where we say "The revision(s)
to rebase onto". This patch therefore renames the `--destination` flag
to `--onto`.
The short name naturally becomes `-o`. That is perhaps a little
unfortunate because it's a common short name for `--output <file>`
arguments, but we don't use that anywhere so it seems fine.
Perhaps we should also rename `--source` (used by `rebase` and `fix`)
to something else. Perhaps the most obvious name is `--descendants`,
but the short form would be `-d`, which is of course already taken by
`--destination` in the case of `rebase`. Either way, I'm leaving that
rename for later. It would be good to do it before next release if we
are going to do it, though.
Closes#7941
The operation timestamp is often very close to the committer timestamp, so I
don't think it's useful in evolog. I also removed "root()" as it wouldn't be
useful either, and the root operation should have no evolved commits.
I had a report from a user at Google who was confused about a config
deprecation message because they had forgotten that they set a
repo-level config. This patch includes the source level in the warning
message. Hopefully that's sufficient. We could of course print the
path too, but that would make the message much longer so we would have
to split it up on two lines and I'm not sure it's worth it.
This is the last part of the series 02722eae54 "view: port bookmark/tag name
types to RefName/RemoteName." Templater doesn't use these name types internally
because we wouldn't gain much type safety in template language.
The return type of .remote() is changed to Option<RefSymbol>. An empty symbol is
rendered with quotes, so it would be weird if local_bookmark.remote() returned
an empty RefSymbol.
the feature reused as much as possible of the `jj rebase` code, with
minimum modifications, in order to avoid reimplementing or complexifying
the rebase code.
It leads to some cases where some commits may be rewritten twice.
For example:
❯ jj -s
@ ylrkuwto (empty) (no description set)
○ qsorlqym file4
│ A file4
○ mmkvzzpw file3
│ A file3
○ uttlrpzl file1&2
│ A file1
│ A file2
◆ zzzzzzzz root()
❯ jj split -r u -A m -m file1 file1
Rebased 3 descendant commits
First part: ulpvupsv aaf064bc file1
Second part: uttlrpzl 0d4358c2 file1&2
Working copy (@) now at: ylrkuwto 60d905c5 (empty) (no description set)
Parent commit (@-) : qsorlqym 8cfbe2d8 file4
❯ jj -s
@ ylrkuwto (empty) (no description set)
○ qsorlqym file4
│ A file4
○ ulpvupsv file1
│ A file1
○ mmkvzzpw file3
│ A file3
○ uttlrpzl file1&2
│ A file2
◆ zzzzzzzz root()
There is an intermediate state created as
@ ylrkuwto (empty) (no description set)
○ qsorlqym file4
│ A file4
○ mmkvzzpw file3
│ A file3
○ uttlrpzl file1&2
│ A file2
○ ulpvupsv file1
│ A file1
◆ zzzzzzzz root()
where the commits in `uttlrpzl::` have been rewritten, then `ulpvupsv`
is moved, effectively rewriting the commits in `ulpvupsv::`.
The number of user visible rewritten commits is accurately reported.
`jj split --parallel` is still done as a special case: implementing
it with the rebase code would put the first commits on the right
branch.
We haven't had any reports of problems from people who opted in. Since
it's early in the release cycle now, let's now test it on everyone who
builds from head, so we get almost a month of testing from those
people before it's enabled by default in a released version.
This impacts lots of test cases because the change-id header is added
to the Git commit. Most are uninteresting. `test_git_fetch` now sees
some divergent changes where it used to see only divergent bookmarks,
which makes sense.
to set the message on the first commit, where the selected
changes are going. The commit with the remaining changes
keeps the original description.
With this option it becomes possible to use split in a fully
non-interactive way, in combination with the filset feature.
It also makes jj split more consistent with other commands
like squash, commit or new.
Since we now have the template alias, it's easy to migrate the config value
programatically. However, there are a couple of minor behavior changes:
a. default description won't be used if user customized the
draft_commit_description template.
b. default description won't be inserted if trailer is added
For (a), I assumed user would inline the default description in the draft
template if they customized the template. (b) should be okay because the
trailers template is a new feature.
Since 7618b52b "cli: consider 'JJ:' lines as comments also when not followed by
space", lines starting with "JJ:" (without space) are also ignored. We can
simply add "JJ:" prefix to empty intro/instruction lines.
Closes#5484
This helps detect whether the last line is "JJ:" instruction or not. It seems
also nice that I don't have to insert newline to reflow the edited paragraph.
This is to ensure two things:
1. That the evolog isn't missing the history from the target commit.
2. That the evolog doesn't include extra "temporary" commits due to
the way `jj split` is implemented.
The impetus for this is the discussion in https://github.com/jj-vcs/jj/pull/5926.
The changes added here detect the extra temporary commit added to the evolog
when `split --parallel` is run using the implementation in #5926.
It wasn't immediately obvious to me what was happening here, but once I
understood it it seemed pretty simple. Perhaps it's worth a comment to explain
it to the next reader.
The consensus about this change, if one ever existed, seems to have dissolved
into two cohorts, one which is happy with the change, and one which thinks we
should move both the bookmark and change id to the child commit instead of
leaving them on the parent.
We may decide to add flags to allow users to choose between the two behaviors,
but there are also other concerns such as where @ should go (parent or child).
Until we agree on a path forward it seems reasonable to delay the breaking
change by disabling it via the config option we added. I don't think it's
necessary to fully revert the code and new tests since we aren't announcing the
option.
#3419
This should help prevent misuse of "jj bookmark set" with e.g. remote bookmark
name. A remote-looking bookmark can be created by quoting the name if needed.
This patch doesn't change the parsing of name patterns. For patterns, I think
it's better to add support for compound expressions (e.g. `glob:foo* & ~bar`)
rather than adding revset::parse_string_pattern(text) -> StringPattern.
Closes#5705
