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

backout: add --template option #3583

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

bnjmnt4n
Copy link
Collaborator

@bnjmnt4n bnjmnt4n commented Apr 26, 2024

Closes #2669.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

.set_description(format!("backout of commit {}", &old_commit.id().hex()))
.set_description(
commit_description
.unwrap_or_else(|| format!("backout of commit {}", &old_commit.id().hex())),
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if the commit description should just be required?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just about to say that :) Then you can remove the i18n comment too. (I'm kind of wondering if we even want this function or if we should just inline it in the CLI crate, but we can decide that later.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and -m/--message could support some form of templating if needed.
#3253

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting adding a --message option, and also using it as a template when --template is also passed, like what the last few comments suggest?

Copy link
Collaborator

@yuja yuja May 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting adding a --message option,

Yes. It's similar to commit -m/describe -m.

also using it as a template when --template is also passed

I have no concrete idea how template expression will be enabled in --message (and other string arguments.)

If there's there isn't an immediate need for templated commit message, I would just add -m/--message.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I started this PR was because I wanted automatically generated git revert-style commit messages, without having to manually type out the old commit's description + commit hash. Since #2669 had requests for different formats, I thought templating would be the preferred solution. Otherwise, if switching to git revert-style commit messages is acceptable, I can change this PR to do that.

I also wanted #3339 (multiple revisions for backout), which I think wouldn't work too well with --message.

Any thoughts? (/cc @thoughtpolice since you commented on #2669.)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I started this PR was because I wanted automatically generated git revert-style commit messages, without having to manually type out the old commit's description + commit hash. Since #2669 had requests for different formats, I thought templating would be the preferred solution.

Perhaps, it will be per-action (e.g. describe/squash/backout/..) description template? Mercurial has a similar feature.
#1354
https://repo.mercurial-scm.org/hg/help/config (search "committemplate")

One tricky part is what the self commit (= template context) should be. In order to generate a default commit description, we'll need the source commit. If we add multi-rev backout, it will be Vec<Commit>. OTOH, we might have to use the new "backout" commit to generate a editor message, which may contain a diff or diff summary. If jj backout is changed to open an editor by default, there might be separate templates for commit description and editor content.

Otherwise, if switching to git revert-style commit messages is acceptable,

I personally think it's acceptable.

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

Successfully merging this pull request may close these issues.

FR: Improve jj backout generated description
3 participants