Skip to content

Conversation

blairconrad
Copy link
Contributor

@blairconrad blairconrad commented Apr 13, 2025

Closes #172.

Specifically, parts 3 and 4 of the proposed plan are completed here:

  • Adjust stack::working_stack to also return a reason why the commit-finding search ended
    (e.g. reached a merge commit, hit the stack depth limit, etc.)
  • Move user-facing logging out of stack.rs and use the reasons found above to
    help the lib.rs code present better messages to the user

The first 4 commits alter the flow through run_with_repo, sometimes exiting early,
and sometimes delaying logging until we know the full effect of searching for
commits in the stack.

Then the last commit incorporates the reason the stack ends into the warnings
(if any) to the user.

Also closes #21.

Some patches do not modify an existing file in place.
Instead they add, remove, or rename a file.
This is a "non-modified patch", and will commute with
everything in the stack.

If all patches are non-modified, then no changes will be absorbed, so
warn the user and stop processing. Even users that auto-stage changes
will be warned, since they would've run git-absorb with the expectation
that it would do something.

Otherwise, if only some patches are non-modified, we can continue
processing with a warning. In this case, do not warn users who
auto-stage changes, since they may routinely keep untracked files in
their working directory.

Before this,

1. users who did not auto-stage were warned when nothing was staged.
   We have a more explicit warning and early exit above now.
2. there were staged changes (auto-staged or not) and all the patches
   were non-modified, we'd warn "Could not find a commit to fix up",
   which is accurate, but not as informative.
@blairconrad
Copy link
Contributor Author

@tummychow does not yet incorporate SlogValue, which I'll look into now.
I was keen to update the code I had in flight with the newly-merged PRs,
and thought I might catch your window of availability.

@tummychow tummychow merged commit e70893c into tummychow:master Apr 13, 2025
6 checks passed
@blairconrad
Copy link
Contributor Author

OMG. Merged without complaint. Thanks for entertaining my wacky ideas, @tummychow.

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.

Proposed Project: overhaul user-facing messages Add instructions on what to do when there are still staged changes after running git absorb
2 participants