[feature request] bind `q` to exit resource details buffer (`quit-window` and `kill-buffer`)

[CsBigDataHub]

press q to exit from any resource details buffer that is created when enter is pressed.

Essentially running quit-window or kill-buffer

[d1egoaz]

would binding q be even possible?

as https://www.gnu.org/software/emacs/manual/html_node/elisp/Keymaps-and-Minor-Modes.html

Minor modes may bind commands to key sequences consisting of C-c followed by a punctuation character.

There is an existing minor mode when viewing the yaml details of a resource,

we could add something like:

(define-minor-mode kubel-yaml-editing-mode
  "Kubel Yaml editing mode.
Use C-c C-c to kubectl apply the current yaml buffer."
  :init-value nil
  :keymap (let ((map (make-sparse-keymap)))
            (define-key map (kbd "C-c C-c") #'kubel-apply)
            (define-key map (kbd "C-c q") #'kill-current-buffer)
            map))

Binding q does not work, I'm not sure why yet, maybe because it's a minor mode and other modes take precedence

            (define-key map (kbd "q") #'kill-current-buffer)

[d1egoaz]

Huh! q worked for me when I was in edit mode (I'm using evil).

Which is undesirable if someone wanted to type the letter q :)

Anyhow, the following worked for me using evil. Not sure if we'd like to add this to kube-evil.el as it changes the default action of q, usually used to define macros in evil.

Or maybe just add it as a comment/note in the readme/wiki.

(evil-define-key 'normal 'kubel-yaml-editing-mode "q" #'kill-current-buffer)

[CsBigDataHub]

@d1egoaz kubernetes.el package has this functionality.

Check this out - https://github.com/kubernetes-el/kubernetes-el/blob/31191bccf759931c2a7f8fe179bd36d0c70de6da/kubernetes-utils.el#L80

https://github.com/kubernetes-el/kubernetes-el/blob/ea81874f0490cea310b09c57718aa0e5c83d578b/kubernetes-evil.el#L56

https://github.com/kubernetes-el/kubernetes-el/blob/76586e13087970d57ee8414c8be950fd7e9e2dc5/kubernetes-modes.el#L45

[Kyrremann]

I've been looking into this today, and what I've managed to do is to override the kubel-get-resource-details function, and add the readonly-parameter to the kubel--exec function.

I also had to modify the function more, cause I never managed to set the optional describe-paramter. Also persoanlly I mostly want to describer the resource, not just get-it. For me it would be logical to describe, if you haven't specified `-y'.

Another thing I have problem with is that kubel-output always is set, so I can't if on that value, and use that to check if you need to get the resource and then call kubel-yaml-editing-mode.

This code kinda works (it doesn't without commenting out the last if)

(defun kubel-describe-resource-details-popup (&optional describe)
  "Get the details of the resource under the cursor

 DESCRIBE is the optional param to describe instead of get."
  (interactive "P")
  (let* ((resource (kubel--get-resource-under-cursor))
         (process-name (format "kubel - %s - %s" kubel-resource resource)))
		(setq kubel-output "") ; this is just because I don't know why it's always set
		(if (or (string-equal kubel-output "yaml") (transient-args 'kubel-describe-popup))
				(kubel--exec process-name (list "get" kubel-resource (kubel--get-resource-under-cursor) "-o" "yaml"))
			(kubel--exec process-name (list "describe" kubel-resource (kubel--get-resource-under-cursor)) t))

    ;(if (or (string-equal kubel-output "yaml") (transient-args 'kubel-describe-popup))
	;			((yaml-mode)
	;			 (kubel-yaml-editing-mode)))
    (goto-char (point-min))))

(advice-add 'kubel-get-resource-details :override #'kubel-describe-resource-details-popup)

When I put the kubel--exec inside the if I haven't managed to get it to work with -y from transient-args, but maybe someone here have some tips? It fails with the following:

if: Invalid function: (kubel--exec process-name (list "get" kubel-resource (kubel--get-resource-under-cursor) "-o" "yaml"))
Error in post-command-hook (transient--post-command): (wrong-type-argument number-or-marker-p nil)
Error in pre-command-hook (transient--pre-command): (wrong-type-argument (or eieio-object class) nil obj)

PS(this should be a new issue?): How come all of the parameters for each function needs to be prepended with -? For example calling kubel-get-resource-details, you can add -y. Would be easier to just use y.

[Kyrremann]

I've done some more work on this one, and the more I look at it, the more strange I think it is that it uses the get-command. So my proposal is to rewrite the kube--get-... to just be kube--describe.... This is kinda similar to kubel--describe-resource, but this one also ends up using get as the functions. At least how I understand it, cause I can't see the optional be used at all.

Also, shouldn't kubel-quick-edit call a kubel-get... funciton, instead of a kubel-describe... function?

This is my take on the new function:

(defun kubel-describe-resource-details-popup (&optional _)
  "Describe the details of the resource under the cursor"
  (interactive "P")
  (let* ((resource (kubel--get-resource-under-cursor))
         (process-name (format "kubel - %s - %s" kubel-resource resource)))
		(kubel--exec process-name (list "describe" kubel-resource (kubel--get-resource-under-cursor)))
		(yaml-mode)
		(view-mode)))

The (yaml-mode) should probably account for different outputs. Such as json.

If you'd like @abrochard, I can make a PR with my change, and how I think it all could look like.

PS: There is also something strange with kubel-output, if you set it to wide, and try to quick edit a deployment, you get kubectl get deployments -o wide, that can't be the intended behaviour?

[abrochard]

This thread confuses me. I think there are multiple issues at hand:

1. there isn't a separate between describe and get, which means that we can't tell if the kubel output is editable or not
2. if we manage to tell that the kubel output isn't editable, we could set it to read-only and bind q to exist
3. we default to yaml-mode when sometimes the output could be json
4. that wide behavior is weird and I don't remember what is going on there Did I get everything?

[Kyrremann]

Sounds like a good summary!

1. We should only do get when the user has chosen -y, or used quick edit, most other commands should use describe or at least end up with a read-only view.
2. q is solved with calling view-mode after any other mode.

[abrochard]

Sorry what is -y? Also I'm not so sure. I regularly switch to deployment view with R and then use enter to view and edit a deployment. I want to avoid making assumptions that could break workflows. Also for 3 and 4, those should be separate issues.

[Kyrremann]

If you press ctrl+u before enter, you will get the transient menu, and there you can add -y.

Im also afraid of breaking people's workflow, but should people use E/quick edit if they want to change a resource?

I think either adding a flag like I've did, or added a new shortcut is the way to go for this feature.

[abrochard]

Got it. I see no problem with making the buffer readonly if describe was used and that seems like a good PR. But I wouldn't touch beyond that without a lot of thinking. I can see a flag like describe-by-default or something where enter runs describe instead of get. Not sure about the name though.

[Kyrremann]

Agree with that, keep the scope small. I can make a PR with the name describe-by-defualt, so we can get going with the code, and think a bit more about the name.

[abrochard]

Sounds great, thank you!

[Kyrremann]

I’ve been looking into this today, and I’m pretty sure I’ve mistaken how at describe works in kubel. Haven’t noticed before now that C-u ENT is the way to run describe instead get. Personally I would have done it the opposite, but that’s not really a up for discussion here*.

So I think the solution would be to make it read only when you run describe, as you can't really apply the files/blob you get (as it adds events for most/all resources). Trying that for now.

Another nice feature would be to set the buffers that pop up when you for example delete a resource, to be read-only mode.

• Maybe we could make a flag that flips this? Setting this new flag to true makes all describe as default, and vice versa :shrug: Not sure if it’s worth the effort, at least compared to the other issues.

PS: This post was posted a bit earlier today, but wanted to make some more research before I posted, so ended up deleting it right after.

[Kyrremann]

Maybe something for a new issue, but view-mode buffers that spawn a child process doesn't kill the process when you close the buffer.

So adding view-mode to the buffer that is spawned when you tail logs or port-forward, makes it so that you need to kill the process-buffer manually (M-x list-process and then tab on the line with the process, then you end up in a buffer that you kan kill with C-k).

A compromise may be to turn on read-only-mode, so that you at least don't start typing in the buffer 🤔