#10373
Issues/jj-vcs/jj/#10373

FR: `jj split` reads commit messages from top and bottom

Openwrigleysteropened 18d ago

Is your feature request related to a problem? Please describe. When editing commit descriptions, particularly when editing multiple commits at once, I find it difficult to search for and navigate to the location where the content that I should fill in goes. Here is an example:

JJ: Enter or edit commit descriptions after the `JJ: describe` lines.
JJ: Warning:
JJ: - The text you enter will be lost on a syntax error.
JJ: - The syntax of the separator lines may change in the future.
JJ:
JJ: describe f72de6d11822 -------
JJ: Enter a description for the selected changes.
This is the title of the first commit

This is the description of the first commit. Lorem
ipsum dolor sit amet, consectetur adipiscing elit.
JJ: Change ID: wlwzzowr
JJ: This commit contains the following changes:
JJ:     M .gitignore
JJ:
JJ: describe 58f06361b43b -------
JJ: Enter a description for the remaining changes.
This is the title of the second commit

This is the description of the second commit. Lorem
ipsum dolor sit amet, consectetur adipiscing elit.
JJ: Change ID: poxyrvxm
JJ: This commit contains the following changes:
JJ:     M package.json
JJ:
JJ: Lines starting with "JJ:" (like this one) will be removed.

Describe the solution you'd like It would be great, if the generated draft supported placing the description at the very top and very bottom of the file. A) Because the top is where the cursor is, when the editor opens, and B) Every editor has a shortcut to jump to the bottom of a file. Thus, the need to hunt for the right line to edit vanishes. In my example, I have placed the warnings together with the existing warning Lines starting with "JJ:" (like this one) will be removed.

Maybe this could be an option, or maybe the templating language could be extended to make this possible:

This is the title of the first commit

This is the description of the first commit. Lorem
ipsum dolor sit amet, consectetur adipiscing elit.
JJ: Enter a commit message above for the first change set.
JJ: Change ID: wlwzzowr
JJ: hash: f72de6d11822
JJ: This commit contains the following changes:
JJ:     M .gitignore
JJ: split -------------------------------------------------
JJ: Enter a commit message at the bottom for the second change set.
JJ: Change ID: poxyrvxm
JJ: hash 58f06361b43b 
JJ: This commit contains the following changes:
JJ:     M package.json
JJ:
JJ: Warning:
JJ: - The text you enter will be lost on a syntax error.
JJ: - The syntax of the separator line may change in the future.
JJ: - Lines starting with "JJ:" (like this one) will be removed.
This is the title of the second commit

This is the description of the second commit. Lorem
ipsum dolor sit amet, consectetur adipiscing elit.

If this is too involved, even swapping the behaviour to look above the "describe"-lines so the first commit message may be placed at the very top of the file, would be an immense improvement.

Describe alternatives you've considered I tried modifying draft_commit_description, but this seems intended for another purpose. I also tried to figure out how to revert to the old 2-editor behaviour, which always places the cursor in the vicinity of the required commit message. But there does not seem to be an option to revert the behaviour..

Activity

3
josephlou518d ago

#10269 added a way to tell your editor where to place your cursor. For example see #10279. You might be able to use this to make this experience a little better.

I also tried to figure out how to revert to the old 2-editor behaviour, which always places the cursor in the vicinity of the required commit message. But there does not seem to be an option to revert the behaviour..

Indeed, I don't think there's a way to revert this.

It would be great, if the generated draft supported placing the description at the very top and very bottom of the file.

I'm not sure that's a great idea, as it doesn't generalize very nicely to eventual support for splitting into multiple revisions (#5312), and also doesn't generalize to jj describe 'many_revisions()'. I think it would also make description editing more annoying (personally) since now a very specific description editor does not follow the normal pattern I'm used to.

I tried modifying draft_commit_description, but this seems intended for another purpose.

Yes, templates.draft_commit_description is for populating the marked lines here:

> JJ: Change ID: wlwzzowr
> JJ: This commit contains the following changes:
> JJ:     M .gitignore
  JJ:
  JJ: describe 58f06361b43b -------
  JJ: Enter a description for the remaining changes.

Also, if your editor supports dimming JJ: lines as "comments", that would probably also help you visually find where your descriptions are. This probably requires some extension that adds a custom language or something to indicate how the editor should apply syntax coloring to the file; for example the jjk VS Code extension adds https://github.com/keanemind/jjk/blob/main/languages/jj-commit.language-configuration.json, which is useful.

selfire116d ago

I find the one-editor split behaviour difficult to use as well! A couple of ideas of improvement:

Reduce boilerplate Every interactive split includes these lines:

JJ: Enter or edit commit descriptions after the `JJ: describe` lines.
JJ: Warning:
JJ: - The text you enter will be lost on a syntax error.
JJ: - The syntax of the separator lines may change in the future.

I wonder if some of those are superfluous:

  • "Enter or edit commit descriptions after the JJ: describe lines.": Maybe this line could move to the docs/help? I feel like the behaviour is quite self-explanatory and I'm not sure this instruction needs to be shown on every use of split -i.
  • "The syntax of the separator lines may change in the future.": Using 0.x software, imo makes it implicit that syntax may change. I'm not sure this warning warrants being shown every time on use.
  • "The text you enter will be lost on a syntax error.": Maybe this could also move to the docs/help? Changing the syntax seems like an edge case

One editor/multi editor configuration option For me, multi-editor provided a very focused workflow:

  1. I see all my selected changes by including git.diff
  2. I describe them
  3. I work on the next change

Especially with multi-way splitting this workflow is helpful I believe (as mentioned in another comment). With a single editor, finding the relevant description gets harder with multi-way splitting.

I appreciate that this was on your radar, yuja, with https://github.com/jj-vcs/jj/pull/10269. For me, it doesn't quite resolve all the pain points: $line only navigates to the first description. The first description is already fairly easy to find. I find subsequent descriptions hard to find, and it doesn't seem like there is an editor standard that supports multiple jump targets.

I wonder if it's worth making the one-editor/multi-editor behaviour configurable? You don't want to end in configuration hell, but there are benefits for the multi-editor workflow. I wonder if there are enough JJ users that would find multi-editor helpful?

The original FR https://github.com/jj-vcs/jj/issues/5432#issue-2804910348 did actually mention to have this option configurable to not break workflows.

Very happy to hear other perspectives as well though, and I really appreciate your work @yuja! 🤗

josephlou516d ago

Reduce boilerplate Every interactive split includes these lines: ... I wonder if some of those are superfluous:

  • "Enter or edit commit descriptions after the JJ: describe lines.": Maybe this line could move to the docs/help? I feel like the behaviour is quite self-explanatory and I'm not sure this instruction needs to be shown on every use of split -i.

Slightly disagree with this. I don't think it's necessarily self-explanatory, especially in the context of editing multiple descriptions in the same editor (e.g., jj describe 'many_revisions()' or jj squash), since you have this structure:

JJ: describe abc123 -------
actual description of commit 1

JJ: templates.draft_commit_description content
JJ: templates.draft_commit_description content
JJ:
JJ: describe def456 -------
actual description of commit 2

JJ: templates.draft_commit_description content
JJ: templates.draft_commit_description content

For example, if all the descriptions were blank, it's not immediately clear where descriptions are supposed to go. If this is a user's first time seeing a multi-description editor, they don't necessarily know where description(s) should go or what the "JJ:" lines mean.

  • "The syntax of the separator lines may change in the future.": Using 0.x software, imo makes it implicit that syntax may change. I'm not sure this warning warrants being shown every time on use.
  • "The text you enter will be lost on a syntax error.": Maybe this could also move to the docs/help? Changing the syntax seems like an edge case

I do agree with these. I may even suggest adding this to the [hints] config table, something like hints.description-editor-syntax = false? But more config options is a maintainer decision.

FWIW, I do think the warning is worth showing, because some people (like me lol) might try to parse or depend on the syntax in these editors, despite version 0.x. It's easier to put warnings in places to absolve the project from responsibility than to have to point to "we use semver, so 0.x means anything can change" in "bug" reports. But indeed, after you've gotten used to the context, I think it doesn't need to be shown as much anymore.