jj icon indicating copy to clipboard operation
jj copied to clipboard

FR: `jj split --message`, like `jj commit --message`

Open jyn514 opened this issue 1 year ago • 23 comments

Is your feature request related to a problem? Please describe. i want to create a commit without context switching between my terminal and my editor. i tried jj split PATH -m 'make a change'. that errored because jj split doesn't support a --message argument.

Describe the solution you'd like support --message in any command that creates a new commit, not just in jj commit

Describe alternatives you've considered

  • add --message only to jj split
  • add a way to run jj split without creating a message, so i can run jj split && jj describe @- --message 'make a change'

Additional context

; jj --version
jj 0.14.0-8e4d1af98eccf1ed8a79a7ddf9500467aa7684f5

jyn514 avatar Feb 15 '24 18:02 jyn514

In the case of split, which one would get the new message?

joyously avatar Feb 15 '24 19:02 joyously

Yes, it's not obvious which commit should get the message, especially if we add --siblings or if we allow splitting into more than 2 commits. I would prefer a --leave-message or something to use the current message for all commits after the split.

martinvonz avatar Feb 15 '24 21:02 martinvonz

A --leave-message or --keep-message option for split seems like the better way to go for the reasons Martin gave.

It basically sounds like you're looking for a non-interactive jj split, which is only possible if you specify the paths for the split and don't have to write a commit message. I think having the --keep-message option to suppress the commit message editor and chaining it with jj describe is the most flexible way to achieve this.

Adding a --keep-message option should be pretty simple... I can mail a patch for it if there's agreement about adding it.

Are there any other commands where you think this could be useful?

emesterhazy avatar Feb 15 '24 21:02 emesterhazy

Adding a --keep-message option should be pretty simple... I can mail a patch for it if there's agreement about adding it.

Sounds good. Thanks.

Are there any other commands where you think this could be useful?

I think the only other commands that ask for a description are jj describe and jj commit and we probably don't need it for either of them. For jj commit, the user can simply use jj new if they don't want to set a description. For jj describe, the goal is usually to set the description. It could possibly be useful to have the flag for combining with e.g. --reset-author. Up to you if you want to add it there.

martinvonz avatar Feb 15 '24 22:02 martinvonz

Should jj split prompt for a commit message interactively at all? What happens if it doesn't do that and the user has to run jj describe explicitly after the fact, or if they have to opt in instead?

arxanas avatar Feb 16 '24 05:02 arxanas

Should jj split prompt for a commit message interactively at all? What happens if it doesn't do that and the user has to run jj describe explicitly after the fact, or if they have to opt in instead?

Good question. Maybe it shouldn't. I think I just copied the behavior from hg split, but hg requires a description on commits, and I don't think I considered skipping it in jj. I later made jj split not ask for a description for the second commit if the original commit had no description. But maybe we should never ask for a description, as you say.

martinvonz avatar Feb 16 '24 05:02 martinvonz

I like the description prompt since by default you're encouraged and able to describe the separate commits right after you've chosen the diffs they contain. Waiting to do it after the fact could be more difficult, especially if we allow more than two splits, which I'd also like to see.

The default of interactively choosing the diff and setting the message makes sense to me.

emesterhazy avatar Feb 16 '24 06:02 emesterhazy

Personally I would like to see jj split not prompt for a message unless -i/--interactive was explicitly passed, including for multi-way splits. I would also argue that jj split with no arguments should print usage help rather than automatically starting interactive splitting.

I like the description prompt since by default you're encouraged and able to describe the separate commits right after you've chosen the diffs they contain. Waiting to do it after the fact could be more difficult

The current workflow also waits until after the fact. You have to declare the splits before you can edit either commit message anyways. scm-record has functionality to actually edit the commit message inline while doing the split, but I don't think there's a good way to get an external diff editor to do that?

arxanas avatar Feb 17 '24 03:02 arxanas

Personally I would like to see jj split not prompt for a message unless -i/--interactive was explicitly passed, including for multi-way splits.

Why? If you've split the commit into one or more parts, it seems like it would be unusual for the same commit message to apply to all of the parts. If you want that behavior there could be a flag for it, but I don't see that being a useful default for most people. Curious to hear why you'd prefer it.

emesterhazy avatar Feb 17 '24 05:02 emesterhazy

I don't think I understand what you think I am in favor of. Maybe I wrote imprecisely? I would expect to assign different commit messages to different parts. But I think that point is orthogonal to interactive vs non-interactive entering of the commit message?

arxanas avatar Feb 17 '24 06:02 arxanas

Sorry, I am trying to ask why that is preferable as a default compared to editing the message interactively. Deferring to later makes it easier to forget what's included in each commit IMO. I think it's fine as a supported workflow, I'm just not sure it should be the default.

emesterhazy avatar Feb 17 '24 12:02 emesterhazy

Sorry, I am trying to ask why that is preferable as a default compared to editing the message interactively. Deferring to later makes it easier to forget what's included in each commit IMO.

When it comes to "deferring to later", I don't understand how the currently-possible interactive vs non-interactive workflows don't both defer editing the message to later. I already can't edit the message while I'm interactively selecting the changes to split, so any message I supply must be after I already performed the split.

I would like to see jj split <paths> not prompt for a commit message by default unless --interactive was passed. You can still provide a commit message immediately by passing -i in that case, but I don't think it's good practice to make certain invocations of jj split interactive based on the rest of the arguments. In particular, jj split $PATHS in a script will hang indefinitely waiting for input instead of being a no-op when $PATHS is empty.

arxanas avatar Feb 17 '24 20:02 arxanas

I don't think it's good practice to make certain invocations of jj split interactive based on the rest of the arguments.

Consistency is good, but isn't it consistently interactive today? I don't think there's a way to avoid the prompt for the first commit. If there is, we could fix that to make it consistently interactive by default instead of non-interactive.

What I'm most curious about is why you think non-interactive is a better default. If you explained at some point I think I've missed it. Could you clarify?

emesterhazy avatar Feb 17 '24 20:02 emesterhazy

What I'm most curious about is why you think non-interactive is a better default. If you explained at some point I think I've missed it. Could you clarify?

Specifically there's this potential footgun:

In particular, jj split $PATHS in a script will hang indefinitely waiting for input instead of being a no-op when $PATHS is empty.

Philosophically, I think commands should be non-interactive by default unless we have a strong reason to make them interactive. jj describe is one such command because it's fundamentally about accepting free-form input from the user.

Currently, we have these commands with --interactive flags:

  • jj commit
  • jj split
  • jj move
  • jj squash
  • jj unsquash

For which

  • jj describe is default-interactive and jj commit is essentially meant to be an alias for jj new && jj describe, so it's also default-interactive.
  • jj move/jj squash/jj unsquash are not default-interactive (?), and do not prompt for interaction unless you pass --interactive.

I think jj split is much more like move/squash/unsquash than like jj describe, and it surprises me whenever jj split behaves interactively without me having passed --interactive. It will become even more similar to the other things if we add something like jj split --by hunk to split into multiple commits based on hunks.

arxanas avatar Feb 17 '24 21:02 arxanas

If you start reading this from the top, you'll be reminded that the original request was for adding a message parameter, not interactivity. Should split even be a thing, if it's too complicated? Perhaps just uncommit or leapfrog to absorb as mentioned elsewhere.

joyously avatar Feb 17 '24 21:02 joyously

If you start reading this from the top, you'll be reminded that the original request was for adding a message parameter, not interactivity

One of the original alternatives is

add a way to run jj split without creating a message, so i can run jj split && jj describe @- --message 'make a change'

Which I think is the same as a request for non-interactivity. I suppose we could add a --no-interactive flag, but that seems unlikely to be popular; or we could toggle the interactivity based on the rest of the parameters, which I'm somewhat opposed to as per the above discussion.

I would actually be in favor of adding --message argument, which would be applied to all commits that don't share the original change ID.

Should split even be a thing, if it's too complicated?

I think split is very useful and I'm not sure how to simplify it here.

  • How would an uncommit command behave to address the split workflow in a way that's different from split? Would it just differ by whether it's interactive or not?
  • I don't think absorb handles the same use-cases as it does the opposite of splitting?

arxanas avatar Feb 17 '24 21:02 arxanas

i think split is very useful. i don't want it to go away.

In the case of split, which one would get the new message?

i would expect the new commit being created to get the message, just like jj commit, and how jj split today interactively applies the message to the new commit.

It basically sounds like you're looking for a non-interactive jj split, which is only possible if you specify the paths for the split and don't have to write a commit message.

yes.

it's not obvious which commit should get the message, especially if we add --siblings or if we allow splitting into more than 2 commits. I would prefer a --leave-message or something to use the current message for all commits after the split.

that's not very useful, though. why would i create several commits that all have the same message?

i would be happy for --message to give a hard error in some future world where split allows creating multiple commits at once and i pass another flag that makes it do that.

Should jj split prompt for a commit message interactively at all? What happens if it doesn't do that and the user has to run jj describe explicitly after the fact, or if they have to opt in instead?

i feel very strongly it should prompt for a message. people can always leave an empty description and run jj describe later. not prompting makes it easy to create a commit you intend to be "finished" that has no description.

I think jj split is much more like move/squash/unsquash than like jj describe, and it surprises me whenever jj split behaves interactively without me having passed --interactive. It will become even more similar to the other things if we add something like jj split --by hunk to split into multiple commits based on hunks.

i disagree. jj split is closer to jj commit, because unlike any of other the commands, it creates a new commit. i think anytime jj creates a new commit it should ask for a description, the only exception is for jj new.

jyn514 avatar Feb 18 '24 00:02 jyn514

as an aside, i'm kind of frustrated that i made what i saw as a small feature request and it got turned into a different feature (ok i guess, but i wish you'd asked if it solves my problem before ok-ing a PR) and then people started proposing completely redoing or removing this command altogether. i don't have the time to constantly check on github throughout the week.

jyn514 avatar Feb 18 '24 00:02 jyn514

@arxanas no offense meant, but I still don't think you've given an explanation of why a non interactive split command is more useful as the default. You said you're philosophically opposed to interactive commands, but that's not enough of a justification in my opinion.

If the command is always interactive by default then I don't think it presents a footgun to script authors.

If you're writing a script you can write --no-interactive or --message once and be done. If I'm typing jj split several times a day to split out some lines of a change and finalize it I don't have that luxury and will have to add -i every time. I genuinely think the interactive workflow is more common and useful. I'd hate to see it removed if there isn't a concrete reason why a non interactive command is more useful to users.

Anyways, I agree this has gotten off track a bit.

Maybe everyone would be satisfied if we had separate split-files and split-lines commands. As far as I know there's no way to do a non-interactive line split, so there shouldn't be any confusion. People that prefer one over the other could just add an alias to their preferred command, or we could have built in aliases "sf" and "sl".

emesterhazy avatar Feb 18 '24 02:02 emesterhazy

If the command is always interactive by default then I don't think it presents a footgun to script authors.

I think @arxanas's argument was that jj split <paths> is not interactive, unless <paths> happens to be empty, so I can see that being a footgun.

However, what's the alternative? How would jj split know how to split if it doesn't ask you interactively? I agree that it would be consistent for it to include an empty set of paths in the first commit, but is that useful? You would almost aways want to follow that up with jj squash -i -r <child> and jj describe <parent> in that case, right?

martinvonz avatar Feb 18 '24 16:02 martinvonz

I think @arxanas's argument was that jj split <paths> is not interactive, unless <paths> happens to be empty, so I can see that being a footgun.

Is that true though? When I call jj split <SOME_FILE> it seems to always open the diff editor. Is there a way to run jj split non-interactively today? I don't see any tests asserting that the editor is never opened either.

emesterhazy avatar Feb 18 '24 17:02 emesterhazy

Is that true though? When I call jj split <SOME_FILE> it seems to always open the diff editor.

It should only do that if you passed -i:

  -i, --interactive
          Interactively choose which parts to split. This is the default if no paths are provided

And that's the behavior I saw when I just tried it.

martinvonz avatar Feb 18 '24 17:02 martinvonz

Oh, I think we are all talking about different things. There's the interactive diff picker, and there's an interactive prompt to enter a commit message. I think a large portion of this discussion is about removing / disabling the interactive commit message prompt, which currently is unavoidable.

So from my perspective (and maybe @arxanas and @jyn514 as well), jj split <paths> is interactive because it blocks waiting for the user to enter a commit message.

emesterhazy avatar Feb 18 '24 19:02 emesterhazy