The idea is simple. New heads are ignored until the node dependency resolution
stuck. Then, only the first head that will unblock the visit will be queued.
Closes#242
The original idea was similar to Mercurial's "topo" sorting, but it was bad
at handling merge-heavy history. In order to render merges of topic branches
nicely, we need to prioritize branches at merge point, not at fork point.
OTOH, we do also want to place unmerged branches as close to the fork point
as possible. This commit implements the former requirement, and the latter
will be addressed by the next commit.
I think this is similar to Git's sorting logic described in the following blog
post. In our case, the in-degree walk can be dumb since topological order is
guaranteed by the index. We keep HashSet<CommitId> instead of an in-degree
integer value, which will be used in the next commit to resolve new heads as
late as possible.
https://github.blog/2022-08-30-gits-database-internals-ii-commit-history-queries/#topological-sorting
Compared to Sapling's beautify_graph(), this is lazy, and can roughly preserve
the index (or chronological) order. I tried beautify_graph() with prioritizing
the @ commit, but the result seemed too aggressively reordered. Perhaps, for
more complex history, beautify_graph() would produce a better result. For my
wip branches (~30 branches, a couple of commits per branch), this works pretty
well.
#242
Since we abuse TOML table syntax to define function aliases, an identical
function alias can be found more than once in the merged config. The merged
config doesn't preserve the definition order, so we need to load aliases
table per layer.
This involves a little hack to insert a lambda parameter 'x' to be used at
keyword position. If the template language were dynamically typed (and were
interpreted), .map() implementation would be simpler. I considered that, but
interpreter version has its own warts (late error reporting, uneasy to cache
static object, etc.), and I don't think the current template engine is
complex enough to rewrite from scratch.
.map() returns template, which can't be join()-ed. This will be fixed later.
A lambda expression will be allowed only in .map() operation. The syntax is
borrowed from Rust closure.
In Mercurial, a map operation is implemented by context substitution. For
example, 'parents % "{node}"' prints parents[i].node for each. There are two
major problems: 1. the top-level context cannot be referred from the inner map
expression. 2. context of different types inserts arbitrarily-named keywords
(e.g. a dict type inserts "{key}" and "{value}", but how we could know.)
These issues should be avoided by using explicitly named parameters.
parents.map(|parent| parent.commit_id ++ " " ++ commit_id)
^^^^^^^^^ global keyword
A downside is that we can't reuse template fragment in map expression. Suppose
we have -T commit_summary, -T 'parents.map(commit_summary)' doesn't work.
# only usable as a top-level template
'commit_summary' = 'commit_id.short() ++ " " ++ description.first_line()'
Another problem is that a lambda expression might be confused with an alias
function.
# .map(f) doesn't work, but .map(g) does
'f(x)' = 'x'
'g' = '|x| x'
I'm going to add a lambda expression, and the current type-error message
wouldn't work for the lambda type. I also renamed "argument" to "expression"
as the expect_<type>() helper may be called against any expression node.
This is similar to the structure of RevsetParseError. It's unlikely we would
need to discriminate parsing errors, so let's avoid wasting time on naming
things.
@joyously found `o` confusing because it's a valid change id prefix. I
don't have much preference, but `●` seems fine. The "ascii",
"ascii-large", and "legacy" graph styles still use "o".
I didn't change `@` since it seems useful to have that match the
symbol used on the CLI. I don't think we want to have users do
something like `jj co ◎-`.
The parameter order follows indent()/label() functions, but this might be
a bad idea because fill() is more likely to have optional parameters. We can
instead add template.fill(width) method as well as .indent(prefix). If we take
this approach, we'll probably need to add string.fill()/indent() methods,
and/or implicit cast at method resolution. The good thing about the method
syntax is that we can add string.refill(), etc. for free, without inventing
generic labeled template functions.
For #1043, I think it's better to add a config like ui.log-word-wrap = true.
We could add term_width/graph_width keywords to the templater, but the
implementation would be more complicated, and is difficult to use for the
basic use case. Unlike Mercurial, our templater doesn't have a context map
to override the graph_width stub.
The argument order is different from Mercurial's indent() function. I think
indent(prefix, content) is more readable for lengthy content. However,
indent(content, prefix, ...) might be better if we want to add an optional
firstline_prefix argument.
This eliminates ambiguous parsing between "func()" and "expr ()".
I chose "++" as template concatenation operator in case we want to add
bit-wise negate operator. It's also easier to find/replace than "~".
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.
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.
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.
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().
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.
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
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.
Let's acknowledge everyone's contributions by replacing "Google LLC"
in the copyright header by "The Jujutsu Authors". If I understand
correctly, it won't have any legal effect, but maybe it still helps
reduce concerns from contributors (though I haven't heard any
concerns).
Google employees can read about Google's policy at
go/releasing/contributions#copyright.
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463). It also lets us remove a `jj export` call
from `test_templater.rs`.
It's useful to know when you've modified a branch that exists on a
remote. A typical case is when you have pushed a branch to a remote
and then rewritten it. This commit adds an indication in the
`branches` template keyword. A branch that needs to be pushed to a
remote now has a `*` at the end (similar to how conflicted branches
have a `?` at the end). Note that the indication only considers
remotes where the branch currently exists, so there won't be an
indication that the branch has not been pushed to a remote.
Closes#254
