Skip to content

feat: add split command #1464

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

claytonrcarter
Copy link
Collaborator

@claytonrcarter claytonrcarter commented Dec 8, 2024

This adds a git split command to extract changes from commits. This mostly addresses #180, except for the -i/--interactive part. The split and rebase happens in-memory, so it should be fast. The command signature is

git [branchless] split <revset> <file> [<file>...]

  • <revset> must resolve to a single commit (or just be a single commit, or @, etc)
  • if only one file is given, then the file name is used in the extracted commit message
  • if 2+ files are split out, then <n> files is used in the extracted commit message
  • in either case, a brief insert/delete summary is also used, eg (+2/-5)
    • eg temp(split): lib.rs (+5) or temp(split): 3 files (+3/-3)

Implemented flags

These alter how the split/rebase happens:

  • default (no flags) is "insert after": extracted changes are inserted into commit graph as a child of the target commit
    • any children of the target commit are rebased onto the extracted commit
    • eg A - B - CA - B' - b' - C
  • --detach: similar to above, but
    • children of the target commit are rebased onto the newly split target commit, not onto the extracted commit
    • eg A - B - CA - B' - C and B' - b
    • that is: b and C are now both children of B'
  • --discard: similar to --detach, but after extracting the changes, they are simply discarded
    • eg A - B - CA - B' - C
  • --before: the extracted changes are inserted as a parent of the newly split target commit
    • children of the target commit are rebased onto the newly split target commit
    • eg A - B - CA - b - B' - C
  • with --detach and --discard, rebase conflicts are possible, and are intended to be resolved in the same way are for git move conflicts

Status

This is "works for me" and has been stable and useful over months of personal testing. That said, the code is mostly OK, but there are still a few FIXME/TODOs to resolve, and I'm certain that the code could be more idiomatic or branchless
in general. I have written a bunch of tests, but it's possible that I have overlooked some edges.

Any and all feedback is welcome. I'm happy with this as is, but I'm looking forward to some review helping me be really proud of it. =)

Specific areas needing feedback and review

  • The temporary commit messages. Style, format, etc. The code that generates them feels sort of complicated; maybe it's not pulling its weight, and we should use a simpler or more familiar option?
  • The structure and flow of the business logic still confuses me a bit, and I suspect I'm overlooking some simplifications. For example, the variable names used for the new commits (extracted_commit and remainder_commit) feel good, but they actually mean the opposite if the --before flag is passed. 🤷 I've commented this to explain, but I'm hoping someone else will have some suggestions.

@claytonrcarter claytonrcarter force-pushed the split branch 3 times, most recently from 4ea0c44 to f94a071 Compare December 10, 2024 16:08
@claytonrcarter claytonrcarter changed the title feat: add split commant feat: add split command Dec 16, 2024
@claytonrcarter claytonrcarter force-pushed the split branch 3 times, most recently from d0966ba to a3f30c7 Compare December 20, 2024 07:33
@claytonrcarter claytonrcarter force-pushed the split branch 2 times, most recently from 455e652 to 6498777 Compare February 24, 2025 02:05
@claytonrcarter claytonrcarter force-pushed the split branch 2 times, most recently from bbfce1b to 6712829 Compare April 29, 2025 23:03
@claytonrcarter claytonrcarter marked this pull request as ready for review April 29, 2025 23:07
@claytonrcarter claytonrcarter force-pushed the split branch 6 times, most recently from 77d3d29 to 5f1491f Compare May 5, 2025 15:24
@claytonrcarter
Copy link
Collaborator Author

I pushed a fix for an issue that I had noticed for a while but couldn't reproduce: using split --discard on the current commit when the current commit was a checked out branch would leave the discarded changes in the index, making them look like they had been staged. My fix is to reset the index to the state of the newly split commit. That passes tests, but it's not clear to me how it will interact w/ something like the working copy snapshots. I think (hope?) those are applied later, but I'll play with this some more to see.

This will be used to circumvent the "autocheckout" setting in `git split` (in
the next commit), so that we can preserve the current detached/attached state.

FIXME is this really needed? Maybe we don't need to force-re-checkout?
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.

1 participant