If a file gets replaced by a directory right after list files in a
directory but before we stat the file, we currently crash. Let's
instead treat it as a missing file, using the mechanism introduced for
#258.
This patch makes us treat special files (e.g. Unix sockets) as absent
when snapshotting the working copy. We can consider later reporting
such files back to the caller (possibly via callback) so it can inform
the user about them.
Closes#258
This patch is essentially f6a516ff6d taken further, to also apply to
when we write a symlink or a conflict. As with regular files, these
races seem very unlikely to happen, but I found these cases while
working on #258, so let's fix. Fixing it also means that we don't need
to handle these transition cases in the next patch (when
`file_states()` can indicate that the file is e.g. a socket).
This patch adds `jj obslog -p` for including the diff compared to the
predecessor (the first predecessor if there are several). If the
predecessor's parents are different, then we create a temporary tree
by rebasing the predecessor to have the same parents and we use the
result as base for the diff. That way, we avoid polluting the diff
with the changes caused by the rebase. (I don't think we currently
have any commands that can change both parents and content, so the
diff should always be empty for rewrites caused by a rebase.)
Working on this also reminded me that it'll be really nice when we
replace `jj obslog` by something based on the operation log - I really
miss seeing information about the operation in the output (like `hg
obslog` gets from its obsmarkers).
I was a bit surprised to see the message when I used `jj git push
--change @-` on a commit that already had a branch because I had
pushed it earlier.
The fix means that we instead print the message even if we later
abandon the transaction (so the branch-creation is not persisted)
because the commit is open, for example. That's already what happens
if the commit is missing a description, and since we're planning to
remove the open/closed concept, I don't think this patch makes it much
worse. We probably should improve it later by printing the message
only once the push has succeeded.
I plan to use this matcher for some future `jj add` command (for
#323). The idea is that we'll do a path-restricted walk of the working
copy based on the intersection of the sparse patterns and any patterns
specified by the user. However, I think it will be useful before that,
for @arxanas's fsmonitor feature (#362).
The new `Visit::Nothing` variant lets us more easily restructure
`DifferenceMatcher::visit()` to make it handle the case of not
removing anything. I think this makes the code a little clearer.
I didn't initially create a `Visit::Nothing` variant because I was
worried that the fact that there then are two ways of expressing this
value (there's also `Visit::Specific` with empty sets). However, the
value is quite useful for pattern matching, so I'm now thinking it's
worth the risk.
I often redirect the jj output to pager, so I set ui.color = "always" in
config file. This patch allows me to remove such config, and instead specify
--color=always only when needed.
This allows us to reconfigure ui with the parsed --color option.
I tried if implementing formatter.into_output() would make sense, and it
turned out to be a bit mess as the Formatter trait doesn't know the lifetime
of the underlying output. Ui could own the formatter behind Color|Plain enum
variant in place of Box<dyn>, but that seemed to unnecessarily change the
Ui interface with little benefit.
Since we just want to reinitialize the ui at very early stage, I think
recreating the formatters is the simplest way to go.
Regarding the formatter API, I have a feeling that Ui should keep the
underlying stdout/stderr/color_map instead of the stateful formatters.
ui.stdout_formatter() will return a temporary formatter, and maybe dropping
it will automatically clear labels. This would also means the temporary
formatter could be created with stdout.lock().
ColorChoice implements FromStr so it can be a clap argument. It could
leverage the ArgEnum derive macro, but I don't think the ui is the layer
which can depend on clap.
According to the NO_COLOR FAQ, "user-level configuration files [...] should
override $NO_COLOR." https://no-color.org/
Unfortunately this makes it harder to test the $NO_COLOR behavior since the
test environment isn't attached to a tty. We could allocate a pty or
LD_PRELOAD shim to intercept isatty(), but I feel it would be too much to do.
https://github.com/assert-rs/assert_cmd/issues/138
We have `edition = "2021"` in our `Cargo.toml` files, so it makes
sense to have `rustfmt` use the same edition. That will let us format
code using `async` and `await`. I noticed this while looking at
@arxanas's fsmonitor PR.
This patch prevents perhaps pushing commits with an empty description
or the placeholder "(no user/email configured)" values for
author/committer.
Closes#322.
If a commit's author field has the placeholder user/email values
(i.e. "(no name configured)" and "(no email configured)"), and they
have now configured their email and username, they probably want us to
update the author field with the new information, so that's what this
patch does. Thanks to durin42@ for the suggestion on #322.
None of our subcommands have any arguments at the level of the
subcommand family (e.g. no `jj git --foo fetch`), so we don't need to
wrap the `Subcommand` attribute in an `Args` attribute.
If the source commit becomes empty as a result of
`move/squash/unsquash`, we abandon it. However, perhaps we shouldn't
do that if the source commit is a working-copy commit because
working-copy commits are often work-in-progress commits.
The background for this change is that @arxanas had just started a new
change and had set a description on it, and then decided to make some
changes in the working copy that should be in the parent
commit. Running `jj squash` then abandoned the working-copy commit,
resuling in the description getting lost.
We had tests for `jj move` and `jj squash`, but not for `jj
unsquash`. I'm about to add a `--keep` option, so let's add tests for
the current behavior first.
The regular `Display` format is (not surprisingly) more user-friendly,
as pointed out by @yuja.
I also switched to using format strings for these cases, and some
nearby strings for consistency.
We can easily make the `DirEntry` available here, so we can call
`.metadata()` on that instead of on the `Path`. I think that avoids
walking the path. I'm sure this has no significant impact on
performance, but it's also almost as readable.
When we have just written a file to disk on checkout, let's record the
size we expected instead of what we got from `fstat()`. This should
address a race where the file was modified between the time we wrote
it and the time we requested its stat. I just happened to notice this
while going through the code; it seems very unlikely to be noticed in
practice.
