When we materialize modify/delete conflicts, we currently don't
include any context lines. That's because modify/delete conflicts have
only two sides, so there's no common base to compare to. Hunks that
are unchanged on the "modify" side are therefore not considered
conflicting, and since they they don't contribute new changes, they're
simply skipped (here:
3dfedf5814/lib/src/files.rs (L228-L230)).
It seems more useful to instead pretend that the missing side is an
empty file. That way we'll get a conflict in the entire file.
We can still decide later to make e.g. `jj resolve` prompt the user on
modify/delete conflicts just like `hg resolve` does (or maybe it
actually happens earlier there, I don't remember).
Closes#1244.
Since type/name checking is made after alias substitution, we need to preserve
the original context to generate a readable error message.
We could instead attach a stack of (alias_id, span) to ExpressionNode, but
the extra AliasExpanded node helps to capture downstream error by a single
.map_err() call.
Supported values are,
- `none` for no author information,
- `full` for both the name and email,
- `name` for just the name,
- `username` for username part of the email,
- (default) `email` (or any other gibberish for that matter) for the full email.
The added expect_arguments() is basically a copy from the template_parser.
I'll reimplement it to support keyword arguments, so I don't care much about
the current implementation.
I leave expect_no/one_argument() as wrappers because parsing 0/1 arguments
is pretty common.
Error messages are slightly changed. I personally prefer not to add extra
code for singular/plural handling, but if we do, I'll add 'if N == 1' case.
The unique-prefixe tests are typically the slowest tests. Here's the
end of my `cargo nextest --workspace` output (from an arbitrary run,
not best-of-5 or anything):
PASS [ 5.129s] jujutsu::test_log_command test_log_prefix_highlight_brackets
PASS [ 5.220s] jujutsu::test_log_command test_log_prefix_highlight_styled
PASS [ 8.523s] jujutsu::test_log_command test_log_prefix_highlight_counts_hidden_commits
These tests create 50-100 commits in a loop. I think much of it comes
from the subprocessing and/or the repeated loading of the repository
in the subprocesses. Rewriting them to use `jj duplicate` for creating
many commits at once speeds them up. Here are the timings after:
PASS [ 2.323s] jujutsu::test_log_command test_log_prefix_highlight_styled
PASS [ 2.330s] jujutsu::test_log_command test_log_prefix_highlight_brackets
PASS [ 3.773s] jujutsu::test_log_command test_log_prefix_highlight_counts_hidden_commits
I felt that the config is too narrow to have it's own top-level [diff]
section, and I couldn't think of another good place to have it. I'm
happy to hear other suggestions.
This will be used as a parameter of id.shortest*() methods. For now, only
decimal literal is supported, and there's no unary negate operator.
"0"-prefix is disallowed because it looks like an octal number.
I don't think we would want multiple integer types in the template language,
so I chose i64 as the integer type of reasonable width.
We have moved from saying "committing the working copy" towards saying
"snapshotting the working copy". More importantly, the option also
means that we don't update the working copy at the end. I went with
the `--ignore-working-copy` name suggested by Ilya. I also updated the
documentation of the option.
Even though the template syntax is experimental, panicking parser makes
it difficult to write tests. So let's add minimal error handling. The error
types are basically copied from the revset module.
I made write_commit_summary() fall back to the default template if user
template had syntax error. It should be better than reporting parse error
after e.g. "jj abandon" finished successfully.
Accept an --include-defaults arg to enable including those.
Listing a nonexistent name is no longer an error, but does output a
warning to stderr when no config entries match to explain why there's no
other output.
I have no idea whether or not any template expressions are intentionally
allowed as a label, but it makes sense to write something like
'label("phase-" phase, ...)' (if we had a phase keyword.) So I decided to
add .into_plain_text() instead of stricter .try_into_string().
This allows us to use "if(description,)" to test empty description. And
I think this change is unavoidable if we want to add support for commit
template.
I think this is the same bug as reported in #922, just simplified a
bit further. The branches in the repo actually look good after the
`undo` operation, but the reverted `master` branch doesn't get
exported to the git repo even though our recorded `refs/heads/master`
in the repo was moved back. Then the next automatic import on `log`
notices that the `master` branch in the git repo still points to the
new commit, and that commit becomes visible again.
Fix a bug where `jj git push` would print "No current branch." when
there is a current branch but it is unchanged. We were conflating the
two because we print the message when no updates were performed, instead
of only when no branches were found.
Add a new git.auto-local-branch config option. When set to false, a
remote-tracking branch imported from Git will not automatically create a
local branch target. This is implemented by a new GitSettings struct
that passes Git-related settings from UserSettings.
This behavior is particularly useful in a co-located jj and Git repo,
because a Git remote might have branches that are not of everyday
interest to the user, so it does not make sense to export them as local
branches in Git. E.g. https://github.com/gitster/git, the maintainer's
fork of Git, has 379 branches, most of which are topic branches kept
around for historical reasons, and Git developers wouldn't be expected
to have local branches for each remote-tracking branch.
Suppose "template" is a sequence of "term"s, it makes more sense to handle
an empty sequence there. It might be even better to disallow empty template
other than the top-level one.
A "list" is a sequence of more than one "term" nodes, so it shouldn't contain
a single parenthesized node.
Also, a parenthesized "term" rule wasn't handled.
I think of it more as style than a format, so using `style` in the
config key makes sense to me.
I didn't bother making upgrades easy by supporting the old name since
this was just released and only a few developers probably have it set.
The name of the [alias] section is inconsistent with other
table-valued sections ([revset-aliases], [colors], [merge-tools]), so
let's rename it. For comparison, `Cargo.toml` also uses plural names
(e.g. `[dependencies]`).
I don't think need to write non-UTF8 bytes to our config files. If we
ever do (maybe to test that we give the user a reasonable error
message), we add a custom function for that.
Fixes#787
If `jj squash` is run on an empty commit, it fails with "Error: No changes selected"
With this change such squash command will behave like `jj abandon`.
I would expect `set_up_fake_[diff_]editor()` to create an empty script
but it turns out they didn't even create the files. That means that
the caller needs to write an empty script to them if they want the
fake editors to not do anything. Let's instead write the empty
scripts, for a less surprising behavior.
I've preferred "working-copy commit" over "checkout" for a while
because I think it's clearer, but there were lots of places still
using "checkout". I've left "checkout" in places where it refers to
the action of updating the working copy or the working-copy commit.
This should fix the panic in the case reported in #1107. It's a bit
hard to reproduce because we normally notice the missing commit when
we snapshot the working copy, but it's possible to reproduce it using
`--no-commit-working-copy`.
I suspect the added test is too brittle because it checks the exact
error message. On the other hand, it might be useful to have one test
case like this so we catch accidental changes in the format.
This is ugly, but we need a special case because root_change_id and
root_commit_id aren't equal but share the same prefix bytes. In practice,
no one would care for the shortest root id prefix, but we'll need to deal
with a similar problem when migrating prefix id resolution to repo layer.
I think the intent of '- 1' here is the separator length, which was
originally ':'. Alternatively, we can ensure that prefix + remainder is
always 12 chars.
`jj cat` better matches `hg cat` and, of course, `cat`. I apparently
called it `jj print` when I added it in 7a013a59ae because I haven't
found `hg cat` useful for actually concatenating files. That's still
true, and I don't know if we will ever bother to teach `jj cat` to
actually concatenate files, but I think the familiarity of `cat` is
more important.
For reference, Git calls it `git show <rev>:<file>`.
I kept `print` as an alias and added a test for it. I also documented
the test better.
By the way, I've considered adding a command for writing from stdin
directly to a specific commit. If we ever do, it might make sense to
call that command `write` (e.g. `echo foo | jj write -r @-
README.md`). Then it would make sense to add `read` as an alias to
`cat`. I'm not sure that's a good idea, but let's leave that for later
anyway.
This creates a templater function `short_underscore_prefix` for commit and
change ids. It is similar to `short` function, but shows one fewer hexadecimal
digit and inserts an underscore after the shortest unique prefix.
Highlighting with an underline and perhaps color/bold will be in a follow-up
PR.
The implementation is quadratic, a simple comparison of each id with every
other id. It is replaced in a subsequent commit. The problem with it is that,
while it works fine for a `jj`-sized repo, it becomes is painfully slow with a
repo the size of git/git.
Still, this naive implemenation is included here since it's simple, and could
be used as a reference implementation.
The `shortest_unique_prefix_length` function goes into `repo.rs` since that's
convenient for follow-up commits in this PR to have nicer diffs.
The `indexmap` crate is used to make `duplicate`'s output have a sane order,
making it easier to test.
It's also used later to remove duplicate revisions in the `abandon` command.
Fixes https://github.com/martinvonz/jj/issues/1050
Thanks to Martin for suggesting the exact fix.
The tests go into the new tests/test_duplicate_command.rs, which will be
expanded shortly with other tests depending on this bugfix.
There are several reasons for this:
* We can more easily skip styling a trailing blank line, which other
internal code then can correctly detect as having a trailing
newline. This fixes the TODO in tests/test_commit_template.rs.
* Some tools (like `less -R`) add an extra newline if the final
character is not a newline (e.g. if there's a color reset after
it), which led to an annoying blank line after the diff summary in
e.g. `jj status`.
* Since each line is styled independently, you get all the necessary
escapes even when grepping through the output.
* Some terminals extend background color to the end of the terminal
(i.e. past the newline character), which is probably not what the
user wanted.
* Some tools (like `less -R`) get confused and lose coloring of lines
after a newline.
We often end up writing escape codes for one style and then
immediately after, we write escape codes for another style. That seems
harmless, but it's a little ugly. More importantly, it prepares for
not emitting any escapes for turning off attributes at the end of
formatted contents across multiple lines (see next commit).
Let's use `crossterm` to make `ColorFormatter` a little more readable,
and maybe also more portable.
This uses the `SetForegroundColor()` function, which uses the escapes
for 256-color support (code 38) instead of the 8-color escapes (codes
30-37) combined with bold/bright (code 1) we were using before. IIUC,
most terminals support the 16 base colors when using the 256-color
escape even if they don't support all the 256 colors. It seems like an
improvement to use actual color codes for the bright colors too,
instead of assuming that terminals render bold as bright (even though
most terminals do).
Before this commit, we relied on ANSI escape 1 - which is specified to
make the font bold - to make the color brighter. That's why we call
the colors "bright blue" etc. When we switch from using code 30-37 to
using 38 to let our color config just control the color (not using
escape1), we therefore lose the bold font on many terminals (at least
in iTerm2 and in the terminal application on my Debian work
computer). As a workaround, I made us still use escape 1 when the
bright colors are used. I'll make boldness a separately configurable
attribute soon. Then we'll be able to remove this hack.
With the switch to `crossterm`, we also reset just the foreground
color (code 39) instead of resetting all attributes (code 0). That
also seems like an improvement, probably making it easier for us to
later support different background colors, underlining, etc.
This can be disabled with the new `--quiet` option.
Printing every commit would give the user all the information of how to undo
this or where to `jj restore` from if they realize they need to after the
abandon.
If all file names are short enough, this will align the conflict
description at 4 spaces past the longest file name, as follows.
```
src/templater.rs 2-sided conflict
src/templater_parsers.rs 2-sided conflict
```
If there is a long file name, conflict descriptions will either start
or column 35 or one space from a file name, whichever is later.
Previously, a single tab was used to separate file name from conflict
description. This didn't look as nice as I hoped when multiple files
are involved. I also don't think `jj` generally uses tabs in output.
The tab uncovered a bug in a watcher program I was using.
If a workspace path is explicitly specified, it must point to the exact
workspace directory. This is the same behavior as 'hg -R'. OTOH, 'git -C'
is the option to chdir, so it makes sense to search .git from that directory.
This also fixes 'jj -R ../..' which would previously look up '../..', '..',
'.', ...
I don't think Workspace::load() should be permissive in that regard.
WorkspaceLoader could provide such function, but I feel it's more like
CLI business. CLI can also look for parent '.git' directory to suggest
'jj init --git-repo=..' if needed.
The divergent changes are marked with ?? (I found a single ? a bit easy to
overlook), and this should be consistent.
Ideally, the conflicted branches would also be red, but this takes a bit
larger changes to `templater.rs`: another `Property` as well as changes to
https://github.com/martinvonz/jj/blob/7f9a0a28/src/template_parser.rs#L385-L395
An important part of the `jj log` output is the commit template. I
suspect we'll have many more tests for that, so let's move it to a
separate file. The main `test_log_command.rs` can focus on which
commits to display.
The `:` was a bit noisy. With upcoming color support, it seems unnecessary.
This should also be a little better for any tools that want to parse the
output.
Finally, `insta` also seems to want to rewrite test snapshots to be shorter.
They seem equivalent.
Since per-repo config may contain CLI settings, it must be visible to CLI.
Therefore, UserSettings::with_repo() -> RepoSettings isn't used, and its
implementation is nullified by this commit.
#616
This will be needed to test functionality for showing shortest
unique prefix for commit and change ids. As a bonus, this also
allows us to test log output with change ids.
As another bonus, this will prevent occasional CI failures like
https://github.com/martinvonz/jj/actions/runs/3817554687/jobs/6493881468.
Suggested by @arxanas.
Actually, it's easier to support these infix ops than erroring out, but I
don't want to make revset syntax more cryptic. "x- y" can't be handled by
this rule because "x-" is parsed as a parents expression.
Since CR+LF vs LF things shouldn't matter in commit description, it's probably
better to normalize newline characters.
In Mercurial, ui.edit() and changelog.stripdesc() handle line normalization,
and trailing newlines are stripped. In Git, cleanup_message() handles that,
and the last newline is added after stripping trailing newlines.
Otherwise the description set by -m would differ from the one set by editor.
This fixes test_describe() which says "make no changes", but previously "\n"
would be added by the second "jj describe".
As you can see, almost all hashes change in CLI tests. This means in-flight
PRs will need to be rebased to update insta snapshots.
Description text could be normalized by CommitBuilder, but the caller would
have to normalize it beforehand to compare with the current description, so
we would need an explicit function anyway. Another idea is to add a newtype
that represents a normalized description, and make CommitBuilder require it.
Commit::description() will return &Description in place of &str to ensure
that commit.description() == raw_str wouldn't compile.
Git CLI provides --cleanup=<mode> option to switch normalization rules, but
I don't think we'll need such feature.
I ran an upgraded Clippy on the codebase. All the changes seem to be
about using variables directly in format strings instead of passing
them as separate arguments.
"jj log -p --summary" now shows summary and color-words diff, like
"hg log -p --stat".
Handling of "-p" is tricky. I first considered "-p" would turn on the default
diff output, but I found it would be confusing if "jj log -p --git" showed
both color-words and git diffs. So the default format is inserted only if
no --git nor --color-words is explicitly specified.
The author timestamp is rarely useful (in my experience). The
committer timestamp, on the other hand, can be useful for
understanding when a change was most recently modified. IIRC, I
originally picked the author timestamp to match the email (which is
the author's), but it's probably not confusing to use the author email
and the committer timestamp. I suspect few users will even reflect on
it.
Clap bails parsing when an "error" is encountered, e.g. a subcommand is missing,
"--help" is passed, or the "help" subcommand is invoked. This means that the
current approach of parsing args does not handle flags like `--no-pager` or
`--color` when an error is encountered.
Fix this by separating early args into their own struct and preprocessing them
using `ignore_errors` (per https://github.com/clap-rs/clap/issues/1880).
The early args are in a new `EarlyArgs` struct because of a known bug where
`ignore_errors` causes default values not to be respected
(https://github.com/clap-rs/clap/issues/4391 specifically calls out bool, but
strings may also be ignored), so when `ignore_errors` is given, the default
values will be missing and parsing will fail unless the right arg types are used
(e.g`. Option`). By parsing only early args (using the new struct) we only need
to adjust `no_pager`, instead of adjusting all args with a default value.
The number of lines in the diff output is unchanged.
This makes diffs a little more readable when the "..." would otherwise hide a
single line of code that helps in understanding the surrounding context lines.
This change mostly rearranges the loop that consumes the diff lines, so it can
buffer up to num_context_lines*2+1 lines instead of just num_context_lines.
There's a bit of extra code to handle times when a "..." replaces the last line
of a diff.
Note that `jj diff --git` is unchanged, and will still output `@@` lines that
replace a single line of context.
This fixes the bug described in the previous commit.
Because we now print the message about failed exports also while
snapshotting, we may end up reporting it twice on one command. I'm not
sure it's worth worrying about that. We can deal with that later if it
turns out to be a common complaint.
If a branch points to the working-copy commit, it will automatically
get updated when the working copy is snapshotted. However, it turns
out that we don't automatically export the change to Git when running
in a colocated working copy (nor in a non-colocated working copy, but
that shouldn't be surprising). The change will not get exported until
a non-snapshot operation runs. We noticed this bug in @hooper's repo
today.
Unfortunately, config::Value is lax and '[7]' could be parsed as '["7"]'.
I don't like it, but I think that's actually better for consistency as we
use config.get_string() in various places.
When a workspace's working-copy commit is updated from another
workspace, the workspace becomes "stale". That means that the working
copy on disk doesn't represent the commit that the repo's view says it
should. In this state, we currently automatically it to the desired
commit next time the user runs any command in the workspace. That can
be undesirable e.g. if the user had a slow build or test run started
in the working copy. It can also be surprising that a checkout happens
when the user ran a seemingly readonly command like `jj status`.
This patch makes most commands instead error out if the working copy
is stale, and adds a `jj workspace update-stale` to update it. The
user can still run commands with `--no-commit-working-copy` in this
state (doing e.g. `jj --no-commit-working-copy rebase -r @ -d @--` is
another way of getting into the stale-working-copy state, by the way).
Also allows several paths to be specified. By default, `jj resolve`
will find the first conflict that matches provided paths (if any)
and try to resolve it.
It seems like I forgot to update the `jj status` output when I decided
(years ago?) that the changes in a commit should always be compared to
the auto-merged parents. I was very confused before I realized that
`jj status` was showing the diff summary against the first parent. I
suppose the fact that `jj status` lists only one parent should have
been a hint. Thanks to ilyagr@ for finding this odd behavior. This
patch fixes it by making the command list all parents, and changes the
diff summary to be against the auto-merged parents.
If this new option is not specified, we start with empty output
file and trust the merge tool did a complete merge no matter
what the file contains.
Includes tests.
This command uses an external merge tool to resolve conflicts
simple enough that they can be resolved with a 3-way merge.
This commit provides a very basic version of `jj resolve` that
is hardcoded to use vimdiff.
This also slightly changes the errors of the Diff Editor, so that
both the diff editor and `jj resolve can share an error type.
Because a unary negation node '~y' is more primitive than the corresponding
difference node 'x~y', '~y' is easier to deal with while rewriting the tree.
That's the main reason to add RevsetExpression::NotIn node.
As we have a NotIn node, it makes sense to add an operator for that. This
patch reuses '~' token, which I feel intuitive since the other set operators
looks like bitwise ops. Another option is '!'.
The unary '~' operator has the highest precedence among the set operators,
but they are lower than the ranges. This might be counter intuitive, but
useful because a prefix range ':x' can be negated without parens.
Maybe we can remove the redundant infix operator 'x ~ y', but it isn't
decided yet.
I can't see any reason the user would want to specify revisions
matching the empty string, so let's disallow it. I created a custom
type for revision arguments instead of repeating `value_parser =
NonEmptyStringValueParser::new()`.
