Add a --use-destination-message option to jj squash

[emesterhazy]

If -d/--drop-message is passed to jj squash, the descriptions of the commits being squashed are dropped and the message of the destination commit is used.

This is useful since I often find myself squashing a commit with a non-empty placeholder name such as "WIP: do something" or even "SQUASH: some change". Without this option jj squash always opens the diff editor and I need to navigate to the bottom of the description to remove the placeholder description. This is annoying and error-prone, and sometimes I accidentally leave something from the message that I wanted to discard.

I'm not tied to the name --drop-message, but I think that whatever name we use should have a short version as well. I also considered --abandon-message/-a, which might be better since it avoids ambiguity about whether -d means destination.

[martinvonz]

I feel like --use-description-from-destination (or other name) is a more often useful. Would that work for you?

[ilyagr]

Maybe --keep-destination-message and --keep-source-message?

[emesterhazy]

I feel like --use-description-from-destination (or other name) is a more often useful. Would that work for you?

I don't really care what we call it as long as there's a short flag for it. The problem with --use-description-from-destination or similar is that it would be mutually exclusive with --message. That's fine, but it makes the implementation and testing more complex because we have to add an additional error case.

Do you think --drop-message is a confusing semantic, or is there something about the semantics of --use-description-from-destination that's more useful?

[ilyagr]

You also need to run cargo insta test and then cargo insta review.

[martinvonz]

Do you think --drop-message is a confusing semantic, or is there something about the semantics of --use-description-from-destination that's more useful?

I think it sounds like an unusual use case that you have described two commits in different ways and now you want no part of either of the descriptions to apply to the combination. It seems like it would be much more common that one of the descriptions is still close to correct.

[martinvonz]

Maybe --keep-destination-message and --keep-source-message?

Works for me. The latter would have to be an error if there are multiple sources, I suppose. Or maybe it means that you want a combination of them? Maybe the flags should instead be --drop-destination-message and --drop-source-messages then? And maybe --drop-all-messages for Evan's use case.

[joyously]

What's the best default for the message?

[emesterhazy]

Oh sorry Martin. This feature does NOT drop all messages, it only drops the source messages. Maybe the flag should be called --drop-source-messages to avoid confusion. But if we want to add --drop-destination-message it's not clear what the short names would be.

I agree that --drop-destination-message is more complex. Realistically we would probably only allow that option if there is a single source revision. It doesn't seem very useful if there are multiple source revisions.

[martinvonz]

Ah! I think I read "the descriptions of the commits being squashed are dropped" and missed "and the message of the destination commit is used". Sorry!

I do think --drop-source-messages is much clearer about what it does than just --drop-message.

[ilyagr]

I do feel like it's better to talk about what message is being kept. That's the message I'd be thinking about when I use this option. So, I still prefer --keep-destination-message. The help description will explain that all the other messages are lost.

I'd also like --keep-source-message, though it's less important and doesn't have to be part of this PR. Martin is right that it only makes sense if there is exactly one --from revision.

[emesterhazy]

If we want "keep" instead of "drop", we should probably say "use" instead of "keep" since it makes it a little clearer that the other messages are dropped.

If we went with --use-destination-message and add --use-source-message later, what should the short version be? I'm weary of using -d and -s because they could be confused with --source and --destination, which are called --from and --into for jj squash (I would suggest changing them to -s and -d).

IMO the proposed names are hard to abbreviate. There's a bit less symmetry, but what about --abandon-source-message/-a and --use-source-message/-u? This way the abbreviated word is a verb and we don't have a conflict between the names of the two flags. Obviously there's still a lack of symmetry that we'd need to look past.

[ilyagr]

If we want "keep" instead of "drop", we should probably say "use" instead of "keep" since it makes it a little clearer that the other messages are dropped.

That sounds like a good idea to me. :)

If we went with --use-destination-message and add --use-source-message later, what should the short version be? I'm weary of using -d and -s because they could be confused with --source and --destination, which are called --from and --into for jj squash (I would suggest changing them to -s and -d).

How about aliases --ud and --us? -D and -S might also work, but I'm worried we will need those for some global option.

I think you are right that these are harder to abbreviate, but I think we could live with that (or we might come up with an even better idea).

IMO the proposed names are hard to abbreviate. There's a bit less symmetry, but what about --abandon-source-message/-a and --use-source-message/-u? This way the abbreviated word is a verb and we don't have a conflict between the names of the two flags. Obviously there's still a lack of symmetry that we'd need to look past.

This has the problem that it'll be hard to remember whether -u uses the source message or the destination message. In fact, since it's more common, I would expect --use-destination-message.

[emesterhazy]

If at all possible I would really like to have a two character (i.e. dash and letter) flag for this. For my workflow at least I almost always only keep the destination description. I actually don't really care about having a flag to keep only the source description, but maybe other people routinely squash commits that way.

Let's sit on it for a few days and see if we can come up with names that everyone's happy with.

[jonathantanmy]

The Git equivalent is probably to format the squashed commit specially (so that "fixup!" appears in its commit message - usually through "git commit --fixup") and then rebase with the config variable rebase.autoSquash set. One solution that might work for everyone is to do a similar thing - require that the squashed commit be formatted specially (from the original post, @emesterhazy seems to already be doing "WIP:" or "SQUASH:", so maybe they would be OK with "fixup!" instead) and add a config variable that makes "jj squash" automatically drop such commit messages.

[emesterhazy]

@ilyagr I just realized there is a jj unsquash command. Considering that, I don't think we need to add a --use-source-message flag to jj squash. We could just add a --abandon-parent-message or similar to jj unsquash.

What do you think?

[ilyagr]

The Git equivalent is probably to format the squashed commit specially (so that "fixup!" appears in its commit message - usually through "git commit --fixup") and then rebase with the config variable rebase.autoSquash set.

We could support that also.

I just realized there is a jj unsquash command. Considering that, I don't think we need to add a --use-source-message flag to jj squash. We could just add a --abandon-parent-message or similar to jj unsquash.

I'd prefer -u/--use-destination-message for the name. This is also a bit clearer if there are multiple --from commits (there's no confusion about whether one or all of the source commit descriptions are dropped).

Other than that, my thoughts are a little scattered at the moment. First, I should say that IMO this whole discussion is IMO about a slight improvement to an already good idea, not an urgent fix to some terrible problem with this PR.

I was previously thinking of either getting rid of unsquash or making it an option to squash, mainly because I never use it. However, maybe we could keep it just for the -u flag. If nobody uses it for that flag, we could get rid of it entirely. This did make me more comfortable with your plan, but I don't think a decision on unsquash should block this PR.

I think your plan (and/or this PR as-is, maybe with the option renamed) would be fine, especially since you seem to feel strongly about it. I'd still prefer --ud and --us even better, but we could see what others think. If nobody objects to your plan and you continue to feel strongly enough about it, it works for me.

---

Another thought, similar to your suggestion in #3427 (comment): we could start with -u now, and then maybe add -U / --use-source-message later. If it is, in fact, true that -u is more commonly used and more natural than -U, hopefully people will remember that -U is the "weird" direction.

Again, I don't mean that we absolutely have to implement -U right now. I just wanted there to be an idea for how to do it if we want it later, but all of that is a nice-to-have, not an urgent necessity.

Anyway, I think I'm thinking in circles now. Let's see if one of us, or somebody else, has some ideas. If not, as I said above, we can just go with what you have, maybe with the option renamed.

[emesterhazy]

I think I'm totally fine with --use-destination-message/-u and maybe eventually --use-source-message/-U. I really just wanted to have a single letter short name for the flag to make it as easy as possible to type.

If that works for everyone let's do it.

[martinvonz]

I tried this:

[profile.dev.package."*"]
opt-level = 3

It increased the build time from 39s to 49s and reduces test time with nextest from 20s to 13s. Seems like a good tradeoff overall.

[emesterhazy]

I tried this:

[profile.dev.package."*"]
opt-level = 3

It increased the build time from 39s to 49s and reduces test time with nextest from 20s to 13s. Seems like a good tradeoff overall.

Sounds great! But maybe you meant to comment this somewhere else XD?

Anyways, I updated this PR and it's ready for another look.

[emesterhazy]

@ilyagr @martinvonz How do you feel about the new flag name --use-destination-message/-u?

[joyously]

Wouldn't the destination message be the default?

[emesterhazy]

The current default is to open an editor that contains the messages from every commit that's been squashed including the destination commit.

[ilyagr]

How do you feel about the new flag name --use-destination-message/-u?

I'm OK with it, and we haven't seen any strong objections. I think this just needs somebody (me or somebody else) to give this another look over and actually approve it.

[emesterhazy]

@martinvonz if there are no objections I'd love to see this in the next release. Would you mind taking another look?

[martinvonz]

@martinvonz if there are no objections I'd love to see this in the next release. Would you mind taking another look?

Sure, will do. I've been on vacation since Wednesday.

[emesterhazy]

Sure, will do. I've been on vacation since Wednesday.

Sorry, I didn't know. Enjoy!!

[martinvonz]

Sorry, I didn't know. Enjoy!!

Oh, it was a short vacation. I'm already back :) Though it's still weekend, which means I try to spend my days with my family instead. I'm back to work tomorrow.