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

Feature/default_view #2866

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

uozalp
Copy link
Contributor

@uozalp uozalp commented Sep 2, 2024

This pull request introduces a new configuration setting called initialView that allows users to specify the initial view in the application. The changes span across multiple files to integrate this new setting into the application logic, configuration, and documentation.

Configuration and Documentation Updates:

Application Logic Updates:

  • cmd/root.go: Modified the run function to set the active view based on the initialView configuration if it is specified.
  • internal/config/k9s.go:
    • Added InitialView as a new field in the K9s struct.
    • Updated the Merge method to include merging the InitialView field.

@uozalp
Copy link
Contributor Author

uozalp commented Sep 2, 2024

Created a new pull request, because I messed up my git.
Original PR: #2857

@uozalp
Copy link
Contributor Author

uozalp commented Oct 6, 2024

@derailed ?

@KevinGimbel
Copy link

Hello there! Thank you for your PR and your work on k9s!

There is already a way of setting active views per cluster - in the cluster specific configuration file ( $XDG_DATA_HOME/k9s/clusters/clusterX/contextY/config.yaml) under k9s.view.active

k9s:
  [...]
  view:
    active: po

The problem here is: the value for the cluster specific config is automatically generated and sets the view to a default value (I think po). This would take precedence the k9s "main" config.

So to implement the initialView feature we'd need to rewrite the logic of looking up the configs or remove the auto-generation of a value (or make initialView take precedence over any cluster specific setting which feels odd.)

@derailed I think your input is required here as to make a decision how to proceed with this feature.

@uozalp
Copy link
Contributor Author

uozalp commented Oct 7, 2024

@KevinGimbel I believe this behavior corresponds to the last active view for the cluster/context.

By default, K9s always starts in the last active view unless the --command flag is provided, which overrides it and takes precedence.

The changes proposed in this PR don't alter that behavior:

  • If K9s is started without any flags, it will start in the last active view.
  • If K9s is started with the --command flag, it will start in the specified view. If the flag is invalid, it falls back to the po view.
  • If K9s is started with the initialView setting, it will start in that view. If the view is invalid, it will also fall back to the po view.
  • If the initialView is configured and K9s is started with the --command flag, the --command flag will take precedence.

@uozalp
Copy link
Contributor Author

uozalp commented Nov 26, 2024

@derailed have you had a chance to look at this?

Copy link
Owner

@derailed derailed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uozalp Yes I think this make sense. I was actually thinking we should rename this defaultView since it is the fallback to either ignore the cli arg or the last stored view when set. What do you think?

@derailed derailed added enhancement New feature or request needs-tlc Pr needs additional updates conflicts labels Feb 16, 2025
@derailed
Copy link
Owner

@uozalp Also looks like we have some conflicts. Can you take a peek? Thank you!

@uozalp
Copy link
Contributor Author

uozalp commented Feb 16, 2025

@derailed I have renamed it to defaultView and updated the PR

@uozalp
Copy link
Contributor Author

uozalp commented Feb 16, 2025

I personally prefer the current behavior since we manage multiple clusters, and I like starting in the ctx view by default.

That said, there are cases where you might have a command like:

k9s -c deployment -n mynamespace --context mycontext

saved in your shell history. If you do a quick reverse search, you’d still want that command to take precedence, even if a default view is configured.

@uozalp uozalp requested a review from derailed February 17, 2025 20:06
@uozalp uozalp changed the title Feature/initial view Feature/defaultview Feb 18, 2025
@uozalp uozalp changed the title Feature/defaultview Feature/default view Feb 18, 2025
@uozalp uozalp changed the title Feature/default view Feature/default_view Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflicts enhancement New feature or request needs-tlc Pr needs additional updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants