FR: Add builtin_immutable_heads()

[jennings]

Is your feature request related to a problem? Please describe.

When redefining immutable_heads(), there is no way to include the default set. Since the default set occasionally changes, the user has to change their config to get the new settings.

Describe the solution you'd like

Define the default set in builtin_immutable_heads() and redefine immutable_heads() like so:

[revset-aliases]
'builtin_immutable_heads()' = 'trunk() | tags() | untracked_remote_branches()'
'immutable_heads()' = 'builtin_immutable_heads()'

If a user overrides immutable_heads() and includes builtin_immutable_heads(), updates to jj's default will automatically be used.

If the user doesn't want to include the default set, they simply omit builtin_immutable_heads(). This means this change wouldn't adversely affect anyone until they opt-in by adding builtin_immutable_heads() to their config.

Describe alternatives you've considered

If we wanted the user to opt out of the default set rather than opting in, the definition could be like so:

[revset-aliases]
'immutable_heads()' = 'none()'

# This becomes the revset function that jj uses
'real_immutable_heads()' = 'immutable_heads() | trunk() | tags() | untracked_remote_branches()'

The user could still opt out by defining real_immutable_heads() instead.

However, I think this alternative is too opinionated, not backwards compatible, and harder to explain:

If you want to include the default set of immutable heads, define immutable_heads(). If you want to completely replace the set, use real_immutable_heads().

Versus:

Define immutable_heads() as desired. The default revset can be included as builtin_immutable_heads().

Additional context

This was discussed in another pull request with @yuja, @scott2000, and @ilyagr.

[essiene]

I'm taking a stab at this in #4195 and I have a few questions:

1. Is the right thing to do here to define builtin_immutable_heads() in code? I'm guessing it's the right thing since we don't want users to be able to overwrite it, on account of being "built-in" an all.

2. If we define it in code, what version of "trunk" should we use? trunk() is currently defined as an alias, so users can redefine it. However, immutable_heads is currently defined as trunk()|tags()|untracked_remote_branches().

   • If we use trunk() the alias, which can be redefined, then builtin_immutable_heads() can be indirectly redefined by redefining trunk.
   • If that's not good enough, then we need to defined trunk() as a built in. But that would mean taking it away from users. So should I define a builtin_trunk() as well?

Thoughts?

[martinvonz]

• If we use trunk() the alias, which can be redefined, then builtin_immutable_heads() can be indirectly redefined by redefining trunk.

That seems fine to me. I think the trunk() alias is typically overridden without keeping any part of the original (builtin) trunk() alias, so I don't think we need to split that up into a separate builtin_trunk() function. Makes sense?

[essiene]

• If we use trunk() the alias, which can be redefined, then builtin_immutable_heads() can be indirectly redefined by redefining trunk.

That seems fine to me. I think the trunk() alias is typically overridden without keeping any part of the original (builtin) trunk() alias, so I don't think we need to split that up into a separate builtin_trunk() function. Makes sense?

Yes. This makes sense. Let me see how I can make that work.

Thanks

[ilyagr]

This is slightly outside the scope of this FR, but I wonder whether we also want want builtin_revsets_log. Somebody on Discord seems to want to show some commits on the main branch in their default revset, and I think being able to set that up in terms of builtin_revsets_log should be good.

[yuja]

but I wonder whether we also want want builtin_revsets_log.

Possibly? but I suspect user log revset would often subtract revisions from the default.

[ErichDonGubler]

but I wonder whether we also want want builtin_revsets_log.

Possibly? but I suspect user log revset would often subtract revisions from the default.

I'm not sure that this is a valid argument against builtin_revsets_log if valid use cases exist and we want to support them. You're saying you think it's unlikely enough that we shouldn't?

[scott2000]

One other option might be to simplify the log revset so that it's easier to remember/understand. When I was first trying to configure the log, the ancestors(immutable_heads().., 2) part was confusing to me, so I'm wondering if we could simplify it like this:

[revsets]
log = "@ | trunk() | context(mutable())"

[revset-aliases]
"context(x)" = "ancestors(x, 2)"

I think this is easier to remember, since it reads more like a sentence ("I want to see @, trunk, and also all mutable commits with context").

[yuja]

I'm not sure that this is a valid argument against builtin_revsets_log if valid use cases exist and we want to support them.

If many users can't leverage it, builtin_* indirection would just add complexity.

I think context(x) is good idea if we can find a better name.