-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(app): add Go Back and Last View #2799
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,14 +23,34 @@ func NewHistory(limit int) *History { | |
} | ||
} | ||
|
||
func (h *History) Pop() string { | ||
// Last returns the most recent history item | ||
func (h *History) Last() string { | ||
if h.Empty() { | ||
return "" | ||
} | ||
|
||
return h.commands[0] | ||
} | ||
|
||
// Pop removes the most recent history item and returns a bool if the list changed. | ||
// Optional argument specifies how many to remove from the history | ||
func (h *History) Pop(n ...int) bool { | ||
if len(h.commands) == 0 { | ||
return false | ||
} | ||
|
||
count := 1 | ||
if len(n) > 1 { | ||
// only one argument is expected | ||
return false | ||
} else if len(n) == 1 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are done here no need for the else clause. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also not keen on the vararg with only one option. Best to leave Pop as such and introduce PopN(n int) |
||
count = n[0] | ||
} | ||
|
||
h.commands = h.commands[count:] | ||
return true | ||
} | ||
|
||
// List returns the current command history. | ||
func (h *History) List() []string { | ||
return h.commands | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,6 +244,8 @@ func (a *App) bindKeys() { | |
tcell.KeyCtrlE: ui.NewSharedKeyAction("ToggleHeader", a.toggleHeaderCmd, false), | ||
tcell.KeyCtrlG: ui.NewSharedKeyAction("toggleCrumbs", a.toggleCrumbsCmd, false), | ||
ui.KeyHelp: ui.NewSharedKeyAction("Help", a.helpCmd, false), | ||
ui.KeyB: ui.NewSharedKeyAction("Go Back", a.previousView, false), | ||
ui.KeyDash: ui.NewSharedKeyAction("Last View", a.lastView, false), | ||
tcell.KeyCtrlA: ui.NewSharedKeyAction("Aliases", a.aliasCmd, false), | ||
tcell.KeyEnter: ui.NewKeyAction("Goto", a.gotoCmd, false), | ||
tcell.KeyCtrlC: ui.NewKeyAction("Quit", a.quitCmd, false), | ||
|
@@ -691,6 +693,54 @@ func (a *App) helpCmd(evt *tcell.EventKey) *tcell.EventKey { | |
return nil | ||
} | ||
|
||
// previousView returns to the view prior to the current one in the history | ||
func (a *App) previousView(evt *tcell.EventKey) *tcell.EventKey { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should really be named previousCmd to avoid confusion |
||
if evt != nil && evt.Rune() == rune(ui.KeyB) && a.Prompt().InCmdMode() { | ||
return evt | ||
} | ||
cmds := a.cmdHistory.List() | ||
if !(len(cmds) > 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A dialog here feels a bit heavy, I think best to use a flash for this instance. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Additionally all these should be on cmdHistory apis. ie the logic for toggle last view and fwd/backwd should be methods on the history and not view logic imho i.ie toggleLastCmd, prevCmd, nextCmd, etc.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Additionally if we surfaced a cmdHistory view all the necessary apis would be already there. |
||
dialog.ShowError( | ||
a.Styles.Dialog(), | ||
a.Content.Pages, | ||
"Can't go back any further") | ||
return evt | ||
} else { | ||
previousCmd := cmds[1] | ||
a.cmdHistory.Pop() | ||
a.gotoResource(previousCmd, "", true) | ||
a.ResetCmd() | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// lastView switches between the last view and this one a la `cd -` | ||
func (a *App) lastView(evt *tcell.EventKey) *tcell.EventKey { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above these are commands and not views since sub-views are not part of the cmd history. |
||
if evt != nil && evt.Rune() == ui.KeyDash && a.Prompt().InCmdMode() { | ||
return evt | ||
} | ||
cmds := a.cmdHistory.List() | ||
if !(len(cmds) > 1) { | ||
dialog.ShowError( | ||
a.Styles.Dialog(), | ||
a.Content.Pages, | ||
"No previous view to switch to") | ||
return evt | ||
} else { | ||
current := cmds[0] | ||
last := cmds[1] | ||
a.gotoResource(last, "", true) | ||
// remove current page and last page | ||
a.cmdHistory.Pop(2) | ||
// re add in opposite order | ||
a.cmdHistory.Push(current) | ||
a.cmdHistory.Push(last) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (a *App) aliasCmd(evt *tcell.EventKey) *tcell.EventKey { | ||
if a.Content.Top() != nil && a.Content.Top().Name() == aliasTitle { | ||
a.Content.Pop() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think 'b' is already in use in some views aka to run benchmarks ;(
Perhaps we can use '-' as the last 2 cmd toggle as you have here but then useuse '<' and '>' to nav the command history?