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.
Follows up c5ed3e1477. Now change/commit ids are resolved at the same
precedence, which means there are at least three types of ambiguity.
I don't think we would need to discriminate these.
Because the use of the change id is recommended, any operation should abort
if a valid change id happens to match a commit id. We still try the commit
id lookup first as the change id lookup is more costly.
Ambiguous change/commit id is reported as AmbiguousCommitIdPrefix for now.
Maybe we can merge AmbiguousCommit/ChangeIdPrefix errors into one?
Closes#799
84b924946f switched to requiring both
SSH_AUTH_SOCK and SSH_AGENT_PID for an agent to be used. This doesn't
seem to be a typical situation, so perhaps it was not intended.
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'
Since syntactic information like symbol or function name is lost after
parse(), alias substitution is inserted to the middle of the post-parsing
stage, not after the whole RevsetExpression tree is built. This is the main
difference from Mercurial. Mercurial also caches parsed aliases, but I don't
think that would have a measurable impact.
The CLI will load aliases from config, insert them one by one, and warn if
declaration part is invalid. That's why RevsetAliasesMap is a public struct
and needs to be instantiated by the caller.
I'll add aliases map, substitution stack (to detect recursion), and locals
(for function aliases) there. Fortunately, we can avoid shared mutables
so a copyable struct should be good.
parse_function_argument_to_string() doesn't need a workspace_ctx, but there
should be no reason to explicitly nullify it either.
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.
To reduce conflicts between branches like `main` and `main/sub`, it's
better to first delete refs in git that have been deleted in jj, and
then add/update refs that have been added/updated in jj.
Since we now write a (partial) view object of the exported branches to
disk (since 7904474320), we can safely skip exporting some
branches. We already skip conflicted branches. This commit makes us
also skip branches that we fail to write to the backing Git repo,
instead of failing the whole operation (after possibly updating some
Git refs).
I made the `export_refs()` function return the branches that
failed. We should probably make that a struct later and have a
separate field for branches that we skipped due to conflicts.
Closes#493.
When skipping branches we fail to update in the backing Git repo, we
must also skip updating the `exported_view` object, so we don't trick
ourselves into thinking the branch was already updated in the Git repo
on the next export.
I'm going to make the export skip branches that we fail to update in
the Git repo. For that, we need to know the branch name while
interacting with the `git2::Repository` object. This little
refactoring prepares for that.
The comment says that we collect the changes to make before making
them, in order to reduce the risk of making some changes before
failing. However, there is nothing in the code that collects changes
that can fail, and it's all doing comparisons in memory, so it should
be very fast. It's been like that since I added it in 47b3abd0f7. We
still need to preserve the structure to avoid mutating `mut_repo`
while iterating over branches, however, so I just updated the comment.
This adds a test for attempting to export both a branch called `main`
and one called `main/sub` (#493), as well as for exporting a branch
with an empty string as name (reported directly to me by @lkorinth).
This adds a `next invocation` command to `fake-editor` that ignores the
rest of the script for *this* invocation, but overwrites the script
with whatever follows `next invocation`.
This is useful to test commands that invoke `fake-editor` several times,
especially the `jj split` command.
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.
This is a simple workaround to make operation IDs predictable (so they
don't include the path to `.../target/debug/jj` or similar). The full
path can probably be useful for troubleshooting, but we can deal with
that later if we see a need for it. It might also be less confusing
for Windows users if it says "jj.exe".
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.
