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: git commit --verbose equivalent for jj describe #1946

Closed
sullyj3 opened this issue Aug 2, 2023 · 10 comments
Closed

FR: git commit --verbose equivalent for jj describe #1946

sullyj3 opened this issue Aug 2, 2023 · 10 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@sullyj3
Copy link
Contributor

sullyj3 commented Aug 2, 2023

Often when writing a commit message, it's useful to be able to see the diff at a glance. Git has git commit --verbose, which displays the diff inline in your editor while you're writing the message. This saves having to try to remember what changes you made.
I personally use this all the time. I have

alias gca='git commit --all --verbose'

which I use for committing almost exclusively.

It would be cool if jj supported this as well! I'm not wedded to the name --verbose, which I think could be clearer (and in fact looking at the help it conflicts with an existing flag with different functionality). Maybe --show-diff?

@sullyj3 sullyj3 changed the title FR: jj describe --verbose FR: jj describe --verbose Aug 2, 2023
@sullyj3 sullyj3 changed the title FR: jj describe --verbose FR: git commit --verbose equivalent for jj describe Aug 2, 2023
@ilyagr
Copy link
Contributor

ilyagr commented Aug 2, 2023

This could apply to quite a few commands (describe, split, squash, etc)

@martinvonz
Copy link
Member

Maybe we can have a configurable template for producing that. We currently include something like this:

JJ: This commit contains the following changes:
JJ:     M lib/src/working_copy.rs
JJ:     M lib/tests/test_working_copy.rs

Maybe that could be replaced by a template like "This commit contains the following changes:\n" ++ indent(" ", diffsummary), and you could replace the diffsummary by diff (or gitdiff, perhaps) to get the full diff.

We don't yet have support for including a diff in templates, so we'd need to implement that first.

#1354 is slightly related.

@ilyagr ilyagr added good first issue Good for newcomers enhancement New feature or request labels Aug 9, 2023
@9999years
Copy link

I went looking for this feature, remembered jj has templates, and was a little surprised that jj describe didn't have a --template option for this yet, so I think templates would be a natural place to implement this. (Although I suspect this is a common enough use-case that a dedicated flag might be worthwhile, even if it's just an alias for --template=commit-with-diff or something.)

@Cretezy
Copy link
Contributor

Cretezy commented May 10, 2024

Looking for consensus before implementing this:

  1. Should we use --show-diff for diff/squash/split?
  2. Should we instead use a template feature for this?
  3. If using template, how will this be used? Will jj ship with a built-in template for this?

@yuja
Copy link
Contributor

yuja commented May 11, 2024

  1. Should we use --show-diff for diff/squash/split?

I think config knob is better. It was discussed previously in
#3470 (comment)

  1. Should we instead use a template feature for this?

Template is preferred because it can solve other problems (such as #1354 and #3385), but it's not easy. I think adding a diff formatting option is also good as a short-term workaround.

  1. If using template, how will this be used? Will jj ship with a built-in template for this?

Yeah. Perhaps, the default ui.default-description will be a template like

concat(
  description,
  "\n",
  "JJ: This commit contains the following changes:\n",
  indent("JJ:     ", format_diff_in_commit_description(diff)),
)

and format_diff_in_commit_description() will be customized by user.

@yuja
Copy link
Contributor

yuja commented Jul 15, 2024

Template is preferred because it can solve other problems (such as #1354 and #3385), but it's not easy.

FWIW, I'm working on commit.diff([paths]) template function, and I think a basic version can be added soon.

@yuja yuja self-assigned this Jul 19, 2024
yuja added a commit to yuja/jj that referenced this issue Jul 25, 2024
This implements a building block of "signed-off-by line" jj-vcs#1399 and "commit
--verbose" jj-vcs#1946. We'll probably need an easy way to customize the diff part,
but I'm not sure if it can be as simple as a template alias function. User
might want to embed diffs without "JJ: " prefixes?

Perhaps, we can deprecate "ui.default-description", but it's not addressed in
this patch. It could be replaced with "default_description" template alias,
but we might want to configure default per command. Suppose we add a default
"backout_description" template, it would have to be rendered against the
source commit, not the newly-created backout commit.

The template key is named as "draft_commit_description" because it is the
template to generate an editor template. "templates.commit_description_template"
sounds a bit odd.

There's one minor behavior change: the default description is now terminated
by "\n".

Closes jj-vcs#1354
yuja added a commit that referenced this issue Jul 25, 2024
This implements a building block of "signed-off-by line" #1399 and "commit
--verbose" #1946. We'll probably need an easy way to customize the diff part,
but I'm not sure if it can be as simple as a template alias function. User
might want to embed diffs without "JJ: " prefixes?

Perhaps, we can deprecate "ui.default-description", but it's not addressed in
this patch. It could be replaced with "default_description" template alias,
but we might want to configure default per command. Suppose we add a default
"backout_description" template, it would have to be rendered against the
source commit, not the newly-created backout commit.

The template key is named as "draft_commit_description" because it is the
template to generate an editor template. "templates.commit_description_template"
sounds a bit odd.

There's one minor behavior change: the default description is now terminated
by "\n".

Closes #1354
@lukerandall
Copy link
Contributor

lukerandall commented Nov 19, 2024

For those who stumble upon this; the nearest equivalent (since #4153) is to add something like this to your config file:

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

or if you don't want it all the time add an alias using --config-toml (in this example I use a template-alias to make the alias easier to read):

[aliases]
diffscribe = [
  "describe",
  "--config-toml=templates.draft_commit_description=\"commit_description_with_diff\"",
]

[template-aliases]
commit_description_with_diff = '''
concat(
  description,
  surround(
    "\nJJ: This commit contains the following changes:\n", "",
    indent("JJ:     ", diff.stat(72)),
  ),
  surround(
    "\nJJ: Diff:\n", "",
    indent("JJ:     ", diff.git()),
  ),
)
'''

The most notable deficiency is that you don't get diff highlighting in your text editor like you would with a git commit diff because of the prefixed JJ:.

Edited to fix git(72)git() as noted by @tbodt below.

@tbodt
Copy link

tbodt commented Nov 24, 2024

 surround(
   "\nJJ: Diff:\n", "",
   indent("JJ:     ", diff.git(72)),
 ),

Important note - you almost certainly don't want diff.git(72), because the 72 becomes the number of lines of diff context! diff.git() appears to default to 3 lines of context which is fine.

@bryceberger
Copy link
Contributor

Since #5155, you can now do something like:

[template-aliases]
commit_description_verbose = '''
concat(
  description,
  "\n",
  "JJ: This commit contains the following changes:\n",
  indent("JJ:    ", diff.stat(72)),
  "JJ: ignore-rest\n",
  diff.git(),
)
'''

[aliases]
dv = ["--config=templates.draft_commit_description=commit_description_verbose", "describe"]

@yuja
Copy link
Contributor

yuja commented Dec 24, 2024

Thanks. I think we can now close this issue?

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

No branches or pull requests

9 participants