Fix incorrect stash diff after rename #4213

brandondong · 2025-01-25T05:05:31Z

PR Description

Reproduction:

git init
git commit --allow-empty -m "first commit"
touch file1
git add .
git stash push -m "stash1"
touch file2
git add .
git stash push -m "stash2"
lazygit

Navigate to the Stash view and rename stash 1

Manually refreshing fixes the diff.

Please check if the PR fulfills these requirements

Cheatsheets are up-to-date (run go generate ./...)
Code has been formatted (see here)
Tests have been added/updated (see here for the integration test guide)
Text is internationalised (see here)
If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
Docs have been updated if necessary
You've read through your own file changes for silly mistakes etc

stefanhaller

Nice, looks good. Just two small remarks; feel free to ignore the first one, I don't think we need to do anything about it right now.

stefanhaller · 2025-01-25T18:04:20Z

pkg/gui/controllers/stash_controller.go

 			if err != nil {
+				_ = self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.STASH}})


At first I was wondering if this is really necessary, on the assumption that if the Rename fails for some reason, then nothing should be changed. However, looking at the implementation of the Rename function it is (theoretically) possible that the operation fails after the stash was dropped, and then the save fails. It is unlikely that this happens, and it would actually be kind of bad because then your stash is gone rather than renamed, but doing something about that is out of scope for this PR I guess.

Yeah I was wondering if the refresh on error was necessary too but saw that it was introduced that way: 11316b7#diff-f92e11c5c84ff4ce46066e55444ceb7f4de7825b815670f9265b7b4e73123f28. It does look like it's there to handle the case of a failed stash store.

stefanhaller · 2025-01-25T18:05:29Z

pkg/gui/controllers/stash_controller.go

 				return err
 			}
 			self.context().SetSelection(0) // Select the renamed stash
 			self.context().FocusLine()
+			_ = self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.STASH}})


Now that the error handling of the actual git operation is no longer in the way, we could change this to return self.c.Refresh(..., because why not. 😄

stefanhaller · 2025-02-06T08:56:49Z

I didn't realize that you force-pushed with a new version. We don't get discord notifications for either force-pushes or thumbs-up reactions, so next time it would be best to add an explicit comment in that case. Thanks! Merging.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.45.2` -> `v0.47.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary> ### [`v0.47.1`](https://github.com/jesseduffield/lazygit/releases/tag/v0.47.1) [Compare Source](jesseduffield/lazygit@v0.46.0...v0.47.1)  #### What's Changed ##### Maintenance ⚙️ - Fix race condition with reselecting the focused branch and rendering by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4268 **Full Changelog**: jesseduffield/lazygit@v0.47.0...v0.47.1 ### [`v0.46.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.46.0) [Compare Source](jesseduffield/lazygit@v0.45.2...v0.46.0) It's time for another Lazygit release! Thanks to all who contributed to this release. #### What's Changed ##### Enhancements 🔥 - Preserve pending commit message when closing/re-opening by [@AzraelSec](https://github.com/AzraelSec) in jesseduffield/lazygit#4191 - Swap position of checkout-commit and checkout-branch menu items by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4238 - In a previous release we had made it so that when you press space on a commit, the first option in the menu was to check out the corresponding branch, rather than the commit itself. We've made it so that now the first option is to check out the commit itself, for the sake of everybody's muscle memory. - Allow user to filter the files view to only show untracked files by [@jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4226 - Expose {{.SelectedCommitRange}} to custom commands by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4204 - Show background fetch status in bottom line by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4215 - Support copying tags to clipboard by [@brunofjesus](https://github.com/brunofjesus) in jesseduffield/lazygit#4218 - Add option to delete local and remote tag by [@AnvarU](https://github.com/AnvarU) in jesseduffield/lazygit#4217 - Show confirmation menu when trying to amend changes while there are conflicts by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4222 - Improve error reporting on config migration by [@kas2020-commits](https://github.com/kas2020-commits) in jesseduffield/lazygit#4210 - Show files filter status by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4230 - Improve contrast of highlighted search results by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4242 ##### Fixes 🔧 - Don't try killing processes if we already know the command finished by [@brandondong](https://github.com/brandondong) in jesseduffield/lazygit#4231 - We believe this was causing all sorts of problems for windows users. - Add '--' to 'git rev-list' to disambiguate branch name from path by [@efussi](https://github.com/efussi) in jesseduffield/lazygit#4185 - Fix adding blank line at end of commit message by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4182 - Fix checking out a different branch while pushing a branch for the first time by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4214 - Fix tag being truncated when copying to clipboard by [@brunofjesus](https://github.com/brunofjesus) in jesseduffield/lazygit#4232 - Disable staging and unstaging lines or hunks when the diff context size is 0 by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4235 - Fix incorrect stash diff after rename by [@brandondong](https://github.com/brandondong) in jesseduffield/lazygit#4213 - Fix possible crash when deleting a branch while filtering is active by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4195 - Fix pasting multi-line text into commit message panel by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4234 - fix: properly detect icon for BAZEL and WORKSPACE files by [@PeterCardenas](https://github.com/PeterCardenas) in jesseduffield/lazygit#4252 - Fix json schema for context of CustomCommand by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4255 ##### Maintenance ⚙️ - Bump go-deadlock version to fix crash with go 1.23 in debug mode by [@ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4248 - Fix auto-release workflow by [@jesseduffield](https://github.com/jesseduffield) in jesseduffield/lazygit#4265 ##### I18n 🌎 - Update translations; includes a new Portuguese translation by [@stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4250 #### New Contributors - [@efussi](https://github.com/efussi) made their first contribution in jesseduffield/lazygit#4185 - [@brunofjesus](https://github.com/brunofjesus) made their first contribution in jesseduffield/lazygit#4218 - [@AnvarU](https://github.com/AnvarU) made their first contribution in jesseduffield/lazygit#4217 - [@kas2020-commits](https://github.com/kas2020-commits) made their first contribution in jesseduffield/lazygit#4210 - [@ChrisMcD1](https://github.com/ChrisMcD1) made their first contribution in jesseduffield/lazygit#4248 - [@PeterCardenas](https://github.com/PeterCardenas) made their first contribution in jesseduffield/lazygit#4252 **Full Changelog**: jesseduffield/lazygit@v0.45.2...v0.46.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

stefanhaller reviewed Jan 25, 2025

View reviewed changes

brandondong force-pushed the stash_rename branch from 27cac55 to 66a60f3 Compare January 25, 2025 21:53

Fix incorrect stash diff after rename

02ca07a

stefanhaller force-pushed the stash_rename branch from 66a60f3 to 02ca07a Compare February 6, 2025 08:55

stefanhaller enabled auto-merge February 6, 2025 08:57

stefanhaller added the bug Something isn't working label Feb 6, 2025

stefanhaller merged commit 31e54ea into jesseduffield:master Feb 6, 2025
13 of 15 checks passed

brandondong deleted the stash_rename branch February 7, 2025 06:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix incorrect stash diff after rename #4213

Fix incorrect stash diff after rename #4213

brandondong commented Jan 25, 2025

stefanhaller left a comment

stefanhaller Jan 25, 2025

brandondong Jan 25, 2025

stefanhaller Jan 25, 2025

stefanhaller commented Feb 6, 2025

		if err != nil {
		_ = self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.STASH}})

Fix incorrect stash diff after rename #4213

Fix incorrect stash diff after rename #4213

Conversation

brandondong commented Jan 25, 2025

stefanhaller left a comment

Choose a reason for hiding this comment

stefanhaller Jan 25, 2025

Choose a reason for hiding this comment

brandondong Jan 25, 2025

Choose a reason for hiding this comment

stefanhaller Jan 25, 2025

Choose a reason for hiding this comment

stefanhaller commented Feb 6, 2025