Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FR: Automatically delete all lines below (and including) a "scissor line" in description buffers #4210

Open
gpanders opened this issue Aug 4, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@gpanders
Copy link
Collaborator

gpanders commented Aug 4, 2024

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

Git supports something they call a "scissor line" in git commit messages. This line and anything below it will be automatically deleted by git before applying the commit message. This allows the commit message buffer to contain text without being commented out. In particular, this is useful for including diffs into commit message buffers.

jj now supports including diffs in description message buffers with the draft_commit_description template and the diff.git() function, e.g.:

[templates]
draft_commit_description = '''
concat(
  description,
  surround(
    "\nJJ: This commit contains the following changes:\n", "",
    indent("JJ:     ", diff.summary()),
  ),
  "\n",
  diff.git(),
)
'''

However, because the diff is not commented with the JJ: comment prefix, the diff becomes part of the description message. Prepending JJ: to the range of the diff does not work because editors lose the ability to syntax highlight the diff (it also makes the diff much harder to read).

I would like jj to support a "scissor line" concept similar to git, where any line below the scissor line is automatically removed before applying the description message, even if it does not contain the JJ: comment prefix.

Describe the solution you'd like

Add a new template function called scissor_line() (or cut_line() or similar, I am indifferent to the name). This function inserts a line according to another config option (ui.scissor-line perhaps) which defaults to JJ: ----------- >8 ----------- (the >8 looks like a pair of scissors, hence the name). This is the same value used by git: again, I'm indifferent to the actual value here, but this works well enough.

When the user exits their editor after writing a description message, jj removes the scissor line and all lines after it.

If/when jj supports writing/modifying multiple descriptions in the same file, then jj would only delete lines between 2 successive scissor lines (or perhaps we would need two separate markers: "start scissor/cut region" and "end scissor/cut region".

Describe alternatives you've considered

This is possible to solve already today with some custom editor integration. I am adding a scissor line "manually" to my draft_commit_description template and am using a hook in my editor (Neovim) to modify the temporary file to remove the scissor line. This works pretty well, but it'd be nice to be able to only use jj for this (and if it does not rely on custom editor integrations it would be easier to include things like diffs in default description messages).

@yuja
Copy link
Collaborator

yuja commented Aug 4, 2024

No template function would be needed. We can just inline "----------- >8 -----------" or make it a template alias for readability. We'll instead need code changes in parsing/cleanup part.

(If we add config knob for the scissor line and JJ: prefix, it's true that we'll need some way to access them from template.)

@yuja yuja added the enhancement New feature or request label Aug 4, 2024
@ilyagr
Copy link
Collaborator

ilyagr commented Aug 4, 2024

We'd also need to decide how this should interact with #3828, multiple commits' description in one file.

@arxanas
Copy link
Collaborator

arxanas commented Aug 4, 2024

Would recommend not using >8 by default, as it's not self-explanatory or discoverable. I spent many years using Git not even realizing that it was a static string (thought it might be some kind of variable-length delimiter encoding — why not >9?). It's hard to search for if you don't already know what it is.

@ilyagr
Copy link
Collaborator

ilyagr commented Aug 4, 2024

To make it work with #3828, we'd probably want an ending marker too. Perhaps something like:

JJ DIFF BEGIN: -------------- 8< ---- BEGIN SCISSORS ---- >8 ----------------
...
JJ DIFF END:   -------------- 8< ----- END SCISSORS ----- >8 ----------------

(This example mixes several ideas that don't feel fully baked yet, we could use some subset of them)

There's also the question of whether there are other tools we care about that recognize Git's scissors lines. I guess it would mostly be commit message editors, which would need special jj support anyway, so maybe not? But if yes, I don't know whether they'll recognize my example lines above.

By the way, I agree with @arxanas that we should not use >8 on their own. I included them since I think they look neat and might feel familiar to some people (and I think using 2 makes it clearer what they are; I was going back and forth on whether the English word "scissors" should be kept), but some other syntax such as the English explanation should be more prominent.


Update: A nice thing about diffs (unified diffs, at least) is that most lines are likely to start with +, -, or , so we don't have to worry about it starting with JJ. I wouldn't worry about it too much regardless, but it's a nice coincidence, and makes we vote that whatever our marker is, it does not start with or -.

@yuja
Copy link
Collaborator

yuja commented Aug 5, 2024

To make it work with #3828, we'd probably want an ending marker too.

It first splits by JJ: describe lines, so the scissor lines will work without modifications. However, it would be visually difficult to find the splitter lines if lengthy diffs are inlined.

@martinvonz
Copy link
Owner

To make it work with #3828, we'd probably want an ending marker too.

It first splits by JJ: describe lines, so the scissor lines will work without modifications.

Yes, I think that should be enough.

However, it would be visually difficult to find the splitter lines if lengthy diffs are inlined.

I suppose users who chose to include a diff can figure out a trailer comment to add to make it easier to find for them.

@ilyagr
Copy link
Collaborator

ilyagr commented Aug 6, 2024

? irst splits by JJ: describe lines, so the scissor lines will work without modifications

Good point! They could also be optional (if we find some use for that, e.g. if there's another type of content we want to put after).

However, it would be visually difficult to find the splitter lines if lengthy diffs are inlined.

I think we should change the splitter lines to something that looks louder than the scissor lines (whatever those will look like). That would probably be good enough. Even without scissor lines, I think our current diff splitter line could be deleted by people who don't realize its important, or missed in long descriptions if they have diagrams or something.

There's no rush, though, we can always figure out what looks good when we have both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants