feat(app): add Go Back and Last View #2799

tyzbit · 2024-07-19T03:52:34Z

Go Back walks back through the history until at the oldest saved view

Last View switches between the current and previous views like how "cd -" works

tyzbit · 2024-08-05T14:51:18Z

Features from these requests not included are:

It is not possible to go forward again through the history. If you go back, the very last view you were on is removed from the history. - doesn't remove anything from history, just rearranges it.
There is no overview to let you see the history and pick a specific view from it.

This PR implements the most powerful parts of the feature requests. Better navigation and richer history could be added later while keeping this functionality intact.

Supersedes #1090

internal/view/app.go

KevinGimbel · 2024-08-07T06:56:11Z

Tested the feature locally and I have some questions:

When I look at containers (Select Pod -> Enter) I cannot go back to the pods with - or b
Same when using Describe (d)

Is this intended? I assume these "views" are somehow different from the generic Object views (Pod, StatefulSet, Deployment, ...)?

In general I really like this feature! Especially - seems useful! Thank you @tyzbit!

tyzbit · 2024-08-07T13:33:44Z

I would defer to someone more familiar with the codebase but Views seem to correlate mostly with Kubernetes resources (with the exception of contexts). Maybe the most accurate way to define them is the screens you get when you use a command (e.g. :pods).

When you dive into a resource, you get a trail of breadcrumbs at the bottom.

For demonstration, I went to pods, namespaces and then deployments and then selected a Deployment and hit Enter to go to the Pod, then hit enter again to go to the container, then I checked the cmdHistory:

k9s doesn't add breadcrumb-related navigation to the command history at the present time.

In terms of addressing the difference in expectations, maybe I could update the documentation and help to clarify that Go Back and Last View only work with k8s resources and contexts?

KevinGimbel · 2024-08-08T08:53:31Z

In terms of addressing the difference in expectations, maybe I could update the documentation and help to clarify that Go Back and Last View only work with k8s resources and contexts?

I think that's a good idea, just to make it clear what to expect from the feature 👍

tyzbit · 2024-08-09T15:12:48Z

I updated the README.

KevinGimbel · 2024-08-12T07:25:55Z

Thanks!

One tiny remark:

Your commit changed all * to - for lists - I assume some plugin or editor configuration on your side is making these changes? It makes reviewing the actual code changes hard, and I wonder if future changes will revert this due to different editor configs.

My 2ct would be to revert these style-changes so only the new documentation is added, but I guess @derailed has the final call here :)

Go Back walks back through the history until at the oldest saved view Last View switches between the current and previous views like how "cd -" works

tyzbit · 2024-08-12T13:59:08Z

@KevinGimbel those formatting changes were not intentional, I've changed my settings so it doesn't happen again. While I was at it, I squashed my commits.

KevinGimbel · 2024-08-13T07:48:19Z

@tyzbit thank you! Looks good to me now. @derailed in my opinion this could be added to the next released :)

derailed

@tyzbit Nice work! Good concept. Just a few items to clean up and simplify.

derailed · 2024-08-15T23:27:36Z

README.md

@@ -355,6 +355,8 @@ K9s uses aliases to navigate most K8s resources.
 | To view and switch to another Kubernetes context (Pod view)                     | `:`ctx⏎                       |                                                                        |
 | To view and switch directly to another Kubernetes context (Last used view)      | `:`ctx context-name⏎          |                                                                        |
 | To view and switch to another Kubernetes namespace                              | `:`ns⏎                        |                                                                        |
+| To switch between the last active view/command (like how "cd -" works)          | `-`                           | Navigation that adds breadcrumbs to the bottom are not commands        |
+| To go back through previous views/commands                                      | `b`                           | Same as above                                                          |


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?

derailed · 2024-08-15T23:28:59Z

internal/view/app.go

@@ -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 {


This should really be named previousCmd to avoid confusion

derailed · 2024-08-15T23:32:03Z

internal/view/app.go

+		return evt
+	}
+	cmds := a.cmdHistory.List()
+	if !(len(cmds) > 1) {


A dialog here feels a bit heavy, I think best to use a flash for this instance.
Also not keen on !(len(cmds) > 1. This is len(cmds) <=1 return.
But... as noted below better to defer these calls to cmdHistory.

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..

Additionally if we surfaced a cmdHistory view all the necessary apis would be already there.

derailed · 2024-08-15T23:36:19Z

internal/view/app.go

+}
+
+// lastView switches between the last view and this one a la `cd -`
+func (a *App) lastView(evt *tcell.EventKey) *tcell.EventKey {


As above these are commands and not views since sub-views are not part of the cmd history.

derailed · 2024-08-15T23:40:09Z

internal/model/history.go

+	if len(n) > 1 {
+		// only one argument is expected
+		return false
+	} else if len(n) == 1 {


You are done here no need for the else clause.

Also not keen on the vararg with only one option. Best to leave Pop as such and introduce PopN(n int)

tyzbit · 2024-09-13T20:36:34Z

Thank you for the great feedback. I'm working on addressing the comments; some planned changes:

Navigate history with [ and ] instead of B (I prefer this over < and > because it is not necessary to hold shift)
Add a view to see your history visually and pick a specific point in the history to go to. I think this will aid greatly in training users in using the new features.
cmdHistory will keep a more accurate history of your commands such as saving the last command used as well as the current command (therefore duplicates are possible and the assumption that history[0] is the current command will be broken, requiring some other updates)

tyzbit changed the title ~~feat(app): add Previous View and Go Back~~ feat(app): add Go Back and Last View Jul 19, 2024

tyzbit force-pushed the tyzbit/quick-return branch 2 times, most recently from be13aa3 to f6bff90 Compare July 19, 2024 04:07

tyzbit marked this pull request as draft July 19, 2024 15:33

tyzbit marked this pull request as ready for review July 19, 2024 15:49

tyzbit marked this pull request as draft July 19, 2024 18:28

tyzbit marked this pull request as ready for review July 19, 2024 20:31

KevinGimbel reviewed Aug 6, 2024

View reviewed changes

internal/view/app.go Outdated Show resolved Hide resolved

internal/view/app.go Outdated Show resolved Hide resolved

tyzbit force-pushed the tyzbit/quick-return branch from 7fbfca2 to c29dd8e Compare August 12, 2024 13:40

feat(app): add Go Back and Last View

8652c78

Go Back walks back through the history until at the oldest saved view Last View switches between the current and previous views like how "cd -" works

tyzbit force-pushed the tyzbit/quick-return branch from c29dd8e to 8652c78 Compare August 12, 2024 13:58

derailed requested changes Aug 15, 2024

View reviewed changes

derailed added enhancement New feature or request question Further information is requested labels Aug 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(app): add Go Back and Last View #2799

feat(app): add Go Back and Last View #2799

tyzbit commented Jul 19, 2024

tyzbit commented Aug 5, 2024

KevinGimbel commented Aug 7, 2024

tyzbit commented Aug 7, 2024

KevinGimbel commented Aug 8, 2024

tyzbit commented Aug 9, 2024

KevinGimbel commented Aug 12, 2024

tyzbit commented Aug 12, 2024

KevinGimbel commented Aug 13, 2024

derailed left a comment

derailed Aug 15, 2024

derailed Aug 15, 2024

derailed Aug 15, 2024

derailed Aug 15, 2024

derailed Aug 15, 2024

derailed Aug 15, 2024

derailed Aug 15, 2024

derailed Aug 15, 2024

tyzbit commented Sep 13, 2024

feat(app): add Go Back and Last View #2799

Are you sure you want to change the base?

feat(app): add Go Back and Last View #2799

Conversation

tyzbit commented Jul 19, 2024

tyzbit commented Aug 5, 2024

KevinGimbel commented Aug 7, 2024

tyzbit commented Aug 7, 2024

KevinGimbel commented Aug 8, 2024

tyzbit commented Aug 9, 2024

KevinGimbel commented Aug 12, 2024

tyzbit commented Aug 12, 2024

KevinGimbel commented Aug 13, 2024

derailed 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tyzbit commented Sep 13, 2024