Skip to content
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

Remove multiple setq calls to tabulated-list-sort-key #59

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

psibi
Copy link

@psibi psibi commented Sep 7, 2021

We current set the variable tabulated-list-sort-key multiple
times. This isn't really needed.

We current set the variable tabulated-list-sort-key multiple
times. This isn't really needed.
@abrochard
Copy link
Owner

Thank you for catching this! I can't remember why we do that. I feel like having a sort key is better than not having one. The risk though is that for some resource the first column isn't NAME. I did some local testing with pod/services/secrets/nodes/deployments and didn't see any issues though. Maybe the solution is to remove (setq tabulated-list-sort-key nil)? Or even better to dynamically pass a sort key?

@psibi
Copy link
Author

psibi commented Sep 16, 2021

I feel like having a sort key is better than not having one. The risk though is that for some resource the first column isn't NAME.

Yeah, agree.

I did some local testing with pod/services/secrets/nodes/deployments and didn't see any issues though. Maybe the solution is to remove (setq tabulated-list-sort-key nil)?

The default behavior I would want is to have a non sorted thing. But I guess we can make optionally configurable with the default value of nil ?

Or even better to dynamically pass a sort key?

What do you mean by this ? You mean some interactive function to set the sort key ?

@abrochard
Copy link
Owner

I did some local testing with pod/services/secrets/nodes/deployments and didn't see any issues though. Maybe the solution is to remove (setq tabulated-list-sort-key nil)?

The default behavior I would want is to have a non sorted thing. But I guess we can make optionally configurable with the default value of nil ?

RIght now the default is to let kubectl output dictate the order (which I believe is always sorted alphabetically on the first column). The advantage to having a sort key is that it lets Emacs do some more work rather than pass arguments to kubectl. There are no disadvantages aside from potentially a bug if the first column returned from the kubectl command isn't call NAME.

Or even better to dynamically pass a sort key?

What do you mean by this ? You mean some interactive function to set the sort key ?

Yeah the column names are dynamically generated based on the output of the kubectl command for the given resource. And the output is always sorted based on first column. A nice feature would be to always make the first column name the sort key.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants