I don't see any reason for `TestEnvironent::normalize_output()` to
just delegate to a free `normalize_output()`. By inlining it, we make
it easier to make the normalization to depend on more state.
The case of `remote_ref.is_tracked() && !remote_ref.is_present()` is
unreachable, so the additional condition is unnecessary. If a tracked
and remotely absent bookmark exists, that must be a local one. In which
case, the "bookmark already exists" error case is reached first.
I added this condition in fd5accb75c.
I believe I added it in an iteration of the code where this check was
peformed after the new, renamed bookmark was already created. If that
bookmark was (absently) tracking, we'd be emitting a warning on the
bookmark we just created. After moving the warning earlier in the code,
I forgot that this check can now be simplified.
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
After the previous commit, `MergedTree` and `MergedTreeId` are almost
identical, with the only difference being that `MergedTree` is attached
to a `Store` instance. `MergedTreeId` is also equivalent to
`Merge<TreeId>`, since it is just a wrapper around it.
In the future, `MergedTree` might contain additional metadata like
conflict labels. Therefore, I replaced `MergedTreeId` with `MergedTree`
wherever I think it would be required to pass this additional metadata,
or where the additional methods provided by `MergedTree` would be
useful. In any remaining places, I replaced it with `Merge<TreeId>`.
I also renamed some of the `tree_id()` methods to `tree_ids()` for
consistency, since now they return a merge of individual tree IDs
instead of a single "merged tree ID". Similarly, `MergedTree` no longer
has an `id()` method, since tree IDs won't fully identify a `MergedTree`
once it contains additional metadata.
Currently, creating a `MergedTree` requires reading all of its root
trees from the store. However, this is often not actually required. For
instance, if the only reason to read the trees is to call
`MergedTree::merge`, and the merge is trivial, then there was no need to
read the trees. Changing `MergedTree` to only require a `Merge<TreeId>`
instead of a `Merge<Tree>` will make it possible to avoid reading trees
unnecessarily in these cases.
One benefit of this approach is that `Commit::tree` no longer requires
reading from the store, so it can be made synchronous and infallible,
which simplifies a lot of code.
Since reading subtrees now returns `Merge<Tree>` instead of
`MergedTree`, we can require that `MergedTree` is only used at the root
directory. This will make the next commit simpler since it won't need to
keep track of directories.
This does complicate `jj debug tree`, since it previously relied on
`MergedTree` to iterate over the entries, but since it's only used for
debugging purposes, I think it's fine to make the code more complicated
if it makes `MergedTree` simpler.
I'm planning to make `MergedTree` contain `Merge<TreeId>` instead of
`Merge<Tree>`, and these methods would all require reading a
`Merge<Tree>`, so I think it makes more sense to have them on
`Merge<Tree>` itself. Also, if `MergedTree` contains conflict labels or
other metadata in the future, it would be better to not have to clone
them each time a subtree is accessed.
When `jj status` reports conflicted bookmarks, it suggests running `jj
bookmark list` for details and `jj bookmark set` to resolve the
conflict. However, it's easy to just read the first half of the hint
and run `jj bookmark list` and then not know what to do. Let's remind
the user of the hint in the `jj bookmark list` output if there are
conflicted bookmarks. It should also be useful for users who run `jj
bookmark list` out of habit (not directly because `jj status`
suggested it).
Now that bookmarks can be marked as tracked without a remote one
necessarily being present, we can preserve the tracking state of a
renamed bookmark.
An existing hint tells users to `jj git push --bookmark {new_bookmark}`
in order to rename the bookmark on the remote. That hint actually didn't
work before, because the `--allow-new` flag was required. Now that
tracked state is preserved, the existing hint becomes correct without
the `--allow-new` flag.
This enables configuration to be conditionally applied based on the
hostname set in `operation.hostname`. Users can now use
`--when.hostnames = ["host-a", "host-b"]` in their config files to apply
settings only on specific machines.
Fixes: #6441
Colocation is about sharing the working copy between jj and git. It's
less important where the repo is stored. I therefore think we should
not call it "colocated repo". I considered renaming it to "colocated
working copy" but that sounded awkward in many places because we often
talk about the whole workspace (repo + working copy), so "In colocated
workspaces with a very large number of branches or other refs" sounds
better than "In colocated working copies with a very large number of
branches or other refs".
Once we support colocate workspaces in non-main Git worktrees, I think
this rename will be even more relevant because then all those
workspaces share the same repo but only some of them may be colocated.
Undoing a push operation does not undo the effects on the remote.
Bookmarks on the remote will stay in place, but the local repository
will forget about their state. If the bookmarks are subsequently
moved and pushed, that later push will fail, since the bookmarks have
"unexpectedly" moved on the remote. Therefore, add a warning telling
users to run `jj redo` to avoid these complications.
The default remote parameter of remote_bookmarks() will be derived from this
parameter. It doesn't make sense to exclude @git bookmarks if the backend isn't
Git. It's also nice that parsing tests don't depend on the feature flag.
in clap, the visible_aliases, e.g. '[aliases: --after]' are shown at
the very end, which makes it confusing if you're reading from top to
bottom.
aliases are currently omitted entirely from the man pages, making it
confusing to see undocumented aliases being used.
A typical use case is to query bookmarked revisions ignoring auto-generated
bookmarks. `bookmarks() ~ bookmarks(x)` doesn't work because a revision may have
multiple bookmarks. It's also nice that we can document the default of
`remote_bookmarks()` as `remote_bookmarks(remote=~exact:"git")`.
Closes#7665
If we removed 'b lifetime to use locally-constructed StringMatcher in place of
StringPattern, find_matches() would have to return iterator capturing the
argument of higher-rank lifetime. This patch makes the callback function collect
items. The cost of extra allocation wouldn't matter.
This is part of a series of changes to make most methods on index traits
(i.e. `ChangeIdIndex`, `MutableIndex`, `ReadonlyIndex`, `Index`)
fallible. This will enable networked implementations of these traits,
which may produce I/O errors during operation. See #7825 for more
information.
- Introduced a few more instances of the existing anti-pattern, `TODO:
indexing error shouldn't be a "BackendError"`. We're tracking this
known issue in #7849.
- Converted `MutableRepo::merge_view` to return a `RepoLoaderError`
instead of a `BackendError`. The only caller, `MutableRepo::merge`,
already returns a `RepoLoaderError`.
- Added three "`fallible_`" iterator helpers to reduce the amount of
noise at `is_ancestor` call sites due to the method now being
fallible. Using these helpers seem to produce code that's a little
more readable than using `process_results` from itertools. One
consideration in this trade-off is that these helpers do not
themselves return iterators: if we find that we need more support for
fallible combinators mid-"chain" of iterator combinators, we might
either want to use `process_results` only in those cases, or switch to
use of `process_results` across the board (in lieu of these new
helpers).
This change was motived by the next Gitoxide version adding an
additional lifetime to `gix::reference::iter::Iter`, which currently
breaks the automated Dependabot upgrade.
I left annonymous lifetime annotations on ref-like types like
`RemoteRefSymbol`.
We currently don't allow you to create two identical commits. It
results in an internal error. I think we need to allow it. Let's start
by testing the current behavior.
I created these as CLI tests because I want to make sure that `evolog`
and `op diff` get tested, including rendering.
I think if a particular test sets e.g. $JJ_TIMESTAMP, they would
expect it to get used by `run_jj()` but we currently overwrite a few
variables in that method. This patch makes it so we respect all
environment added to the test environment.
I'm going to use this for a test case that attempts to create
identical commits.
Just moments ago, I ran into a case where I wanted to change both the
author and committer of a change. I knew about the new `jj metaedit`
command, but until now that only gave me the ability to update the
author of my change, so the committer would remain the same. My only
other option was to reach for `jj desc ... --reset-author`, which has
a deprecation warning:
Warning: `jj describe --reset-author` is deprecated; use `jj metaedit --update-author` instead
In spite of that warning, this had the desired effect: both the author
and committer were updated.
So I figured I'd help `metaedit` grow these flags so it can be a _true_
replacement for `jj desc --reset-author`.
---
The above all happened before I was kindly informed in Discord
(https://discord.com/channels/968932220549103686/1431315533164314674)
that `jj metaedit --update-committer-timestamp` should do what I was
hoping for, which I had initially disregarded and never tried because
the help text originally said:
This updates the committer date to now, without modifying the committer.
However, after trying it out, I noticed it actually _did_ modify the
committer.
So, I set out to improve the situation by renaming the flag (so it's
not partially-wrong: it does update the committer timestamp, as well as
everything else about the committer) and improving the help text in the
various flags.
---
In no particular order, thanks to:
* obtuse, for informing me that the `--update-committer-timestamp`
flag does what I wanted in the first place
* gaetan, for pointing out that my way of thinking was not quite meshing
with how jj does things (every modification will update the committer)
* Martin, for the new flag name
This is part of a series of changes to make most methods on index traits
(i.e. `ChangeIdIndex`, `MutableIndex`, `ReadonlyIndex`, `Index`)
fallible. This will enable networked implementations of these traits,
which may produce I/O errors during operation. See #7825 for more
information.
