Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I4e7e5f6cd3c1afa0b42ee88725bc89d3bfafdc0a

Yuya added `coalesce()` as a built-in function with variable arity in
a2a9b7decb586, and we want to use it with more-than-two arguments in the default
`log_node` configuration in order to provide better graph node symbols for
users.

However, `test_templater` already defined this name as an alias, but with
only two parameters; this alias now conflicts with the built-in definition and
overrides it, causing the test to fail catastrophically because now the default
configuration is considered invalid.

Simply renaming it fixes this without invalidating the actual test case.

Signed-off-by: Austin Seipp <aseipp@pobox.com>
Change-Id: I18593f64b9168fc334001c2ff5d49ad8d81c006d

It's not special compared to the other argument errors, and we can now track
the error source separately.

This makes the summary line more informative. Even though it just duplicates
the message printed later, I think it's easier to follow.

This patch also adjusts some RevsetParseError messages because it seemed
redundant to repeat "revset function", "argument", etc.

This patch moves all "source" errors to the source field to conform to
thiserror API. It will probably help to keep ErrorKind enums comparable.

Because the CLI error handler now prints error sources in multi-line format,
it doesn't make much sense to render Revset/TemplateParseError differently.

This patch also fixes the source() of the SyntaxError kind. It should be
self.pest_error.source() (= None), not self.pest_error.

I'm going to make TemplateParseError hold RevsetParseError as Box<dyn _>, but
Box<dyn std::error::Error ..> doesn't implement Eq. I could remove Eq from
ErrorKind enums, but it's handly if these enums remain as value types.

This change will also simplify fmt::Display and error::Error impls.

Since all callers of RevsetParseError have some reasonable span, we don't
need a special case for WorkingCopyWithoutWorkspace error.

I think it's more like data corruption, which is usually reported as an
internal error.

I don't think it needs to convert absolute file paths to workspace paths.

It's common to normalize an empty directory path as ".". This change unblocks
the use of from_relative_path() in edit_sparse().

There are a couple of callers who do to_fs_path(Path::new("")), but they all
translate non-directory paths, which should never be empty.

This can be used to flatten nested "if()"s. It's not exactly the same as "case"
or "switch" expression, but works reasonably well in template. It's not uncommon
to show placeholder text in place of an empty content, and a nullish value
(e.g. empty string, list, option) is usually rendered as an empty text.

Bumps the cargo-dependencies group with 1 update: [chrono](https://github.com/chronotope/chrono).


Updates `chrono` from 0.4.35 to 0.4.37
- [Release notes](https://github.com/chronotope/chrono/releases)
- [Changelog](https://github.com/chronotope/chrono/blob/main/CHANGELOG.md)
- [Commits](https://github.com/chronotope/chrono/compare/v0.4.35...v0.4.37)

---
updated-dependencies:
- dependency-name: chrono
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: cargo-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>

I think a colorized commit summary is easier to follow.

A formatted error is not a string containing ANSI escape sequences because 1.
the output may be differently colored inside "hint", 2. the caller might not
be accessible to ui.new_formatter().

We currently include the commits in `parent_mapping` and `abandoned`
in the set of commits to visit when rebasing descendants. The reason
was that we used to update branches and working copies when we visited
these commits. Since we started updating refs after rebasing all
commits, there's no need to even visit these commits.

Similar to the previous two commits.

Similar to the previous commit.

We only use `new_commits` in `update_heads()`, so let's calculate it
there. It should also be more correct in case other commits were
created after we initialized `DescendantRebaser`.

Now that we only call `update_references()` in one place, there's no
reason to have it also update `heads_to_add` and `heads_to_remove`. By
moving it out of the function, we can consolidate the logic in one
place.

The function only uses state from `MutableRepo`, so it should be
implemented on that type.

When `rebase_commit_with_options()` decides to abandons a commit, it
records the new parents in the `MutableRepo`, but it's currently the
caller's responsibility to remember to mark it as abandoned. Let's
move that logic into the function to reduce the risk of future bugs.

By adding the abandoned commit's parents to `parent_mapping`, we can
remove a bit more of the special handling of abandoned commitsin
`DescendantRebaser`.

Bumps the cargo-dependencies group with 3 updates: [clap](https://github.com/clap-rs/clap), [insta](https://github.com/mitsuhiko/insta) and [serde_json](https://github.com/serde-rs/json).


Updates `clap` from 4.5.3 to 4.5.4
- [Release notes](https://github.com/clap-rs/clap/releases)
- [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md)
- [Commits](https://github.com/clap-rs/clap/compare/v4.5.3...v4.5.4)

Updates `insta` from 1.36.1 to 1.37.0
- [Changelog](https://github.com/mitsuhiko/insta/blob/master/CHANGELOG.md)
- [Commits](https://github.com/mitsuhiko/insta/compare/1.36.1...1.37.0)

Updates `serde_json` from 1.0.114 to 1.0.115
- [Release notes](https://github.com/serde-rs/json/releases)
- [Commits](https://github.com/serde-rs/json/compare/v1.0.114...v1.0.115)

---
updated-dependencies:
- dependency-name: clap
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: cargo-dependencies
- dependency-name: insta
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: cargo-dependencies
- dependency-name: serde_json
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: cargo-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>

Bumps the github-dependencies group with 1 update: [actions/setup-python](https://github.com/actions/setup-python).


Updates `actions/setup-python` from 5.0.0 to 5.1.0
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](https://github.com/actions/setup-python/compare/0a5c61591373683505ea898e09a3ea4f39ef2b9c...82c7e631bb3cdc910f68e0081d67478d79c6982d)

---
updated-dependencies:
- dependency-name: actions/setup-python
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: github-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>

Highlighting "{n}: " will help to follow error sources containing multi-line
messages. I'm going to make revset/template alias errors be formatted as plain
error chain.

Since we update all branches at the end now, we never update them in
several steps, so there are no intermediate locations we need to
remember.

In the normal case when we don't abandon a commit because it became
empty, then `CommitBuilder::write()` will have recorded the new commit
as a rewrite of the old commit. We don't need to do that again in
`rebase_one()`.

A subset of the state in `DescendantRebaser` now matches exactly what
`MutableRepo` already stores, so we can avoid copying that state and
have `DescendantRebaser` use it directly instead. Having a single
source of truth for the state will enable further simplifications and
improvements.

I'm going to make `DescendantRebaser` share the state about rewritten
commits with `MutableRepo` next. That means that the call to
`rebase_commit_with_options()` will update that state, which would
make this assertion fail. So let's move it a little earlier to avoid
that.

This is just to match `DescendantRebaser`, to make the next commit a
bit simpler. I think `MutableRepo` still has few enough fields that
just `abandoned` is clear enough. Maybe we'll move the three
rewrite-related fields into a new struct at some point.

With this patch, `MutableRepo` has the same tracking of rewritten
commits as `DescendantRebaser`, so we can simply pass that state into
`DescendantRebaser` when we create it. The next step is to remove the
state from `DescendantRebaser`.

I think long message is easier to read if printed in the default color. Errors
and warnings are printed in bold to make them distinct.

Since it doesn't terminate jj, it can be considered a warning.

It's inconsistent that some warnings have headings and some don't, and it seems
the choice is arbitrary. Let's unify the style. There are two exceptions:
1. continued line following labeled message,
2. "unrecognized response" followed by prompt.

Bumps the cargo-dependencies group with 3 updates: [async-trait](https://github.com/dtolnay/async-trait), [rayon](https://github.com/rayon-rs/rayon) and [syn](https://github.com/dtolnay/syn).


Updates `async-trait` from 0.1.78 to 0.1.79
- [Release notes](https://github.com/dtolnay/async-trait/releases)
- [Commits](https://github.com/dtolnay/async-trait/compare/0.1.78...0.1.79)

Updates `rayon` from 1.9.0 to 1.10.0
- [Changelog](https://github.com/rayon-rs/rayon/blob/main/RELEASES.md)
- [Commits](https://github.com/rayon-rs/rayon/compare/rayon-core-v1.9.0...rayon-core-v1.10.0)

Updates `syn` from 2.0.53 to 2.0.55
- [Release notes](https://github.com/dtolnay/syn/releases)
- [Commits](https://github.com/dtolnay/syn/compare/2.0.53...2.0.55)

---
updated-dependencies:
- dependency-name: async-trait
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: cargo-dependencies
- dependency-name: rayon
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: cargo-dependencies
- dependency-name: syn
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: cargo-dependencies
...

Signed-off-by: dependabot[bot] <support@github.com>

Since we've introduced Option type, it no longer makes sense that git_head
returns a Vec<RefName>.

The lowercase "warning: " is unified to "Warning: " as it is the jj's
convention afaik.

The _default() suffix could be dropped from these methods, but it's probably
better to break the existing codebase for the moment. Otherwise, the caller
might do writeln!(ui.warning(), "Warning: ..").

The existing .hint() method is renamed to .hint_no_heading() to clarify that
it's not the default choice to print a hint. I'll add .hint_default() later,
which will be the shorthand for .hint_with_heading("Hint: ").

This will be used to label "Error: " heading and content differently. I want
to see an error message in the default (white) color because it's easier to
read, but I still want to highlight the "Error: " heading.

We can achieve that without introducing new wrapper, but the resulting code
would look something like "writeln!(ui.error("Error: ")?, ..)?", and it would
get messier if the caller had to suppress io::Error.

I don't think we have any callers left that call
`record_rewritten_commit()` multiple times within a transaction and
expect it to result in divergence. I think we should consider it a bug
to do that.

All callers now pass a single new commit and I would like to keep it
that way.

I plan to remove `record_rewritten_commit()` and instead make repeated
rewrites replace the rewrite state.

When rebasing descendants, we generally move branches, child commits,
the working copy to the rewritten commit(s). However, we don't move
the working copy to the new rewritten commit (s) if the old commit had
been abandoned, and we don't move child commits if the rewriten was
divergent.

This patch aims to make it clearer that there's only one mapping from
old to new parents, and that is in `parent_mapping`. It does so by
merging the current `divergent` map into it, and makes the `divergent`
just a set instead. When finding the new parents for a child, we leave
the existing parent if it's in the set.

My longer-term goal is to move `parent_mapping`, `abandoned`, and
`divergent` into `MutableRepo` (maybe in a nested struct), so we can
do some transformations on descendants as we rebase them. By having
the state in a single place (not moving it from `MutableRepo` to
`DescendantRebaser` as we currently do), I hope it will be easier to
write a `MutableRepo::transform_descendants(callback)`, where the
callback gets a `CommitBuilder` and can change parents of the commit,
for example.

This is the same formatting as "jj abandon" output. It should improve the
discoverability of the trailing hint.

I'm going to introduce two changes: 1. indent commit summary, 2. colorize
output. The former can be implemented without using the templater API, but the
latter can't.

Since the context (or self) property is no longer passed by argument, it's easy
to implement short-circuiting behavior.

They are similar in a way that both of them can represent dynamic/static
evaluation, but their behaviors are different in error handling.

IntoTemplate will be cleaned up later. Perhaps, the lifetime parameter can be
removed at this point, but I'm planning to remove the IntoTemplate trait at all.