Skip to content

Comments

Add --one-reflog-entry option#162

Closed
sh-at-cs wants to merge 2 commits intotummychow:masterfrom
sh-at-cs:fix-gh-157-option-4-one-reflog-entry
Closed

Add --one-reflog-entry option#162
sh-at-cs wants to merge 2 commits intotummychow:masterfrom
sh-at-cs:fix-gh-157-option-4-one-reflog-entry

Conversation

@sh-at-cs
Copy link
Contributor

@sh-at-cs sh-at-cs commented Mar 10, 2025

This implements option (4) from #157, adding a command-line flag to make git-absorb create only a single reflog entry (instead of one per commit as it normally does).

This change would allow users to easily "undo" all of git-absorb's commits at once by doing git reset HEAD@{1}.

Fixes #157.

Comment on lines +345 to +349
let mut head_ref = repo.head()?;
head_ref.set_target(head_commit.id(), "absorb: adding fixup commits")?;
// If you don't like the fancy custom reflog message above and want to stick to Git's own
// messages, this would also work:
// repo.reset(head_commit.as_object(), ResetType::Soft, None)?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If everything else is fine, an opinion would be needed here.

Comment on lines +344 to +351
if config.one_reflog_entry && !config.dry_run {
let mut head_ref = repo.head()?;
head_ref.set_target(head_commit.id(), "absorb: adding fixup commits")?;
// If you don't like the fancy custom reflog message above and want to stick to Git's own
// messages, this would also work:
// repo.reset(head_commit.as_object(), ResetType::Soft, None)?;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: This almost definitely doesn't work in combination with --and-rebase, which I forgot to consider => fix & add test.

Or would it be fine to just make --one-reflog-entry mutually exclusive with --and-rebase? 🤔 Because I think making these work together could become quite involved...

Copy link
Contributor Author

@sh-at-cs sh-at-cs Mar 10, 2025

Choose a reason for hiding this comment

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

Or, another idea: Call the option --one-reflog-entry-for-commits or something like that, which makes it clear that this applies to the commits only and not to the subsequent rebase of --and-rebase. Thoughts?

Comment on lines +586 to +590
// sanity checks: all is as usual when it comes to commits and index:
let mut revwalk = ctx.repo.revwalk().unwrap();
revwalk.push_head().unwrap();
assert_eq!(revwalk.count(), 3);
assert!(nothing_left_in_index(&ctx.repo).unwrap());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just copied this from the --one-fixup-per-commit test, but should anything else be checked to ensure this doesn't accidentally alter the behavior in other ways?

@blairconrad
Copy link
Contributor

blairconrad commented Mar 10, 2025

Drive-by comment: if the new flag is accepted, it should be documented in Documentation/git-absorb.adoc.

@tummychow
Copy link
Owner

so as a baseline: the way git-rebase handles reflog entries is it makes several of them for HEAD, but only one of them for the branch being modified (assuming that HEAD is not detached, of course). if i was going to accept this feature, i would expect it to work the same way: leave the full trail of reflog entries in HEAD, but have one entry on the branch. this also means you don't have to clean up the HEAD reflog entries generated by the rebase. ie:

  • no --and-rebase: make one reflog entry on HEAD for each absorb commit
    • then (if HEAD is attached) make one reflog entry on the branch for the final absorb commit
  • with --and-rebase: make one reflog entry on HEAD for each absorb commit, then let git-rebase make a bunch of reflog entries on HEAD for the rebase itself
    • then (if HEAD is attached) do not add any reflog entries on the branch at all, because git-rebase has already made exactly one entry for the post-rebase HEAD

also, i wouldn't bother gating this behind a flag, it should just be always-on

@sh-at-cs
Copy link
Contributor Author

Closing because this wouldn't play nicely with --and-rebase (#162 (comment)) and as was pointed out in #162 (comment), you do generally want to have the reflog history of which commits were made in what order _somewhere.

@sh-at-cs sh-at-cs closed this Mar 11, 2025
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.

Feature request: "Undo" subcommand / command-line flag

3 participants