fix: Add enabled config for fix tools

[jennings]

As inspired by @ilyagr: https://discord.com/channels/968932220549103686/969291218347524238/1276278608729608243

Adds an optional fix.tools.TOOL.enabled config that disables use of a fix tool (if omitted, the tool is enabled). This is useful for defining tools in the user's configuration without enabling them for all repositories:

# ~/.jjconfig.toml
[fix.tools.rustfmt]
enabled = false
command = ["rustfmt", "--emit", "stdout"]
patterns = ["glob:'**/*.rs'"]

Then to enable it in a repository:

$ jj config set --repo fix.tools.rustfmt.enabled true

This also allows Jujutsu to ship some built-in tool configurations that are disabled by default.

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• I have updated the config schema (cli/src/config-schema.json)
• I have added tests to cover my changes

[martinvonz]

Sounds good. Mercurial has a similar feature. @hooper might want to review this

[yuja]

This also allows Jujutsu to ship some built-in tool configurations that are disabled by default.

Since enabled is optional field, adding builtin config with enabled = false can break user-defined tool of the same name. We have to be careful.

[jennings]

Since enabled is optional field, adding builtin config with enabled = false can break user-defined tool of the same name. We have to be careful.

Oh, that's a very good point, I didn't think of that possibility.

[ilyagr]

Since enabled is optional field, adding builtin config with enabled = false can break user-defined tool of the same name. We have to be careful.

Oh, that's a very good point, I didn't think of that possibility.

Me neither. Very good point!

The best solution I came up with so far would be to split fix.tools into two, let's call them fix.default-tools and fix.tools. We could encourage users to only add tools to the latter. In fact (even better?), we could make enabled default to true in fix.tools (or we could not even allow enabled in there) and have it default to false in fix.default-tools. Then, people could set up fix.default-tools in their user config and only enable those tools in some repos.

We could, of course, also default enabled to false in general, but the only way I can think of making this not too annoying is if we encourage people to use jj fix as jj fix --tool rustfmt --tool another_tool (so only a few tools are enabled by default). I feel like we probably want to encourage the opposite, though: have the repo set up all the fixes (in a not yet implemented and TBD fashion) so that the user doesn't have to think about it.

P.S. Thanks for working on this, Stephen!

[yuja]

This might be a rejected idea, but we can add a list of tool names to be used, and the default list will be constructed from fix.tools of config_source != Default.

Oh, the default fix.tools.<name>.enabled could be derived from config_source != Default, but that might be too cryptic?

[ilyagr]

Oh, the default fix.tools..enabled could be derived from config_source != Default, but that might be too cryptic?

I didn't know that was an option. If we stick with Stephen's current approach, we could use config_source to print a warning if it looks like our enabled=false overrides the user's config. (That is, if the user-level config for the tool is different from the Default-level, but enabled is false)

Though, the approach I suggested above would also help the user if they want to set up default tools in their user config without accidentally disabling a tool config in their repo config.

[jennings]

Maybe the simplest solution would be for any built-in fix tools to be named builtin-*: builtin-prettier, builtin-rustfmt, etc.? Then it's reasonable to assume we wouldn't conflict with anything the user configured themselves.

If we'd rather do the config_source version, maybe we can add that when we're ready to add built-in tools?

[yuja]

Maybe the simplest solution would be for any built-in fix tools to be named builtin-*: builtin-prettier, builtin-rustfmt, etc.? Then it's reasonable to assume we wouldn't conflict with anything the user configured themselves.

Yes. It's not fancy, but works.

@hooper any comments?

I'm also interested in whether or not something like the following was rejected. It will solve the ordering issue, but it's not fancy either.

we can add a list of tool names to be used, and the default list will be constructed from fix.tools of config_source != Default.

[jennings]

I'm also interested in whether or not something like the following was rejected. It will solve the ordering issue, but it's not fancy either.

we can add a list of tool names to be used, and the default list will be constructed from fix.tools of config_source != Default.

I think it’s a good idea, but was hoping to defer the work when/if we actually want to ship built-in tools.

[yuja]

was hoping to defer the work when/if we actually want to ship built-in tools.

That's reasonable, but the list (of enabled tool names) will replace <tool>.enabled flag, so I'm asking.