derailed / k9s

🐶 Kubernetes CLI To Manage Your Clusters In Style!
https://k9scli.io
Apache License 2.0
26.62k stars 1.67k forks source link

Support invoking plugins beyond limited (19?) amount of keyboard shortcuts #2793

Open gberche-orange opened 2 months ago

gberche-orange commented 2 months ago




Is your feature request related to a problem? Please describe.

k9s is a great tool, and plugins are very successful at extending its scope. Plugins can only be invoked by shortcuts which are currently supporting patterns [a-z], Shift-[A-Z], Ctrl-[A-Z].

There is therefore currently support for 26*3=78 shortcuts total

To keep built-in features useable, plugins should avoid standard K9s shortcuts https://github.com/derailed/k9s/blob/350439b98553f23672f7ce0b650637d0afdd4104/README.md?plain=1#L551

Looking in k9s markdown docs files, I could not yet spot a full list of builtin shortcuts to avoid. Greping through the source code, I count 59 shortcuts used (see details below). This currently leaves users with 19 custom shortcuts.

Describe the solution you'd like

Add support for invoking plugins using an optional command such as :trace, specified as new commandAlias in the plugin.yaml. It would benefit from the completion, and be listed within the help page for easy discovery

Describe alternatives you've considered

Multi stroke shortcuts. This is complex however to avoid collision.

I looked at how other developer tools heavily relying on shortcuts are handling the problem:

In intellij ide:

In vim from https://stackoverflow.com/questions/5390683/vim-available-shortcuts

Type :help index to see the mappings (shortcuts as you name them) and commands defined by vim itself. Type :map to see the mappings defined by your vimrc and plugins. Type :verbose map to know where each mapping was defined. Also :help map-listing to check what's displayed,

Additional context

We currently have 17 shortcuts in our plugins.yaml file, which is preventing us from adding more shortcuts

our custom shortcuts ``` cat k9s-plugins.yaml | yq .plugins[].shortCut | uniq -c | sort -nr #> 6 Shift-Z #> 4 Shift-T #> 4 Ctrl-L #> 2 Shift-S #> 1 x #> 1 v #> 1 t #> 1 Shift-Z #> 1 Shift-T #> 1 Shift-Q #> 1 Shift-K #> 1 Shift-I #> 1 Shift-H #> 1 Shift-D #> 1 s #> 1 p #> 1 g cat k9s-plugins.yaml | yq .plugins[].shortCut | uniq -c | sort -nr | wc -l #> 17 ```

Currently, here is our process to assign or reassign shortcuts, which we run when integrating new plugin or bumping k9s version

  1. enable debug logs k9 -l debug and check related logs such as

    WRN Plugins load failed: duplicate plugin key found for "Shift-C" in "trace" INF Action "Shift-S" has been overridden by hotkey in "shift-s"

  2. discover used keyboard shortcuts by builtin commands

Adhoc grep source code to discover which shortcuts are currently in use ```shell grep --color=always -r -E 'ui\.(Key[A-Z]|KeyShift[A-Z]|KeyCtrl[A-Z])[:, ]' | grep --color=always -vE '[:space]*//' | grep -vE '_test.go|keys_in_source.txt' | tee keys_in_source.txt #> internal/view/cronjob.go: ui.KeyT: ui.NewKeyAction("Trigger", c.triggerCmd, true), #> internal/view/cronjob.go: ui.KeyS: ui.NewKeyAction("Suspend/Resume", c.toggleSuspendCmd, true), #> internal/view/cronjob.go: ui.KeyShiftL: ui.NewKeyAction("Sort LastScheduled", c.GetTable().SortColCmd(lastScheduledCol, true), false), #> internal/view/node.go: ui.KeyC: ui.NewKeyActionWithOpts( #> internal/view/node.go: ui.KeyU: ui.NewKeyActionWithOpts( #> internal/view/node.go: ui.KeyR: ui.NewKeyActionWithOpts( #> internal/view/node.go: aa.Add(ui.KeyS, ui.NewKeyAction("Shell", n.sshCmd, true)) #> internal/view/node.go: ui.KeyY: ui.NewKeyAction(yamlAction, n.yamlCmd, true), #>[..] ``` ```shell grep --color=always --no-filename --only-matching -r -E '(ui|tcell)\.(Key[A-Z]|KeyShift[A-Z]|KeyCtrl[A-Z])[:, ]' | grep --color=always -vE '[:space]*//' | grep -vE '_test.go|keys_in_source.txt|used_keys.txt' | sed 's/[,:]//g'| sort | uniq -c | tee used_keys.txt cat used_keys.txt | wc -l #> 59 ``` Here is current output ``` 1 tcell.KeyCtrlA 1 tcell.KeyCtrlC 9 tcell.KeyCtrlD 2 tcell.KeyCtrlE 1 tcell.KeyCtrlF 1 tcell.KeyCtrlG 1 tcell.KeyCtrlK 3 tcell.KeyCtrlL 1 tcell.KeyCtrlP 2 tcell.KeyCtrlQ 2 tcell.KeyCtrlR 15 tcell.KeyCtrlS 2 tcell.KeyCtrlU 5 tcell.KeyCtrlW 1 tcell.KeyCtrlX 3 tcell.KeyCtrlZ 11 ui.KeyA 6 ui.KeyB 12 ui.KeyC 8 ui.KeyD 12 ui.KeyE 11 ui.KeyF 2 ui.KeyI 6 ui.KeyL 4 ui.KeyM 8 ui.KeyN 2 ui.KeyO 6 ui.KeyP 12 ui.KeyR 16 ui.KeyS 6 ui.KeyT 14 ui.KeyU 6 ui.KeyV 2 ui.KeyW 4 ui.KeyX 10 ui.KeyY 4 ui.KeyZ 33 ui.KeyShiftA 2 ui.KeyShiftB 16 ui.KeyShiftC 4 ui.KeyShiftD 1 ui.KeyShiftE 8 ui.KeyShiftF 3 ui.KeyShiftI 2 ui.KeyShiftJ 8 ui.KeyShiftK 10 ui.KeyShiftL 4 ui.KeyShiftM 17 ui.KeyShiftN 7 ui.KeyShiftO 6 ui.KeyShiftP 21 ui.KeyShiftR 17 ui.KeyShiftS 8 ui.KeyShiftT 6 ui.KeyShiftU 10 ui.KeyShiftV 1 ui.KeyShiftW 2 ui.KeyShiftX 2 ui.KeyShiftZ ```
derailed commented 1 month ago

@gberche-orange Interesting! Thank you for the research and input! I like the idea and I am aware of the current limitations. Currently command are just that ie a user specifies a command and a related view is displayed.

The issue with most plugins is that they need adhoc context based on what is currently selected. So perhaps we could use a naming convention as part of the command to fire off a plugin i.e something like p:trace so that the command interpreter can serialize the current state and make it available to the plugin environment.

is this inline with what you are proposing or am I missing it?

gberche-orange commented 3 weeks ago

thanks @derailed for the follow up !

My suggestion focuses on being able to invoke plugins with one or more aliases (such as ":trace" for the following flux trace plugin), just like for :pod or :cronjob, and suppporting completion for plugin aliases, and plugin aliases be listed within the help page for easy discovery. I likely have misused the term "command".

For instance, the flux trace plugin https://github.com/derailed/k9s/blob/35893ce49b941c8a2b8706a50cfc35a449572231/plugins/flux.yaml#L154-L162 would declare one or more aliases like below, so that it can be invoked by typing :trace or :fluxtrace on scoped resources (all in this example)

   trace: 
     shortCut: Shift-A 
     aliases: #used by less frequent plugins where all available shortcuts have been assigned to other plugins
        - trace
        - fluxtrace
     confirm: false 
     description: Flux trace 
...

I'm not familiar enough with k9s codebase to comment on whether it make sense to try to implement this feature through an actual command or rather specific logic in the "command interpreter" to invoke plugins that registered an alias, and whose scope is matching the currently selected resource.

So perhaps we could use a naming convention as part of the command to fire off a plugin i.e something like p:trace [...]

From a user point of view, is there a need for such naming convention ? Do users need to distinguish plugins that can be invoked by aliases (and hence require a naming convention), from regular commands ?

In terms of implementation, could instead the command interpreter check for all plugins that were registered to be invoked with an alias without requiring a naming convention ?