It's useful to have `signature()` live on `UserSettings` because that
will let us cache information (such as the timestamp) in the
instance. It will also make it easier to have the timestamp settable
via regular config files. I don't know that that will be useful, but
it seems like a clean way of implementing it if we can have
environment variables simply as an overlay of configs.
We don't really need a BTreeMap for keeping the unchanged ranges. The
only place it helps a bit is when refining a diff because we may then
insert some more unchanged ranges in the list. I think there has to be
very many unchanged ranges for that to matter, however. This patch
therefore replace the BTreeMap by a sorted Vec. `cargo bench` says
that a few tests got ~20% faster.
I'm looking into this code now because I'm thinking of copying some of
it for the "partial conflict resolution" tool I'm working on for
Mercurial.
I wanted to replace the BTreeMap by a Vec and noticed that we actually
sometimes end up having a `0..n` range followed by a `0..0` after
refinement. We currently compare those two as equal because I had not
thought that we could end up attempting to add two ranges with the
same start point. When trying to insert the second range (`0..0`), the
BTreeMap will keep the existing key (`0..n`) and replace the
value. That's probably works, but it's clearly not what I
intended. Let's fix by sorting by the end point if the start point is
equal. This actually improves some benchmarks by a few percent (maybe
because the subsequent compaction can then remove the `0..0` range).
We used to have documention about how conflicts are implemented, but I
removed that a long time ago when I rewrote the README to target users
rather than VCS hackers. Let's have a doc for the VCS hackers (and
curious users) as well, though.
A few people in different forums asked if there's any reason to use
the native backend and what the reason for its existance is, so let's
document that.
We should only need the `minimal` profile. Also, we already have
configured the action to use the `clippy` component, so I don't think
we need to explicitly add it with a separate `run` action.
The `trim_trailing_whitespace` config is not working well with
multi-line string literals. I've tried to work around
intellij-rust/intellij-rust#5368 twice and now I want to use the
`insta` crate so I'd need to find another workaround. Let's just
disable the config instead. I wouldn't be surprised if other editors
have similar bugs as IntelliJ.
We don't want variables from the developer's environment to affect
tests. You can make tests fail if you set `XDG_CONFIG_HOME` such that
`$XDG_CONFIG_HOME/jj/config.toml` exists and has e.g. an email
set. The fix is to not propagate any environment variables. Thanks for
@arxanas for pointing this out in #104.
We still need to set `$HOME` to prevent configs from being read from
the process owner's home directory (because that's what
`dirs::config_dir()` seems to fall back to if `$HOME` is not set). By
the way, I suspect we'd still not immune to configs from the
developers home directory on Windows, because that doesn't seem to be
controlled by `$HOME`.
When the backing Git repo is inside the workspace (typically directly
in `.git/`), let's point to it by a relative path so the whole
workspace can be moved without breaking the link.
Closes#72.
When using an internal Git repo (`jj init --git`), we make
`.jj/repo/store/git_target` point directly to the repo (which is bare
in that case). It makes sense to do the same when using an external
Git repo (`jj init --git-repo`), so the contents of
`.jj/repo/store/git_target` doesn't depend on whether the user
included the `.git/` on the CLI.
This patch introduces a `JJ_TIMESTAMP` environment variable that lets
us specify the timestamp to use in tests. It also updates the tests to
use it, which means we get to simplify the tests a lot now that that
the hashes are predictable.
We need the app (top-level `clap::Command`) in order to check if
e.g. `-R` was passed to `jj init` (for #101), and it seems cleaner to
pass the instance around than to re-create it when needed.
As pointed out by @arxanas in #88, the message saying something like
"At least 'bin/.DS_Store' was added back ..." is confusing especially
when the command you ran was just `jj untrack bin/.DS_Store`. Let's
clarify the message by saying exactly how many more files there are,
and specialize the message for when there is only one file. Also
update the message to say "would be added back" instead of "was added
back" since we don't actually change anything if some files would be
added back (since 4b91ad408c).
Should we even list all the files? I'm concerned that such a list
could be very long. On the other hand, it can also be annoying to have
to run `jj untrack some/dir/` and only be told about single file to
add to the ignore patterns every time.
The new `.editorconfig` tells editors to strip trailing whitespace,
but IntelliJ has a bug where it strips trailing whitespace even inside
multi-line strings. We can work around it in the cases we have because
they're regexes, so `[ ]` works as a space.
The only overlap between the instructions for Linux, Mac, and Windows
is the `cargo install` step, so let's instead split it up so it's easy
for the user to skip to the relevant part, especially as we add more
platform-specific instructions.
I promoted some sections to one level higher sections to try to
clarify that the initial configuration and the command-line completion
applies to all platforms.
When I recently changed the working copy to not have a commit ID
(e098c01935), I lost the check in `update_working_copy()` in
`commands.rs` that made us not print "Working copy now at: " if the
commit was unchanged. Now we always print, which is unnecessary and
confusing (it makes it seem like the commit changed even if it
didn't). Let's restore the check.
I think I got the current template from Google's open-sourcing
process, but I don't care about most of it (GitHub actions already
check that tests pass, for example). Let's instead tell contributors
that they don't need to write anything at all there and that they
should describe the changes in the commit message(s) instead.
I agree with @unrelentingtech's comment in #54 that the steps to
reproduce should come before the expected and actual behavior. I don't
know where I got the current template from. I think it was from
Google's process for open-sourcing the project.
I also removed the version field from the template for now since I
haven't started updating it regularly yet. I should start doing that.
It can be useful in command prompts and scripts to be able to quickly
get e.g. the `jj status` output without spending time committing the
working copy (perhaps because some background process continuously
commits the working copy). One can already do that by passing
`--at-op=<operation ID>`, but then one needs to look up the operation
ID first. That is both extra work for the user/script and it means
there's an extra `jj op log` invocation to get the operation ID. Let's
have a global flag to make it easy and efficient to do.
Since #85, we load the user's config from a path under
`dirs::config_dir()`. It's probably not obvious to all users where to
put the file, so let's describe that. (I didn't know where to put the
file on my Mac until I looked at the function's documentation.)
Originally, I had thought that these warnings would only potentially show up in nightly because there was a feature which exposed these functions, and we would be able to enable that feature and conditionally not define the conflicting methods. But it looks like these warnings also show up in stable. I've just suppressed each of them individually. Other options would be to rename them and just make them wrapper methods, or to disable `unstable_name_collisions` warnings at a higher scope (possibly including at the crate level).
