By separating the value spaces change ids and commit ids, we can
simplify lookup of a prefix. For example, if we know that a prefix is
for a change id, we don't have to try to find matching commit ids. I
think it might also help new users more quickly understand that change
ids are not commit ids.
This commit is a step towards that separation. It allows resolving
change ids by using hex digits from the back of the alphabet instead
of 0-f, so 'z'='0', 'y'='1', etc, and 'k'='f'. Thanks to @ilyagr for
the idea. The regular hex digits are still allowed.
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 is basically a copy of revset::RevsetAliasesMap. We could extract a
common table struct, but that wouldn't be worth the effort since the core
alias substitution logic can't be easily abstracted.
I'll add an alias table there. Since this function borrows self, it can't
always be used in between mutable operations. For log-like commands, this
should just work fine.
This prepares for template aliases support #1190. Unlike revset, template
expressions can be of various types, whereas alias substitution will process
untyped nodes. That's one reason that ExpressionNode is closer to parsed tree
than evaluatable Property structs. Another reason is that it's uneasy to split
name/type checking into "parsing" and "building property function" stages.
We could do alias expansion at once while building Property functions, but
that would make testing harder because Property isn't Debug + PartialEq.
I'm going to split 'parse() -> Expression' functions into 'parse() -> AST'
and 'build(AST) -> Expression'. The duplicated functions will be baseline of
new 'parse() -> AST' functions.
Without a pager configured (in `ui.pager` or `$PAGER`), running
`LC_CTYPE=foo jj log` would replace the Unicode characters in our
(Sapling's) "curved" graph style by escapes, like this:
```
@ 1d4ae2372dd2 martinvonz@google.com 2023-02-11 14:56:00.000 -08:00 a8eac1f9efe8
<E2><94><82> (no description set)
```
This fixes that by including the `LESSCHARSET=utf-8` environment
variable in our default `ui.pager` value.
We would like our default pager of `less -FRX` to also get passed
`LESSCHARSET=utf-8`. I don't like having defaults that the user can't
specify themselves, so this commits provides users with a way to pass
environment variables to their configured pager (or editor, or merge
tool, ...).
I didn't document this feature and I didn't add it to the config
schema because it seems it's going to be so rarely useful to users.
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.
This partially reverts the change in d7f64c07e0 "cli: handle last-minute
ui.write() error." In this commit, I tried to handle the case when the pager
process goes through env/sh, and exits immediately with "command not found".
However, I missed EPIPE could also occur when the pager was closed by user.
Apparently, hg is killed by SIGPIPE in that case (exits with 141), and chg
exits with 255. With this change, jj will silently exits with 3.
This works if the pager exits instantly and jj is slow enough to notice
EPIPE. If the pager exits late, no error would be reported.
Since the pager process is asynchronous, EPIPE could occur in
handle_command_result(). That's why I made it not panic.
There's a subtle difference between
- 'expression = { whitespace* ... whitespace* }', and
- '_{ whitespace* ~ expression ~ whitespace* }'.
The former includes surrounding whitespace in an "expression", the latter
doesn't. This affects the span of error indication.
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.
Our internal backend at Google uses a 32-byte change id, so I'd like
to make the backend able to decide the length. To start with, let's
make the backend able to decide what the root change id should
be. That's consistent with how we already let the backend decide what
the root commit id should be.
The function is currently only about the length of commit IDs, so
let's clarify that. I'm going to add another function for the length
of change IDs next. I don't know if we're going to care about lengths
of other hashes in the future. We might even be able to remove the
current restriction that all commit IDs and all change IDs have the
same length.
The assumption here is temp_dir wouldn't contain invalid utf-8 bytes. If it
can contain invalid bytes, maybe we can remove temp_dir from arguments, and
chdir(temp_dir) instead.
This unblocks the use of Regex. We could use regex::bytes, but it's way
more complex as we would have to go back and forth between str/OsStr and
bytes.
The default edit_args will be changed to ["$left", "$right"] to support
variable substitution without breaking the existing configuration too much.
The default merge_args could also be set if we could come up with something
meaningful.
