Show loader when rebasing #2851

KarlHeitmann · 2023-07-31T01:22:27Z

PR Description

Solves issue #1488 and also #2610 . Shows WithLoaderPanel window when a rebase simple is make.

I tested it with a big repository I have, and switched rebased from one commit to the other.

If am missing anything just let me know so I can fix it.

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

Couple comments :)

jesseduffield · 2023-07-31T02:40:18Z

pkg/i18n/english.go

@@ -795,6 +796,7 @@ func EnglishTranslationSet() TranslationSet {
 		NoChangedFiles:                      "No changed files",
 		PullWait:                            "Pulling...",
 		PushWait:                            "Pushing...",
+		RebaseWait:                          "Rebasing...",


The fields ending in 'Wait' are for popup panels, but for rebases I don't want a popup. Instead we want to add a new field under the Actions field for this and have the value be Rebasing (no ...)

jesseduffield · 2023-07-31T02:40:40Z

pkg/gui/controllers/helpers/merge_and_rebase_helper.go

@@ -224,8 +225,10 @@ func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error {
 			Key:   's',
 			OnPress: func() error {
 				self.c.LogAction(self.c.Tr.Actions.RebaseBranch)
-				err := self.c.Git().Rebase.RebaseBranch(ref)
-				return self.CheckMergeOrRebase(err)
+				return self.c.WithLoaderPanel(self.c.Tr.RebaseWait, func(task gocui.Task) error {


Let's use WithWaitingStatus instead as that's the pattern we use for things that don't involve talking to a remote.

EDIT: Just realised I said to use WithLoaderPanel in the issue. My bad! Let's use WithWaitingStatus instead

I think I've read in one of your comments in other issue to use something different that WithWaitingStatus , but I was lazy to search it again. Now I have changed it, it is a spinner at the left bottom corner of the UI.

KarlHeitmann · 2023-07-31T04:52:48Z

Submitted changes, please let me know if there is something more I can do there.

jesseduffield · 2023-07-31T05:49:41Z

pkg/i18n/english.go

@@ -1323,6 +1324,7 @@ func EnglishTranslationSet() TranslationSet {
 			ForceCheckoutBranch:               "Force checkout branch",
 			DeleteBranch:                      "Delete branch",
 			Merge:                             "Merge",
+			Rebase:                            "Rebasing",


My apologies: looks like Actions is the wrong place to put this (it's using present tense and everything else is tenseless). In fact, we already have a RebasingStatus to use! So let's just use the existing one

jesseduffield

One more thing

KarlHeitmann · 2023-07-31T23:34:24Z

One more thing

I think I've got it now @jesseduffield , I rebased the branch, solved the conflicts one by one and verified in each commit the spinner appeared. I don't know if you are going to squash the changes later or not, but that's what I did.

Let me know if there are more things to fix or if there is something else missing.

Best

jesseduffield · 2023-07-31T23:38:10Z

LGTM. Can you squash the commits?

KarlHeitmann · 2023-07-31T23:49:24Z

Ready @jesseduffield !

jesseduffield · 2023-07-31T23:58:20Z

Great work @KarlHeitmann

jesseduffield reviewed Jul 31, 2023

View reviewed changes

jesseduffield requested changes Jul 31, 2023

View reviewed changes

jesseduffield added the enhancement New feature or request label Jul 31, 2023

jesseduffield changed the title ~~Wraps rebase function with WithLoaderPanel to show spinner when rebasing~~ Show loader when rebasing Jul 31, 2023

KarlHeitmann force-pushed the issue_1488 branch from 724a20a to d838088 Compare July 31, 2023 23:32

KarlHeitmann requested a review from jesseduffield July 31, 2023 23:34

Wraps rebase func with WithWaitingStatus to show loader when rebasing

c7e6a73

KarlHeitmann force-pushed the issue_1488 branch from d838088 to c7e6a73 Compare July 31, 2023 23:48

jesseduffield merged commit b44d0c4 into jesseduffield:master Jul 31, 2023

topher200 mentioned this pull request Aug 11, 2023

Missing UI feedback when rebasing onto another branch #1488

Closed

stefanhaller mentioned this pull request Jan 17, 2024

Show progress indicator for more operations, e.g. "discard changes" #2610

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show loader when rebasing #2851

Show loader when rebasing #2851

KarlHeitmann commented Jul 31, 2023 •

edited

Loading

jesseduffield left a comment

jesseduffield Jul 31, 2023

jesseduffield Jul 31, 2023 •

edited

Loading

KarlHeitmann Jul 31, 2023

KarlHeitmann commented Jul 31, 2023

jesseduffield Jul 31, 2023

jesseduffield left a comment

KarlHeitmann commented Jul 31, 2023

jesseduffield commented Jul 31, 2023

KarlHeitmann commented Jul 31, 2023

jesseduffield commented Jul 31, 2023

Show loader when rebasing #2851

Show loader when rebasing #2851

Conversation

KarlHeitmann commented Jul 31, 2023 • edited Loading

jesseduffield left a comment

Choose a reason for hiding this comment

jesseduffield Jul 31, 2023

Choose a reason for hiding this comment

jesseduffield Jul 31, 2023 • edited Loading

Choose a reason for hiding this comment

KarlHeitmann Jul 31, 2023

Choose a reason for hiding this comment

KarlHeitmann commented Jul 31, 2023

jesseduffield Jul 31, 2023

Choose a reason for hiding this comment

jesseduffield left a comment

Choose a reason for hiding this comment

KarlHeitmann commented Jul 31, 2023

jesseduffield commented Jul 31, 2023

KarlHeitmann commented Jul 31, 2023

jesseduffield commented Jul 31, 2023

KarlHeitmann commented Jul 31, 2023 •

edited

Loading

jesseduffield Jul 31, 2023 •

edited

Loading