cli: add template-aliases.'should_sign_off()' to configure sign offs

[thoughtpolice]

Many of the desired features of the default commit template can be solved with a simple pattern:

1. Write a template-aliases function should_do_something() = 'false' to your global config

2. Inside template-aliases.draft_commit_message, use the following:

   if(should_do_something() && !description.contains("..."), ...)`
   

3. Set should_do_something() = 'true' in repos where you want it enabled.

This patch uses this idea to configure a new, built-in signed_off_by() template alias, which defaults to false. With this, a user only has to configure this to true in order to get Signed-off-by lines included in their repositories.

While Signed-off-by: is a git mechanism, it is a common one and this makes it easier to use jj OOTB with repositories that might require those, without adding any more real surface area to the Rust codebase. This general idea can be configured by the user though, e.g. for BUG=123 templates, or Gerrit Change-Ids, or other various things.

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

[yuja]

I'm not sure if I like this pattern to be supported formally. It looks like a bit of hack. Another option is to add config(name).to_<type>() function, but they are technically not so different in that random global data is referred to.

[yuja]

nits:

• missing newline?
• just author should work (name <email> is the default output)
• maybe it should be committer?

[yuja]

nit: probably better to pass self explicitly to should_sign_off(self)

[thoughtpolice]

Yes, I wanted some opinions on what people thought of this, and it was only like 5 lines, so I figured submitting a patch would be the best way to kick things off. :) /cc @martinvonz @ilyagr @arxanas

One reason I was thinking about this is because I used it to add Change-Id items to my commit descriptions in my config recently, and I was thinking that this could be a piece of code that could go in without needing the rest of #2845. In fact, now that we have draft_commit_description, I don't think there's much reason at all — for now — to think about programmatic footers or anything like that. We could at least drop a reasonable amount of code that way. We don't have to talk about Gerrit right now, but I figured this was an easier starting point for a discussion and part of the motivation.

[thoughtpolice]

I also admit I don't like using [template-aliases] for what is effectively setting a boolean, i.e. configuration. Maybe if there was a formatter function config_option_set(name: String) -> Bool we could do something like if (config_option_set("ui.signed-off-by"), ...) and use that?

[arxanas]

I'm not sure if I like this pattern to be supported formally. It looks like a bit of hack. Another option is to add config(name).to_<type>() function, but they are technically not so different in that random global data is referred to.

Don't we already support several formal pieces of global data/configuration? Namely trunk() and immutable_heads().

• Currently: I don't have an issue with the approach that we're already using. Granted, those are revset functions and not templating functions, but I believe the same philosophy applies.
• I'm not opposed to adding more "magic" global configuration by itself, but we could consider namespacing it for intelligibility.
• In the future: The "global configuration" system might improve the quality of certain workflows.
  • If the repository can set some of that configuration itself, then new users don't have to configure things like the main branch, commit message template, etc.
  • Therefore: It might be a good abstraction to keep when considering future extensibility.

I also admit I don't like using [template-aliases] for what is effectively setting a boolean, i.e. configuration. Maybe if there was a formatter function config_option_set(name: String) -> Bool we could do something like if (config_option_set("ui.signed-off-by"), ...) and use that?

As an alternative level of abstraction, we could use "trailers" (as per Git), and have some kind of git.trailers global configuration data structure, and you add to that whenever you want to add a new trailer.

• To implement the desired feature, the default template would just use some simple list formatting and joining.
• Then it works for Change-Id, Signed-off-by, Co-authored-by:, and any other bespoke workflows.
• I guess maybe it doesn't work for repeated keys.
  • I think TOML doesn't support repeating keys, but Git trailers do?
  • Alternative: Have each table entry be of the form { key = "Signed-off-by", value = "Me" }, etc. If using a table, then the table key would be ignored if the key property is explicitly set.
  • Alternative: Use TOML arrays instead of TOML tables.
• I don't know if the interaction becomes complicated with the existing jj interfaces when you try to add elements to the table from multiple places. (Can we do that via --config today?)

for BUG=123 templates

To handle this workflow:

• We could reuse the above infrastructure but perhaps somehow configure separator = "=" instead of the default separator = ": ".
• We could use a different system altogether (not git.trailers or whatever).
• We could elect not to support it directly in the default configuration. Even if we don't represent it directly, it should be straightforward for a user to replicate the above trailer approach for a different purpose on their own.

[yuja]

Don't we already support several formal pieces of global data/configuration? Namely trunk() and immutable_heads().

Yes. They're technically the same. I just had a feeling that using aliases as boolean flags is a bit hackish.
if(config(name).to_bool(), ...) would look like more proper way, but there aren't much difference.

To be clear, I'm not against this PR nor adding config() template function.

[yuja]

if(config(name).to_bool(), ...)

#5328