altsem / gitu

A TUI Git client inspired by Magit
MIT License
1.7k stars 88 forks source link

fix: stash working tree should work when everything/nothing is staged #103

Closed golden-expiriensu closed 4 months ago

golden-expiriensu commented 4 months ago

Stash working tree is running 3 commands so there is some edge cases which we should take into account:

  1. If everything is staged don't do anything
  2. If nothing is staged don't try to save index in the stash and then later pop it (either errors or pops non-related stashes)

Addresses: https://github.com/altsem/gitu/pull/100#issuecomment-2025832482

altsem commented 4 months ago

Trying to do some digging if there's a better way to handle all this. There's multiple places where things might fail. What if there's an ongoing merge etc?

If only git had a git stash push --worktree :man_facepalming:. The git stash pop 1 is scary because it assumes nothing went wrong.

One idea is to return an error code from run_cmd and react to it.

I found this function in the git source code: https://github.com/git/git/blob/d6fd04375f9196f8b203d442f235bd96a1a068cc/builtin/stash.c#L1288

altsem commented 4 months ago

The C function does something like this I believe (123 would be the pid):

$ git diff --name-only | GIT_INDEX_FILE=.git/index.stash.123 git update-index --ignore-skip-worktree-entries --add --remove --stdin

And then it writes a "stash commit" somehow after. Although when I try this, then I'd see a lot more things staged than just the single file i had in my diff.

$ GIT_INDEX_FILE=.git/index.stash.123 git status
On branch fix/stash-working-tree-edge-cases
Your branch is up to date with 'golden-expiriensu/fix/stash-working-tree-edge-cases'.

Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
    deleted:    .cliffignore
    deleted:    .envrc
    deleted:    .github/dependabot.yml
    deleted:    .github/workflows/cachix.yml
...

Git creates this .git/index.stash.123 file temporarily.

altsem commented 4 months ago

I made a separate PR that makes it so that run_cmd breaks execution flow if the return code is not 0. This, in combination with your changes here should be pretty solid. At worst, users will end up with the temporary stash I think.

https://github.com/altsem/gitu/pull/105

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.41%. Comparing base (699b133) to head (55c2042). Report is 2 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #103 +/- ## ========================================== + Coverage 88.21% 88.41% +0.20% ========================================== Files 41 41 Lines 3885 3954 +69 ========================================== + Hits 3427 3496 +69 Misses 458 458 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

altsem commented 4 months ago

Looked good! :)