Unlike matcher_from_values(), this function is trivial and isn't widely
used. Let's simply do .map() and .collect().
TODO comment is relocated to matcher_from_values(). I think glob and
fileset-like stuff will be added to the matcher parser, not to the
'Path -> RepoPath' function. And it's probably implemented in lib crate.
It seems a bit invasive that RepoPath constructor processes an environment
like cwd, but we need an unmodified input string to build a readable error.
The error could be rewrapped at cli boundary, but I don't think it would
worth inserting indirection just for that.
I made s/file_path/fs_path/ change because there's already to_fs_path()
function, and "file path" in RepoPath context may be ambiguous.
This patch addresses TODOs described in parse_file_path_wc_in_cwd() test.
Since the input string is considered a filesystem path, I think it makes
sense to normalize the cwd + input path first.
These utility functions will probably be moved to lib to implement file()
revset resolution.
Let the test harness suppress uninteresting output. Anyway, these tests
would print nothing.
I think Ui::with_cwd() can also be removed after refactoring file path
handling.
I find myself using this argument almost every time, but it's
long to type. At the same time, fish completion does not work well
for `--allow-backwards` for some reason (though I think an alias would
be nice regardless).
There's a risk of forgetting to call `remove_label()` and I've wanted
to reduce that risk for a long time. I considered creating RAII
adapters that implement `Drop`, but I didn't like that that would
ignore errors (such as `BrokenPipe`) that can happen while emitting an
escape sequence in `remove_label()`. I would ideally have liked
Python's context managers here, but Rust doesn't have that. Instead,
we get to use closures. That works pretty well, except that we can't
return other errors than `io::Error` inside the closures. Even with
that limitation, we can use the new `with_label()` method in all but a
few cases.
We can't define the `with_label()` method directly in the trait
because `&mut self` is not a trait object, so we can't pass it on to
the closure (which expects a trait object). The solution is to use
`impl dyn Formatter` (thanks to @kupiakos for figuring that
out!). That unfortunately means that we can *only* call the function
on trait objects, so if `f` is a concrete formatter type
(e.g. `PlainTextFormatter`), then `f.with_label()` won't
compile. Since we only ever access the formatters as trait objects,
that's not really a problem, however.
It seems simpler to do the snapshotting after merging any concurrent
operations instead of snapshotting on top of one of the operations,
especially since the attempt to snapshot may end up noticing that the
working copy is stale.
More importantly, snapshotting before resolving operations resulted in
a crash if the working copy was modified. That happened because we
held a lock on the operation heads (`locked_op_heads`) while we tried
to record the operation committing the working copy. I noticed this
only after adding the test.
We don't always want to do the snapshotting etc. right away, so let's
have the callers decide when to do it. This is just a refactoring;
I'll change the behavior slightly in separate commits.
This patch moves most of the remainder of
`WorkspaceCommandHelper::for_loaded_repo()` into a new function. We'll
inline `WorkspaceCommandHelper::for_loaded_repo()` next.
`WorkspaceCommandHelper::for_loaded_repo()` creates a new instance,
imports Git refs (if using the Git backend), and snapshots the working
copy. This patch adds a new constructor function that only does the
first step. I think we'll want that functionality when we add a
command for updating a stale working copy (instead of doing it
automatically as we currently do).
This commit adds a reminder in `finish_transaction()` if the user
hasn't configured their name and email. That means they'll get a
reminder after most mutating commits, except for commands that only
snapshot the working copy, and a few more cases.
Closes#530.
The native backend is just a proof of concept and there's no real
reason to use it other than for testing, so let's reduce the risk of
accidentally creating repos using it.
Since the concrete Formatter type is hidden behind the Ui, there wouldn't
be many reasons to use dyn Write at the formatter layer. This allows us
to create a formatter against MutexGuard<Box<dyn Write>> without one more
Box<dyn Write> wrapper.
This should help to create a temporary ColorFormatter instantly.
A cached_colors table could also be shared across formatters, but doing that
would require some locking mechanism. Since commands like cmd_log/diff()
use a single formatter instance, I don't think shared mutable cache would be
needed for the moment.
This allows callers to instantiate long-lived formatter. ui.write() can't
be invoked while formatter is borrowed. My plan is to make Ui directly hold
output streams so the color settings can be easily reset.
ui.stdout/stderr_formatter() will create a temporary formatter there.
write_commit_summary() could be hosted by WorkspaceCommandHelper to eliminate
some of the function arguments, but that would make the source RepoRef unclear
while transaction is in progress.
Let WorkspaceCommandHelper clone it. WorkspaceCommandHelper could return
workspace_id by reference, but doing that would introduce noisy .clone()
calls and lifetime mess.
I'll make TreeState::snapshot() return a boolean denoting whether tree_state
is updated or not. New tree_id can be obtained from TreeState, so let's
remove it from the return value.
I was reading a draft of "Git Rev News: Edition 91" [1] where Peff
mentions some unfinished patches to allow negative timestamps in
Git. So I figured I should add support for that before I forget. I
haven't checked if libgit2 supports it, so it might be that our Git
backend still doesn't support it after this patch.
[1] https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-91.md
I changed the "GLOGAL OPTIONS" help heading to use title case, to
match clap's new help style.
I also removed our override of the help text for `-h, --help` because
the default text now includes "(use `-h` for a summary)" (and it's
harder override now).
Perhaps the most obvious difference to users will be the changed help
output
(https://epage.github.io/blog/2022/09/clap4/#polishing-help-output). It
no longer has color, and long lines are not wrapped. I suppose we
should wrap the text ourselves instead..
When we resolve aliases and we try to resolve something that is not a
real command and not an alias, we currently replicate `clap`'s error
message. I don't know why I did that because it seems that we should
just stop resolving aliases and let `clap` try to parse what we have
resolved at that point. This patch makes that change, thereby removing
an additional call to `process::exit()`.
In many of these places, we don't need an owned value, so using a
reference means we don't force the caller to clone the value. I really
doubt it will have any noticeable impact on performance (I think these
are all once-per-repo paths); it's just a little simpler this way.
I feel it doesn't make sense for a simple getter function to create an
owned vec after 0108673087 "backend: let each backend handle root commit
on write."
This commit moves much of the `commands` module that isn't specific to
a particular command into a new `cli_util`.
Much of this is actually not even CLI-specific, so we should move that
further down into the library, but that can come later. That includes
the code in `WorkspaceCommandHelper` for snapshotting the working copy
and automatically importing/exporting to a colocated Git working copy.
I'm trying to make it easier to create a custom `jj` binary with
custom commands, without having to replace the `Args` type we parse
arguments into. For that to work, we need to resolve aliases using the
`clap::Command` type instead.
Now that open commits are disabled by default, seeing `close` and
`open` in `jj help` is confusing.
I'm a little ambivalent about hiding `jj commit` because it's useful
as a way of adding a description and starting a new change with a
single command. We can always add a new command for that if we want
to.
As pointed out by IntelliJ (maybe it came from rust-analyzer or
something, I don't know). I also lifted out
`Err(CommandError::UserError())` and extracted a variable.
Commands that take both paths and revisions as arguments take paths as
positional arguments and use `-r` for revisions (or more specific
options like `--from` and `--to`). It's therefore natural for the user
to think that they can run e.g. `jj describe -r abc123`. However,
since `jj describe` doesn't accept paths, it doesn't have a `-r`
argument either. Let's allow the user to specify a superfluous `-r`
that we just ignore.
This handling means that the user could run `jj show -r` (without a
revision argument) and it would use the default revision. That's
confusing, but at least we won't accept e.g. `jj show -r README.md`
(unless `README.md` is a revision), so it's probably okay?
`wc_commit` seems clearer than `checkout` and not too much longer. I
considered `working_copy` but it was less clear (could be the path to
the working copy, or an instance of `WorkingCopy`). I also considered
`working_copy_commit`, but that seems a bit too long.
I had missed that `Ui::write_commit_summary()` still shows open
commits in green even when open commits are disabled (as they are by
default these days). This patch fixes that.
The user can still override templates to format open commits
differently, but I'm not sure it's worth fixing that.
In the current implementation, tree is diffed twice if both PATH and -p
are specified. If this adds significant cost, we'll need to reimplement
it without using a revset abstraction (or maybe adjust revset/graph API.)
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
"###);
}
```
While debugging stale working-copy recovery, I noticed the checkout commit
resurrected from the ancestor_op was way beyond the ancestor commit made by
jj squash. This patch is a workaround for the problem.
I'm not pretty sure if updating operation id is the right thing. If not, maybe
we'll need to look up more recent operation corresponding to the wc tree id from
the <ancestor_op>: range?
Otherwise directories containing files cannot be removed.
I think making files readonly is enough to tell user that the left side
cannot be edited, but I haven't tested that with Meld. If that's not enough,
we'll probably need to undo set_readonly_recursively() at exit.
I keep explaining the functionality as hiding the commit, so it seems
that `jj hide` is a natural name for it. Let's start by adding it as
an alias. My only hesitation with making `hide` the real command name
and `abandon` an alias that we start phasing out is that it feels more
natural to `abandon` the working copy in order to start working on a
new change than it does to `hide` it.
Add the `jj interdiff` command for comparing only the diffs of commits.
Its args are identical to that of `jj diff`, minus `--revision` (because
interdiff always requires two commits).
Like `jj obslog -p`, Changes introduced by intervening commits are
ignored by rebasing `--from` onto `--to` 's parents.
`jj merge` just creates an empty change, which is practically the same
as `jj new`. The main difference is that the former requires more than
one argument and the latter requires at most one argument. It seems
cleaner to generalize them and make them aliases. This patch starts
doing that by making `jj new` accept more than one argument.
Instead of having `jj merge` be exactly an alias for `jj new`, we may
want to make it a thin wrapper that just checks that more than one
argument was given. That would probably be less confusing to users who
run `jj merge` without arguments to see what it does.
We should probably make `jj checkout` also be an alias for `jj new`,
but that will have to wait until we have removed support for open
commits (since `jj checkout` still has logic for dealing with open
commits).
