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

format list to resemble kubectl outputs #393

Merged
merged 1 commit into from
Oct 5, 2021
Merged

format list to resemble kubectl outputs #393

merged 1 commit into from
Oct 5, 2021

Conversation

mxyng
Copy link
Collaborator

@mxyng mxyng commented Oct 4, 2021

  • Add CURRENT header value
  • No newline before printing table

@@ -9,7 +9,7 @@ import (
)

type statusRow struct {
CurrentlySelected string `header:" "` // * if selected
CurrentlySelected string `header:"CURRENT"` // * if selected
Copy link
Contributor

Choose a reason for hiding this comment

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

Since some destinations in the future may not support a "current" context, should we avoid making this a full column for all destinations? An alternative some products use is adding a * after the name (see below for an example – docker context ls).

NAME            DESCRIPTION                               DOCKER ENDPOINT                                 KUBERNETES ENDPOINT                      ORCHESTRATOR                                                
default *       Current DOCKER_HOST based configuration   unix:///var/run/docker.sock                     https://localhost:8443/proxy (default)   swarm

Copy link
Contributor

Choose a reason for hiding this comment

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

Leaving as-is is also an option: several tools also use the star without a header (e.g. git branch) and I think it's become a familiar pattern to mean "current":

$ git branch
  lightweight
  lint
* list-roles
  local-fix
  lock-login

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point. For the first point, I was thinking we would have different sections for different destinations, similar to kubectl when you pass get all. In that case, each section has different headers. So for Kubernetes, there will be CURRENT and NAMESPACE columns while for something like SSH there will be an ADDRESS column.

The counterpoint I would have for the git case is that git doesn't have headers at all. If we omit headers from our output, it would look similar.

The main goal of this change is to mimic the kubectl config get-contexts output which users should be familiar with if they're using Kubernetes

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good! Wouldn't over-rotate on kubectl familiarity (it's not an easy tool to use in general) but I do think this is a good edit! :shipit:

Copy link
Contributor

@jmorganca jmorganca left a comment

Choose a reason for hiding this comment

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

Optional comment to consider as we add more destinations

@mxyng mxyng merged commit 124f9b9 into main Oct 5, 2021
@mxyng mxyng deleted the format-list branch October 5, 2021 17:55
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