Skip to content

Fix #3422 - ns cmd keep kind #3441

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

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

Conversation

mpas97
Copy link
Contributor

@mpas97 mpas97 commented Jul 7, 2025

No description provided.

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.

@mpas97 Needs TLC...

ns, ok := p.NSArg()
if !ok {
return false
}

if ns != "" {
_ = p.Reset("pod " + ns)
currentCommand, ok := c.app.cmdHistory.Top()
Copy link
Owner

Choose a reason for hiding this comment

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

I think this code has multiple issues ;(

If the last command is :sec fred we will end up with sec fred blee."

This is going to impact the command history i.e if we switch ns A->B any commands in the history buffer that does not explicitly specify a namespace will change. So if a user has po|cm|sec in her buffer that were issued prior to the switch, going back in history will now run all these in ns B vs A. Which is likely not going to be what the user expects. Same argument applies when switching context i.e do we preserve the history or axe it. A blessing and a curse... when going from dev->prod but likely not when going to a completely different cluster.
Perhaps a config option wipeHistoryOnContextSwitch might come in handy?

Looking for feedback/ideas here if anyone else wants to pipe in...

@derailed derailed added question Further information is requested needs-tlc Pr needs additional updates labels Jul 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-tlc Pr needs additional updates question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants