rename stash #2220

Ryooooooga · 2022-10-14T13:57:58Z

PR Description

same as:

customCommands:
  - key: r
    prompts:
      - key: Message
        type: input
        title: "Rename stash: {{.SelectedStashEntry.RefName}}"
        initialValue: "{{.SelectedStashEntry.Name}}"
    command: >-
      git stash drop {{.SelectedStashEntry.RefName}}
      | sed -E 's/.*\(([0-9a-f]+)\).*/\1/'
      | xargs git stash store -m {{.Form.Message | quote}}
    context: stash

Please check if the PR fulfills these requirements

Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.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)
Docs (specifically docs/Config.md) have been updated if necessary
You've read through your own file changes for silly mistakes etc

jesseduffield

Looking good :) Can we add an integration test as well?

pkg/commands/git_commands/stash.go

jesseduffield · 2022-10-14T15:39:07Z

pkg/gui/controllers/stash_controller.go

+		InitialContent: stashEntry.Name,
+		HandleConfirm: func(response string) error {
+			self.c.LogAction(self.c.Tr.Actions.RenameStash)
+			output, err := self.git.Stash.Drop(stashEntry.Index)


I would move this logic of this HandleConfirm callback into pkg/commands/git_commands/stash.go because it's quite specific to git. The LogAction and Refresh calls can stay here

Moved to stash.go.

jesseduffield · 2022-10-14T15:39:37Z

pkg/gui/controllers/stash_controller.go

+			stashShaPattern := regexp.MustCompile(`\(([0-9a-f]+)\)`)
+			matches := stashShaPattern.FindStringSubmatch(output)
+			stashSha := ""
+			if len(matches) > 1 {


in what situation would we not find a match? Worth leaving a comment explaining that

Normally, this regex is always matched, but added error handling

Ryooooooga · 2022-10-15T02:35:51Z

pkg/commands/git_commands/stash.go

+	stashShaPattern := regexp.MustCompile(`\(([0-9a-f]+)\)`)
+	matches := stashShaPattern.FindStringSubmatch(output)
+	if len(matches) <= 1 {
+		return errors.New("Output of `git stash drop` is invalid") // Usually this error does not occur


not internationalized because this error never occurs

Ryooooooga · 2022-10-15T03:02:35Z

added an integration test

jesseduffield · 2022-10-15T16:57:21Z

pkg/commands/git_commands/stash.go

+
+	// `output` is in the following format:
+	// Dropped refs/stash@{0} (f0d0f20f2f61ffd6d6bfe0752deffa38845a3edd)
+	stashShaPattern := regexp.MustCompile(`\(([0-9a-f]+)\)`)


Just tested this out locally, and it works, but I think we should change the approach a bit. You can use git rev-parse stash@{<index>} to obtain the hash, and I think we ought to do that for a couple of reasons:

git rev-parse's output is never going to change, but I can imagine the output of git stash drop changing. If git stash drop does change its output, we'll have just lost the user's stash which is going to make them very mad.

it spares us the complexity of regexes

So I propose we run three commands:

git rev-parse stash@{<index>} to get the hash

git stash drop stash@{<index>} to drop it

git stash store <hash> to store the renamed stash entry

changed that way:)

jesseduffield · 2022-10-15T16:57:29Z

pkg/integration/tests/stash/rename.go

+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var Rename = NewIntegrationTest(NewIntegrationTestArgs{


jesseduffield

one thing

Co-authored-by: Jesse Duffield <jessedduffield@gmail.com>

jesseduffield · 2022-10-16T04:26:14Z

pkg/integration/tests/stash/rename.go

+		shell.
+			EmptyCommit("blah").
+			CreateFileAndAdd("foo", "change to stash").
+			StashWithMessage("bar")


sorry for missing this the first time but could we add another stash entry beforehand so that when we assert that the text has been updated after renaming the stash, we're also implicitly verifying that the cursor has remained on the stash entry?

jesseduffield

only one more thing I promise ;)

Ryooooooga · 2022-10-16T07:26:42Z

updated

jesseduffield · 2022-10-16T16:56:37Z

Awesome work @Ryooooooga !

Ryooooooga force-pushed the rename-stash branch 2 times, most recently from 1dda24b to bcf8e9f Compare October 14, 2022 14:00

Ryooooooga changed the title ~~feat: add rename stash~~ rename stash Oct 14, 2022

Ryooooooga force-pushed the rename-stash branch from bcf8e9f to 451115f Compare October 14, 2022 14:19

jesseduffield requested changes Oct 14, 2022

View reviewed changes

Ryooooooga force-pushed the rename-stash branch 2 times, most recently from ff3b460 to f2c11c5 Compare October 15, 2022 02:29

Ryooooooga commented Oct 15, 2022

View reviewed changes

Ryooooooga force-pushed the rename-stash branch from d465de5 to a6fe332 Compare October 15, 2022 02:59

Ryooooooga requested a review from jesseduffield October 15, 2022 03:02

jesseduffield reviewed Oct 15, 2022

View reviewed changes

jesseduffield requested changes Oct 15, 2022

View reviewed changes

Ryooooooga and others added 3 commits October 16, 2022 09:12

feat: add rename stash

11316b7

chore: remove unnecessary space

8a9eefa

Co-authored-by: Jesse Duffield <jessedduffield@gmail.com>

chore: refactor rename stash

eceb3a5

Ryooooooga force-pushed the rename-stash branch from a6fe332 to 3af2686 Compare October 16, 2022 00:22

Ryooooooga requested a review from jesseduffield October 16, 2022 00:26

Ryooooooga force-pushed the rename-stash branch from 3af2686 to 3bf2e8f Compare October 16, 2022 00:27

Ryooooooga added 2 commits October 16, 2022 09:30

test: add an integration test for rename stash

e78e829

chore: refactor rename stash

3103398

Ryooooooga force-pushed the rename-stash branch from 3bf2e8f to 3103398 Compare October 16, 2022 00:30

jesseduffield reviewed Oct 16, 2022

View reviewed changes

jesseduffield requested changes Oct 16, 2022

View reviewed changes

test: update stash/rename integration test

2ec0b67

Ryooooooga requested a review from jesseduffield October 16, 2022 07:26

jesseduffield merged commit 36c6462 into jesseduffield:master Oct 16, 2022

Ryooooooga deleted the rename-stash branch October 16, 2022 22:41

strboul mentioned this pull request Dec 13, 2022

Rename stash messages #2189

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rename stash #2220

rename stash #2220

Ryooooooga commented Oct 14, 2022 •

edited

Loading

jesseduffield left a comment

jesseduffield Oct 14, 2022

Ryooooooga Oct 15, 2022

jesseduffield Oct 14, 2022

Ryooooooga Oct 15, 2022

Ryooooooga Oct 15, 2022

Ryooooooga commented Oct 15, 2022

jesseduffield Oct 15, 2022

Ryooooooga Oct 16, 2022

jesseduffield Oct 15, 2022

jesseduffield left a comment

jesseduffield Oct 16, 2022

jesseduffield left a comment

Ryooooooga commented Oct 16, 2022

jesseduffield commented Oct 16, 2022

rename stash #2220

rename stash #2220

Conversation

Ryooooooga commented Oct 14, 2022 • edited Loading

jesseduffield left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ryooooooga commented Oct 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jesseduffield left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jesseduffield left a comment

Choose a reason for hiding this comment

Ryooooooga commented Oct 16, 2022

jesseduffield commented Oct 16, 2022

Ryooooooga commented Oct 14, 2022 •

edited

Loading