Anytime an external tool is referenced in the config, the command can be
provided as a string or as a token array. In the latter case, the array
must not be empty; at least the command name must be provided.
The schema didn't previously object to an empty array, though; this has
now been rectified. I've added more sample configs to cover this case.
Those same configs can also be used to illustrate that this is indeed
jj's current behavior:
$ jj --config-file cli/tests/sample-configs/invalid/ui.pager_empty_array.toml show
Config error: Invalid type or value for ui.pager
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.pager.command_empty_array.toml show
Config error: Invalid type or value for ui.pager
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.editor_empty_array.toml config edit --user
Config error: Invalid type or value for ui.editor
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.diff-editor_empty_array.toml split
Error: Failed to load tool configuration
Caused by:
1: Invalid type or value for ui.diff-editor
2: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.merge-editor_empty_array.toml resolve
Error: Failed to load tool configuration
Caused by:
1: Invalid type or value for ui.merge-editor
2: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/ui.diff.tool_empty_array.toml diff
Config error: Invalid type or value for ui.diff.tool
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
$ jj --config-file cli/tests/sample-configs/invalid/fix.tools.command_empty_array.toml fix
Config error: Invalid type or value for fix.tools.black
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
in `command`
As a notable exception, `ui.default-command` *is* allowed to be an empty
array. In that case, `jj` will print a usage message. This is also
covered by a valid sample config.
While `ui.pager` can be a string which will be tokenized on whitespace,
and argument token array, or a command/env table, the `command` key
within that table currently must be an array. The schema previously
explicitly also allowed it to be a string but that does not actually
work, as exemplified by running:
```sh
$ jj --config-file cli/tests/sample-configs/invalid/ui.pager_command-env_string.toml config list
Config error: Invalid type or value for ui.pager
Caused by: data did not match any variant of untagged enum CommandNameAndArgs
```
`CommandNameAndArgs` should potentially be changed to allow strings.
For now, the schema has been updated to reflect the status quo. A new
sample toml has been added to the `invalid` directory to cover this;
prior to updating the schema, this new test case failed. Once the
behavior is changed to allow string, the file merely needs to be moved
to `valid`.
These are two more instances where the default values were wrong and in
fact not even consistent with the schema itself.
I've found these by running
```sh
jq -r 'paths(type == "object" and has("default")) as $p | getpath($p).default | tojson as $v | $p | map("\"\(select(. != "properties"))\"") | join(".") as $k | "\($k) = \($v)"' cli/src/config-schema.json | taplo check --schema=file://$PWD/cli/src/config-schema.json -
```
which uses `jq` to filter the default values from the schema definition
to create a rudimentary TOML file containing all the defaults according
to the schema and then uses `taplo` the validate this TOML against the
schema.
This approach could be developed further to also parse the intermediate
TOML file and compare the result with the default config (from parsing
an empty config). That would not only test for self-consistency of the
schema's proclaimed defaults but also for consistency with the actual
defaults as assumed by jj.
Since we now have the template alias, it's easy to migrate the config value
programatically. However, there are a couple of minor behavior changes:
a. default description won't be used if user customized the
draft_commit_description template.
b. default description won't be inserted if trailer is added
For (a), I assumed user would inline the default description in the draft
template if they customized the template. (b) should be okay because the
trailers template is a new feature.
I don't have any plan to implement incremental UI for file annotation, but I
think the new API is nicer in that they have fewer function arguments.
Note that this wouldn't help implement interactive UI to go ancestor annotation
by clicking annotated line. To achieve that cheaply, we'll need weave-like data.
This makes it easier to override just the default description without
having copy the whole default template (and having to keep it up to
date with new versions).
An existing "push-*" bookmark should usually be in position. If it wasn't
because of e.g split, I think the user should be aware of that and take an
explicit action.
namely Signed-off-by and Change-Id
`format_signed_off_by_trailer` will be formatted properly if the author
name is not set, but will contain the email placeholder if the author
email is not set, as I haven't found a way to make the template
generation fail.
`format_gerrit_change_id_trailer` is based on jj's change id, but it
needed to be padded to reach 40 characters. Zero-padding is kind of
boring so I've used `6a6a6964`, the hexadecimal representation of `jjid`
in ascii.
Because the trailer value runs up to the end of the line, they are
all terminated with a new line. This way it's also convenient to
define these trailers in the `commit_trailers` template:
[templates]
commit_trailers = '''
format_signed_off_by_trailer(self)
++ format_gerrit_change_id_trailer(self)
'''
This allows the customization of the duplicated commit descriptions.
An ideal use case for this is emulating `git cherry-pick -x`, as
illustrated in the tests.
Add a new `template.commit_trailer` configuration option. This template
is used to add some trailers to the commit description.
A new trailer paragraph is created if no trailer paragraph is found in
the commit description.
The trailer is not added to the trailer paragraph when the trailer is
already present, or if the commit description is empty.
As Martin suggested, "we should instead make it an error to try to resolve the
content-level conflict before resolving the executable-bit-level conflict. I
think we should do the same when merging copy records."
https://github.com/jj-vcs/jj/pull/6288#pullrequestreview-2751367755
We're going to make "jj resolve" error out on exec bit conflict. This behavior
should be more consistent with tree::try_resolve_file_conflict(), which exits
early if the tree value had absent entries.
Since unchanged exec bit shouldn't be lost when resolving change-delete content
conflict, the resolution function falls back to the original state if the
exec-bit conflict is resolved to None.
try_materialize_file_conflict_value() and to_file_merge() shouldn't fail because
of exec bit changes. And error.to_string() shouldn't include trailing newline.
This helps propagate error from tx.write(). I made the error variants
non-transparent because it's easier than manually implementing From<> that maps
each inner error to the other inner error.
When read/writing commits from the git-backend, populate the git commit
header with a backwards hash of the `change-id`. This should enable
preserving change identity across various git remotes assuming a
cooperative git server that doesn't strip the git header.
This feature is behind a `git.write-change-id-header` configuration flag
at least to start.
The original idea was to flatten left/right conflict trees and pair up adjacent
negative/positive terms. For example, diff(A, B-C+D) could be rendered as
diff(A, B) and diff(C, D). The problem of this formalization is that one of the
diff pairs is often empty (because e.g. A=B), so the context is fully omitted.
The resulting diff(C, D) doesn't provide any notion why the hunk is conflicted,
and how it is different from A.
Instead, this patch implements diffs in which each left/right pair is compared.
In the example above, the left terms are padded, and the diffs are rendered as
diff(A, B), diff(-A, -C), diff(A, D). This appears to be working reasonably well
so long as either side is resolved or both sides have the same numbers of terms.
Closes#4062
This helps extract hunk rendering function for non-materialized color-words
diffs. In conflict hunk, an identical diff pair will be omitted with "..."
marker.
This patch introduces a subtle behavior change when "ignore whitespace" options
are used. Before, "..." wouldn't be printed if contents differ only in
whitespace. I don't think the new behavior is bad because the file header says
"Modified regular file" in that case.
I originally thought we could remove args.reset_author tests in later pass, but
we can't because author timestamp may be updated if a commit is discardable.
Still it's nice that we can deduplicate some logic.
