From 46e9946854086f170ec01f12daf075e197e420f7 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Wed, 23 Feb 2022 19:44:48 +1100 Subject: refactor credential handling --- pkg/cheatsheet/generate.go | 1 - pkg/gui/confirmation_panel.go | 21 +++++++ pkg/gui/context/context.go | 4 -- pkg/gui/context_config.go | 12 ---- pkg/gui/controllers/helpers/credentials_helper.go | 68 +++++++++++++++++++++++ pkg/gui/credentials_panel.go | 57 ------------------- pkg/gui/gui.go | 33 +++++------ pkg/gui/gui_common.go | 4 ++ pkg/gui/keybindings.go | 12 ---- pkg/gui/layout.go | 1 - pkg/gui/popup/popup_handler.go | 2 + pkg/gui/types/common.go | 9 +++ pkg/gui/view_helpers.go | 2 +- pkg/i18n/chinese.go | 1 - pkg/i18n/dutch.go | 1 - pkg/i18n/english.go | 2 - 16 files changed, 118 insertions(+), 112 deletions(-) create mode 100644 pkg/gui/controllers/helpers/credentials_helper.go delete mode 100644 pkg/gui/credentials_panel.go diff --git a/pkg/cheatsheet/generate.go b/pkg/cheatsheet/generate.go index 47de22a40..804cb6b45 100644 --- a/pkg/cheatsheet/generate.go +++ b/pkg/cheatsheet/generate.go @@ -87,7 +87,6 @@ func localisedTitle(mApp *app.App, str string) string { "commitMessage": tr.CommitMessageTitle, "commits": tr.CommitsTitle, "confirmation": tr.ConfirmationTitle, - "credentials": tr.CredentialsTitle, "information": tr.InformationTitle, "main": tr.MainTitle, "patchBuilding": tr.PatchBuildingTitle, diff --git a/pkg/gui/confirmation_panel.go b/pkg/gui/confirmation_panel.go index f1eff4849..0a61c50b5 100644 --- a/pkg/gui/confirmation_panel.go +++ b/pkg/gui/confirmation_panel.go @@ -120,6 +120,7 @@ func (gui *Gui) prepareConfirmationPanel( hasLoader bool, findSuggestionsFunc func(string) []*types.Suggestion, editable bool, + mask bool, ) error { x0, y0, x1, y1 := gui.getConfirmationPanelDimensions(true, prompt) // calling SetView on an existing view returns the same view, so I'm not bothering @@ -136,6 +137,7 @@ func (gui *Gui) prepareConfirmationPanel( // for now we do not support wrapping in our editor gui.Views.Confirmation.Wrap = !editable gui.Views.Confirmation.FgColor = theme.GocuiDefaultTextColor + gui.Views.Confirmation.Mask = runeForMask(mask) gui.findSuggestions = findSuggestionsFunc if findSuggestionsFunc != nil { @@ -154,7 +156,25 @@ func (gui *Gui) prepareConfirmationPanel( return nil } +func runeForMask(mask bool) rune { + if mask { + return '*' + } + return 0 +} + func (gui *Gui) createPopupPanel(opts types.CreatePopupPanelOpts) error { + // if a popup panel already appears we must ignore this current one. This is + // not great but it prevents lost state. The proper solution is to have a stack of + // popups. We could have a queue of types.CreatePopupPanelOpts so that if you + // close a popup and there's another one in the queue we show that. + // One important popup we don't want to interrupt is the credentials popup + // or a process might get stuck waiting on user input. + if gui.currentContext().GetKey() == context.CONFIRMATION_CONTEXT_KEY { + gui.Log.Error("ignoring create popup panel because a popup panel is already open") + return nil + } + // remove any previous keybindings gui.clearConfirmationViewKeyBindings() @@ -164,6 +184,7 @@ func (gui *Gui) createPopupPanel(opts types.CreatePopupPanelOpts) error { opts.HasLoader, opts.FindSuggestionsFunc, opts.Editable, + opts.Mask, ) if err != nil { return err diff --git a/pkg/gui/context/context.go b/pkg/gui/context/context.go index f57cb507d..add336cfd 100644 --- a/pkg/gui/context/context.go +++ b/pkg/gui/context/context.go @@ -24,7 +24,6 @@ const ( MAIN_PATCH_BUILDING_CONTEXT_KEY types.ContextKey = "patchBuilding" MAIN_STAGING_CONTEXT_KEY types.ContextKey = "staging" MENU_CONTEXT_KEY types.ContextKey = "menu" - CREDENTIALS_CONTEXT_KEY types.ContextKey = "credentials" CONFIRMATION_CONTEXT_KEY types.ContextKey = "confirmation" SEARCH_CONTEXT_KEY types.ContextKey = "search" COMMIT_MESSAGE_CONTEXT_KEY types.ContextKey = "commitMessage" @@ -51,7 +50,6 @@ var AllContextKeys = []types.ContextKey{ MAIN_PATCH_BUILDING_CONTEXT_KEY, MAIN_STAGING_CONTEXT_KEY, // not focusable for secondary view MENU_CONTEXT_KEY, - CREDENTIALS_CONTEXT_KEY, CONFIRMATION_CONTEXT_KEY, SEARCH_CONTEXT_KEY, COMMIT_MESSAGE_CONTEXT_KEY, @@ -80,7 +78,6 @@ type ContextTree struct { Staging types.Context PatchBuilding types.Context Merging types.Context - Credentials types.Context Confirmation types.Context CommitMessage types.Context Search types.Context @@ -103,7 +100,6 @@ func (self *ContextTree) Flatten() []types.Context { self.Stash, self.Menu, self.Confirmation, - self.Credentials, self.CommitMessage, self.Normal, self.Staging, diff --git a/pkg/gui/context_config.go b/pkg/gui/context_config.go index 348c5b21b..bc9df8ba4 100644 --- a/pkg/gui/context_config.go +++ b/pkg/gui/context_config.go @@ -114,18 +114,6 @@ func (gui *Gui) contextTree() *context.ContextTree { OnFocus: OnFocusWrapper(func() error { return gui.renderConflictsWithLock(true) }), }, ), - Credentials: context.NewSimpleContext( - context.NewBaseContext(context.NewBaseContextOpts{ - Kind: types.PERSISTENT_POPUP, - ViewName: "credentials", - WindowName: "credentials", - Key: context.CREDENTIALS_CONTEXT_KEY, - Focusable: true, - }), - context.ContextCallbackOpts{ - OnFocus: OnFocusWrapper(gui.handleAskFocused), - }, - ), Confirmation: context.NewSimpleContext( context.NewBaseContext(context.NewBaseContextOpts{ Kind: types.TEMPORARY_POPUP, diff --git a/pkg/gui/controllers/helpers/credentials_helper.go b/pkg/gui/controllers/helpers/credentials_helper.go new file mode 100644 index 000000000..9da3a46a2 --- /dev/null +++ b/pkg/gui/controllers/helpers/credentials_helper.go @@ -0,0 +1,68 @@ +package helpers + +import ( + "sync" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/gui/types" +) + +type CredentialsHelper struct { + c *types.HelperCommon +} + +func NewCredentialsHelper( + c *types.HelperCommon, +) *CredentialsHelper { + return &CredentialsHelper{ + c: c, + } +} + +// promptUserForCredential wait for a username, password or passphrase input from the credentials popup +func (self *CredentialsHelper) PromptUserForCredential(passOrUname oscommands.CredentialType) string { + waitGroup := sync.WaitGroup{} + waitGroup.Add(1) + + userInput := "" + + self.c.OnUIThread(func() error { + title, mask := self.getTitleAndMask(passOrUname) + + return self.c.Prompt(types.PromptOpts{ + Title: title, + Mask: mask, + HandleConfirm: func(input string) error { + userInput = input + + waitGroup.Done() + + return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + }, + HandleClose: func() error { + waitGroup.Done() + + return nil + }, + }) + }) + + // wait for username/passwords/passphrase input + waitGroup.Wait() + + return userInput + "\n" +} + +func (self *CredentialsHelper) getTitleAndMask(passOrUname oscommands.CredentialType) (string, bool) { + switch passOrUname { + case oscommands.Username: + return self.c.Tr.CredentialsUsername, false + case oscommands.Password: + return self.c.Tr.CredentialsPassword, true + case oscommands.Passphrase: + return self.c.Tr.CredentialsPassphrase, true + } + + // should never land here + panic("unexpected credential request") +} diff --git a/pkg/gui/credentials_panel.go b/pkg/gui/credentials_panel.go deleted file mode 100644 index ba654e0eb..000000000 --- a/pkg/gui/credentials_panel.go +++ /dev/null @@ -1,57 +0,0 @@ -package gui - -import ( - "strings" - - "github.com/jesseduffield/lazygit/pkg/commands/oscommands" - "github.com/jesseduffield/lazygit/pkg/gui/types" -) - -type credentials chan string - -// promptUserForCredential wait for a username, password or passphrase input from the credentials popup -func (gui *Gui) promptUserForCredential(passOrUname oscommands.CredentialType) string { - gui.credentials = make(chan string) - gui.OnUIThread(func() error { - credentialsView := gui.Views.Credentials - switch passOrUname { - case oscommands.Username: - credentialsView.Title = gui.c.Tr.CredentialsUsername - credentialsView.Mask = 0 - case oscommands.Password: - credentialsView.Title = gui.c.Tr.CredentialsPassword - credentialsView.Mask = '*' - case oscommands.Passphrase: - credentialsView.Title = gui.c.Tr.CredentialsPassphrase - credentialsView.Mask = '*' - } - - if err := gui.c.PushContext(gui.State.Contexts.Credentials); err != nil { - return err - } - - return nil - }) - - // wait for username/passwords/passphrase input - userInput := <-gui.credentials - return userInput + "\n" -} - -func (gui *Gui) handleSubmitCredential() error { - credentialsView := gui.Views.Credentials - message := strings.TrimSpace(credentialsView.TextArea.GetContent()) - gui.credentials <- message - credentialsView.ClearTextArea() - if err := gui.c.PopContext(); err != nil { - return err - } - - return gui.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) -} - -func (gui *Gui) handleCloseCredentialsView() error { - gui.Views.Credentials.ClearTextArea() - gui.credentials <- "" - return gui.c.PopContext() -} diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 511406e77..061c6e7ec 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -86,7 +86,6 @@ type Gui struct { Config config.AppConfigurer Updater *updates.Updater statusManager *statusManager - credentials credentials waitForIntro sync.WaitGroup fileWatcher *fileWatcher viewBufferManagerMap map[string]*tasks.ViewBufferManager @@ -247,7 +246,6 @@ type Views struct { Options *gocui.View Confirmation *gocui.View Menu *gocui.View - Credentials *gocui.View CommitMessage *gocui.View CommitFiles *gocui.View Information *gocui.View @@ -403,7 +401,6 @@ func initialViewContextMapping(contextTree *context.ContextTree) map[string]type "stash": contextTree.Stash, "menu": contextTree.Menu, "confirmation": contextTree.Confirmation, - "credentials": contextTree.Credentials, "commitMessage": contextTree.CommitMessage, "main": contextTree.Normal, "secondary": contextTree.Normal, @@ -448,17 +445,6 @@ func NewGui( InitialDir: initialDir, } - guiIO := oscommands.NewGuiIO( - cmn.Log, - gui.LogCommand, - gui.getCmdWriter, - gui.promptUserForCredential, - ) - - osCommand := oscommands.NewOSCommand(cmn, oscommands.GetPlatform(), guiIO) - - gui.os = osCommand - gui.watchFilesForChanges() gui.PopupHandler = popup.NewPopupHandler( @@ -475,6 +461,19 @@ func NewGui( guiCommon := &guiCommon{gui: gui, IPopupHandler: gui.PopupHandler} helperCommon := &types.HelperCommon{IGuiCommon: guiCommon, Common: cmn} + credentialsHelper := helpers.NewCredentialsHelper(helperCommon) + + guiIO := oscommands.NewGuiIO( + cmn.Log, + gui.LogCommand, + gui.getCmdWriter, + credentialsHelper.PromptUserForCredential, + ) + + osCommand := oscommands.NewOSCommand(cmn, oscommands.GetPlatform(), guiIO) + + gui.os = osCommand + // storing this stuff on the gui for now to ease refactoring // TODO: reset these controllers upon changing repos due to state changing gui.c = helperCommon @@ -751,7 +750,6 @@ func (gui *Gui) createAllViews() error { {viewPtr: &gui.Views.Search, name: "search"}, {viewPtr: &gui.Views.SearchPrefix, name: "searchPrefix"}, {viewPtr: &gui.Views.CommitMessage, name: "commitMessage"}, - {viewPtr: &gui.Views.Credentials, name: "credentials"}, {viewPtr: &gui.Views.Menu, name: "menu"}, {viewPtr: &gui.Views.Suggestions, name: "suggestions"}, {viewPtr: &gui.Views.Confirmation, name: "confirmation"}, @@ -825,11 +823,6 @@ func (gui *Gui) createAllViews() error { gui.Views.Confirmation.Visible = false - gui.Views.Credentials.Visible = false - gui.Views.Credentials.Title = gui.c.Tr.CredentialsUsername - gui.Views.Credentials.FgColor = theme.GocuiDefaultTextColor - gui.Views.Credentials.Editable = true - gui.Views.Suggestions.Visible = false gui.Views.Menu.Visible = false diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index 2f44ebbce..7d8354bf6 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -65,3 +65,7 @@ func (self *guiCommon) Render() { func (self *guiCommon) OpenSearch() { _ = self.gui.handleOpenSearch(self.gui.currentViewName()) } + +func (self *guiCommon) OnUIThread(f func() error) { + self.gui.OnUIThread(f) +} diff --git a/pkg/gui/keybindings.go b/pkg/gui/keybindings.go index 19eb86baf..7401f3416 100644 --- a/pkg/gui/keybindings.go +++ b/pkg/gui/keybindings.go @@ -430,18 +430,6 @@ func (self *Gui) GetInitialKeybindings() ([]*types.Binding, []*gocui.ViewMouseBi Handler: self.handleCopySelectedSideContextItemToClipboard, Description: self.c.Tr.LcCopyCommitShaToClipboard, }, - { - ViewName: "credentials", - Key: opts.GetKey(opts.Config.Universal.Confirm), - Modifier: gocui.ModNone, - Handler: self.handleSubmitCredential, - }, - { - ViewName: "credentials", - Key: opts.GetKey(opts.Config.Universal.Return), - Modifier: gocui.ModNone, - Handler: self.handleCloseCredentialsView, - }, { ViewName: "menu", Key: opts.GetKey(opts.Config.Universal.Return), diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index 097625f4c..350549311 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -224,7 +224,6 @@ func (gui *Gui) onInitialViewsCreation() error { gui.Views.Menu, gui.Views.Suggestions, gui.Views.Confirmation, - gui.Views.Credentials, // this guy will cover everything else when it appears gui.Views.Limit, diff --git a/pkg/gui/popup/popup_handler.go b/pkg/gui/popup/popup_handler.go index 20a7c8d80..c3681fbe6 100644 --- a/pkg/gui/popup/popup_handler.go +++ b/pkg/gui/popup/popup_handler.go @@ -109,7 +109,9 @@ func (self *RealPopupHandler) Prompt(opts types.PromptOpts) error { Prompt: opts.InitialContent, Editable: true, HandleConfirmPrompt: opts.HandleConfirm, + HandleClose: opts.HandleClose, FindSuggestionsFunc: opts.FindSuggestionsFunc, + Mask: opts.Mask, }) } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 650aa51eb..cb39c87b5 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -40,6 +40,11 @@ type IGuiCommon interface { GetAppState() *config.AppState SaveAppState() error + + // Runs the given function on the UI thread (this is for things like showing a popup asking a user for input). + // Only necessary to call if you're not already on the UI thread i.e. you're inside a goroutine. + // All controller handlers are executed on the UI thread. + OnUIThread(f func() error) } type IPopupHandler interface { @@ -73,6 +78,7 @@ type CreatePopupPanelOpts struct { HandlersManageFocus bool FindSuggestionsFunc func(string) []*Suggestion + Mask bool } type AskOpts struct { @@ -88,6 +94,9 @@ type PromptOpts struct { InitialContent string FindSuggestionsFunc func(string) []*Suggestion HandleConfirm func(string) error + // CAPTURE THIS + HandleClose func() error + Mask bool } type MenuItem struct { diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index 234be7a4c..496b99e7f 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -80,7 +80,7 @@ func (gui *Gui) globalOptionsMap() map[string]string { } func (gui *Gui) isPopupPanel(viewName string) bool { - return viewName == "commitMessage" || viewName == "credentials" || viewName == "confirmation" || viewName == "menu" + return viewName == "commitMessage" || viewName == "confirmation" || viewName == "menu" } func (gui *Gui) popupPanelFocused() bool { diff --git a/pkg/i18n/chinese.go b/pkg/i18n/chinese.go index 551f55750..9fa455d73 100644 --- a/pkg/i18n/chinese.go +++ b/pkg/i18n/chinese.go @@ -191,7 +191,6 @@ func chineseTranslationSet() TranslationSet { TagsTitle: "标签页面", MenuTitle: "菜单", RemotesTitle: "远程页面", - CredentialsTitle: "证书", RemoteBranchesTitle: "远程分支(在远程页面中)", PatchBuildingTitle: "构建补丁中", InformationTitle: "信息", diff --git a/pkg/i18n/dutch.go b/pkg/i18n/dutch.go index 147cf3c6e..c77321ae2 100644 --- a/pkg/i18n/dutch.go +++ b/pkg/i18n/dutch.go @@ -161,7 +161,6 @@ func dutchTranslationSet() TranslationSet { TagsTitle: "Tags Tabblad", MenuTitle: "Menu", RemotesTitle: "Remotes Tabblad", - CredentialsTitle: "Credentials", RemoteBranchesTitle: "Remote Branches (in Remotes tabblad)", PatchBuildingTitle: "Patch Bouwen", InformationTitle: "Informatie", diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 649b04fd9..aba4e9368 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -177,7 +177,6 @@ type TranslationSet struct { TagsTitle string MenuTitle string RemotesTitle string - CredentialsTitle string RemoteBranchesTitle string PatchBuildingTitle string InformationTitle string @@ -748,7 +747,6 @@ func EnglishTranslationSet() TranslationSet { TagsTitle: "Tags Tab", MenuTitle: "Menu", RemotesTitle: "Remotes Tab", - CredentialsTitle: "Credentials", RemoteBranchesTitle: "Remote Branches (in Remotes tab)", PatchBuildingTitle: "Patch Building", InformationTitle: "Information", -- cgit v1.2.3