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.
The `atty` crate seems unmaintained. There's
https://rustsec.org/advisories/RUSTSEC-2021-0145 filed against it,
which `cargo-deny` complains about. A fix for that has been open for
well over a year without being fixed
(https://github.com/softprops/atty/pull/51). It turns out the
functionality is also available via the `crossterm` crate (thanks,
@yuja), which we already depend on.
Since we also depend on `atty` via `clap`, I also added an exception
to the `cargo-deny` config.
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.
