Open ilyagr opened 1 year ago
My feeling for merge-tools.<tool>.<op>-args = { command = ..., env = ... }
is it's too structured. Implementation-wise, it's correct, but I prefer a flattened structure for human interface.
Somewhat related:
If we add a feature like GIT_EXTERNAL_DIFF
, we might want to specify a tool with jj diff/log --tool <name>
(as in --git
/--summary
). We can also add a readonly GUI/CUI diff command/option which will take -t/--tool <name>
option.
Should these tools be defined in the single merge-tools
namespace?
I think that all makes sense.
Regarding item 5: Since the arguments usually have to be specified anyway, it doesn't seem like much more work to specify the name of the binary, even if it's often the same as the tool name, like diff-tools.meld.command = ["meld", "$left", "$right"]
or diff-tools.meld.command = "meld $left $right"
.
My feeling for
merge-tools.<tool>.<op>-args = { command = ..., env = ... }
is it's too structured. Implementation-wise, it's correct, but I prefer a flattened structure for human interface.
I agree. I think that's what item 4 says, right? I.e. that we can allow more complex form but we can also allow a form that's easier to read and write for when that's good enough (which is most of the time).
If we add a feature like
GIT_EXTERNAL_DIFF
, we might want to specify a tool withjj diff/log --tool <name>
(as in--git
/--summary
). We can also add a readonly GUI/CUI diff command/option which will take-t/--tool <name>
option.
I think we can reuse the [diff-tools]
for that. Do you mean that there might be a need to treat "diff viewers" and "diff editors" differently?
My feeling for merge-tools.
. -args = { command = ..., env = ... } is it's too structured. Implementation-wise, it's correct, but I prefer a flattened structure for human interface.
Yeah, this is one of the problems I wanted to solve. Instead I want configs to look as follows:
ui.diff-editor = ["cmd", "tool"]
ui.merge-editor = { command = "...", env = "..."}
# ui.diff-editor = "TOOL" is also valid
[tools.merge.TOOL]
command = "..."
args = ["..."] # This might look different if we implement my point number 5.
env = ["..."]
[tools.diff.TOOL]
command = "..."
args = ["..."]
env = ["..."]
# The following could also be valid, maybe
[tools.diff]
ANOTHER_TOOL = ["sh", "-c", "script", "$left", "$right"]
This could potentially support different kinds of tools as well.
Oh, I though diff-tools.TOOL
in the item 2 were typo of merge-tools.TOOL
.
Mercurial started with separate diff-tools/merge-tools sections, and they've got merged later. I don't know the exact reason, but that might be related to tool lookup on Windows. Mercurial supports registry key to resolve full executable path, and such configuration can be shared within merge/diff tools.
Regarding item 5: Since the arguments usually have to be specified anyway, it doesn't seem like much more work to specify the name of the binary,
I think it's not uncommon on Windows to override only the executable path.
I think it's not uncommon on Windows to override only the executable path.
Good point, that's good to remember.
Registry paths are also an interesting point, though to me it doesn't seem prohibitive to simply repeat them. I doubt end users often use them, so it's just a matter of the default config.
Registry paths are also an interesting point, though to me it doesn't seem prohibitive to simply repeat them. I doubt end users often use them, so it's just a matter of the default config.
True. It's not meant for user to configure.
I think we can reuse the [diff-tools] for that. Do you mean that there might be a need to treat "diff viewers" and "diff editors" differently?
A diff tool of GIT_EXTERNAL_DIFF
type can be streamed (so log -p -t <tool>
should work), but vimdiff
for example can't. I'm not sure if they should be defined separately, or flagged like .type = stream|gui|cui
.
Regarding "diff viewer" vs "diff editor", command arguments can be different. I wouldn't pass kdiff3 --merge
for viewing.
Continuing a discussion with @martinvonz and @yuja from ... (couldn't find where), it might be good to redesign the configuration for merge and diff editors along the following lines:
ui.merge-editor
can be either a single word like "TOOL", or a value that would be valid formerge-tools.TOOL
.ui.diff-editor
can be either a single word like "TOOL", or a value that would be valid fordiff-tools.TOOL
. We might also allow it to be a command that can be executed ascommand $left $right
.vim
to execute a scriptvimdirdiff
(orsh -c SOMETHING
) while the merge toolvim
executesvimdiff
.ui.merge-editor
orui.diff-editor
can take is a TOML table, an array["command", "arg1", "arg2"]
, maybe a space-separated string.args
key that works likemerge-args
anddiff-args
work now, as well as acommand
key that always includes the command. It would then be illegal to specify both.Part of the motivation is that I expect the config for diff and merge tools to become more complex with time. For example, we might want to support diff tools that cannot compare dirs but only compare two files. There may be multiple way to call such tools (some might support opening many tabs with separate diffs, some might not) This seems to make the overlap between a merge tool and a diff tool configs confusing as well as the difference between what can be specified where.
WDYT?