FR/RFC: allow edit/diff/merge-tools to have warnings or be explicitly disallowed

[ilyagr]

It is currently difficult to deprecate merge tools. I also wonder how many people run jj split --tool vscode and get a non-functional interface.

To address this, we could reorganize merge-tools.toml (current state) and merge tool config a bit; I was thinking about how to do it for a while. One possibility is to change edit-args to edit.args, and then add a few fields to allow the following:

[merge-tools.meld]
# Most user config would look like this, which should hopefully not feel
# any more difficult than the current status-quo.
merge.args = ["$left", "$base", "$right", "-o", "$output", "--auto-merge"]

[merge-tools.vscode]
program = "code"
diff = { error = true }
# `error = "message"` is also allowed. `error=true` gives a generic
# message, e.g. "The tool `vscode` cannot be used for diff editing".
edit = { error = true }
merge = {
  args = ["--wait", "--merge", "$left", "$right", "$base", "$output"]
  edits-conflict-markers = true   # This could also stay as `merge-tool-edits-conflict-markers` for ease of implementation
  warning = "This tool is deprecated and will soon be removed. Use `code` instead"
}

[merge-tools.code]
# ...

We could also rename the whole [merge-tools] table while we are at it, but I wouldn't do it unless we can come up with a great name. I don't like [tools] or [external-tools] since there are also a different category of tools for jj fix, unless we want those to be tools.NAME.fix (which might not make much sense, their config is very different).

---

See also the older #1285. This could be considered as a subset of that issue.

[yuja]

It is currently difficult to deprecate merge tools. I also wonder how many people run jj split --tool vscode and get a non-functional interface.

I don't know the vscode problem, but I think we can freely introduce breaking change if it's already broken.

Regarding the config structure, I tend to prefer somewhat flattened form. To disable "edit", maybe we can explicitly nullify edit-args = []? It will be overwritten by the user-configured edit-args if any, so it's a bit safer than adding separate edit.error field, which can disable user-configured tool.

Related
#1285

[ilyagr]

To disable "edit", maybe we can explicitly nullify edit-args = []?

I think I can do this first. IMO, edit-args = false would look clearer on first reading if we can implement it (I think I mentioned this somewhere before).

I think this would help a lot in the short term. However, it wouldn't help with (for example) creating a deprecation warning for vscode is we want to rename it to code. Custom error messages would also be helpful for this (after it's deprecated, have vscode report that it's called code now).

I tend to prefer somewhat flattened form.

I think it's worth putting in some effort so that the user does not have to create complicated nested structures just to make a simple config work. I'm not sure this is an advantage for built-in config. Also, I think there's some elegance in replacing edit-args with edit.args. But perhaps there are more problems I didn't think of.

it's a bit safer than adding separate edit.error field, which can disable user-configured tool.

Yeah, this is a bit annoying. We can allow overriding edit.error with edit.error = false, though this wouldn't be entirely obvious so we would have to document it. Update: We can also print an error if both args and error are specified. Then, it'll be self-documenting.

[yuja]

IMO, edit-args = false would look clearer on first reading

I prefer typed values, but that wouldn't be super important. (fwiw, merge-args is by default [], and it means the tool doesn't support merging.)

However, it wouldn't help with (for example) creating a deprecation warning for vscode is we want to rename it to code.

Correct. If we really need a per-tool deprecation warning/error message, we'll have to add a separate field for it. (It might have to be a separate table of strings if we add i18n support.)