FR: Configuration setting to use --skip-empty by default when rebasing.

[afgomez]

Hi! First of all, thank you for building jj :)

tl;dr: As a user, I'd like a configuration setting to always execute jj rebase with the --skip-empty flag.

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

My team uses merge --squash to integrate the changes in the main brach. I often do stacked PRs. For example I would have a branch_1 whose PR uses main as a base, and a branch_2 whose PR uses branch_1 as a base.

C branch_2
|
B branch_1
|
A main

When the PR for branch_1 gets approved, the merge --squash creates a new commit D on top of main with the changes of branch_1. When rebasing the branch_2 on top of main, the default behaviour of jj rebase will create an empty commit.

The empty commit can of course be avoided with jj rebase --skip-empty, but I do this kind of rebasing often enough that I almost always add the flag.

Describe the solution you'd like

I'd like configure jj rebase to add --skip-empty by default. The command will need a new --no-skip-empty flag to use the original behaviour when needed.

When executing the rebase, it would also be useful to output which commits have been skipped when running the command. (I don't recall if this is the case already, but this can be a separate issue).

Describe alternatives you've considered

I can do this as an alias (reskip or something) but I would prefer to have this as a config flag. git does this for several flags (there are settings for always running git pull --rebase or git rebase --update-refs). I find it more user friendly.

I think --skip-empty could also be the default behaviour, but I don't have any arguments for this. But regardless of the default, a config flag to allow the user to change it still has a use.

[ilyagr]

Cc to @matts1, at least to let him know you found the feature he recently implemented useful.

I have a minor worry: it might be confusing if rebase behaves differently from all the automatic rebasing jj does, if you don't explicitly specify the flag and forget it's there. However, if the option is convenient enough, we could still do it. It's hard for me to judge, I don't use this feature often.

Aside: It'd be natural to want the automatic rebasing to skip empty commits as well when the option is set, but it unfortunately loses information (descriptions of empty commits). Also, I'm slightly biased at the moment, since I'm working on refactoring related code, so I don't want to add new features like that right now.

The alias is a great suggestion. Could you try using that to see how it goes? I'm curious, why do you think it's less user friendly? I wonder whether we could solve those issues instead. My main problem with aliases is that they break command-line completion; I wonder whether we could try fixing that, at least for simple aliases. I should, at least, file a bug.

[afgomez]

@ilyagr thank you for your reply, and thank you to @matts1 for building the feature indeed!

it might be confusing if rebase behaves differently from all the automatic rebasing jj does. [...] It'd be natural to want the automatic rebasing to skip empty commits as well when the option is set, but it unfortunately loses information (descriptions of empty commits)

I didn't consider this. As a user I would indeed expect any rebase operation skips the empty commits. Regarding losing the information, I think I'd be OK with it... but I cannot think of any scenarios where it might be a problem.

One way to minimise the issues here is to be explicit in the command output when a commit is skipped. Imagine this operation would create an empty commit:

% jj diffedit -r @--
Created wxqzllkp 391cea5e ABC
Rebased 1 descendant commits
Skipped empty commit: zyxwvutr 12345678 Wadus       # <=== Tell the user here. Use another color to bring more attention to it.
Working copy now at: rpyurrzs d40210b6 ABCD
Parent commit      : wxqzllkp 391cea5e ABC
Added 0 files, modified 1 files, removed 0 files

^^^ I think it is useful to tell the user if we are skipping commits regardless of the outcome of this issue. I can create a new issue to track this change

why do you think [the alias] it's less user friendly?

Maybe 'user friendly' was too strong of a term from my side. Maybe I just like it more.

The following is very much my train of thought and it's not well organised, so apologies in advance :D.

I think with an alias I end up building muscle memory to a command that only exists in my config. I want to change the default behaviour of a command (rebase in this case)... but my only option is to build muscle memory to a command that only exists in my config :D.

git has precedents where it allows the user to set up the default behaviour via a config flag. For example, the default git pull behaviour is to create a merge commit. However I don't think that has ever been what I wanted when pulling from a remote. So instead of having to specify git pull --rebase every time, or create a pullr alias that I need to remember, I can just add this to my config:

[pull]
rebase = true

And be done with it. I use the command that git provides. As a user I find more user-friendly (read: I like it more) to be able to configure the default commands other than create custom aliases that only I use.

Makes sense?

However...

Could you try using [an alias] to see how it goes?

I also wonder myself if the only scenario where I want the empty commits to be gone is when I'm restacking PRs, and with any other kind of rebasing I would like the default behaviour to still keep the empty commits... Right now an alias is a way to test this for myself.

[martinvonz]

#2600 has some related discussion. I'm currently leaning towards having the auto-rebase logic not change the shape of the graph, but maybe we could make an exception for empty commits.

Note that if your proposed config is about abandoning all empty commits (if they were rebased), not just emptied commits, then you'd often lose the working-copy commit and merge commits. So if we do add a config for it, I think we'd want it to be about abandoning emptied commits (i.e. commits that were non-empty and became empty as a result of the rebase).

[afgomez]

Note that if your proposed config is about abandoning all empty commits [...] I think we'd want it to be about abandoning emptied commits (i.e. commits that were non-empty and became empty as a result of the rebase).

@martinvonz yes! That's actually what I meant. (I guess that's also what --skip-empty does internally?)

[ilyagr]

One way to minimise the issues here is to be explicit in the command output when a commit is skipped.

That's a good point, it'd help. This would only show one line of the description but. That might be OK since, given the commit id of the removed commit, it is possible to recover it's description with something like jj log -T 'description' commit_id. This is currently not well-documented, but we could put it in a hint or something. One problem is that this could be annoying if too many commits are emptied, but I don't think that should be very common.

The following is very much my train of thought and it's not well organised, so apologies in advance :D.

No problem, this is what I was asking for, thanks for writing it!

Overall, my own mind is not made up one way or another, but there's food for thought here that I might try to digest at some later point, and perhaps others will have more thoughts or stronger opinions.

[ilyagr]

I want to change the default behaviour of a command (rebase in this case)... but my only option is to build muscle memory to a command that only exists in my config :D.

This argument can also swing the other way. I could argue that if you don't have your config, it's safer to see "this command does not exist" as opposed to the command silently behaving differently than you expected. With git's options like pull.rebase (or, worse, the options configuring the behavior of submodules on pull), I'm worried about forgetting what I put in my config and what's default behavior. In other words, I'm responsible for indefinitely remembering both if I ever come up on Git without my config.

[joyously]

This argument can also swing the other way.

This is what I wanted to say, but you worded it better than I would.

I'm responsible for indefinitely remembering both if I ever come up on Git without my config.

It's not just you and your config, but a support team having to get user config in order to help them, and project teams having to dictate the config for the team so tools all behave the same way everywhere.

[afgomez]

This argument can also swing the other way. I could argue that if you don't have your config, it's safer to see "this command does not exist" as opposed to the command silently behaving differently than you expected.

Yes, this was also in my mind. I tend to lean the other direction... but to be honest I don't have a strong argument for it, other than "git does it like that" and "I like it more".

I appreciate the discussion about the feature though. If you decide to not implement it I would still use jj, so it's not a big deal.