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

Add gh cs view command #7496

Merged
merged 8 commits into from May 30, 2023
Merged

Add gh cs view command #7496

merged 8 commits into from May 30, 2023

Conversation

dmgardiner25
Copy link
Contributor

@dmgardiner25 dmgardiner25 commented May 26, 2023

Fixes: #7479

Adds the gh cs view command to give more details about a specific codespace. By default, we only show a few of the fields that most users will care about (similar to what VS Code does, shown below) but a wider array of options are available via the --json flag. Also, if the user is in a codespace and runs the command it will show details about that codespace.

What VS Code shows:
image

gh cs view (no flags) shows the codespace picker:
image

And once you select a codespace or provide the name using the -c flag, it will show this:
image

All of the data available with the --json flag:
image

Tests are WIP pending some design review.

Copy link
Contributor

@cmbrose cmbrose left a comment

Choose a reason for hiding this comment

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

Looks solid, just a few nits and requests ⚡

internal/codespaces/api/api.go Show resolved Hide resolved
pkg/cmd/codespace/view.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/view.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/view.go Outdated Show resolved Hide resolved
pkg/cmd/codespace/view.go Show resolved Hide resolved
pkg/cmd/codespace/view.go Show resolved Hide resolved
@craiglpeters
Copy link

Looks good!

pkg/cmd/codespace/view.go Show resolved Hide resolved
@dmgardiner25 dmgardiner25 marked this pull request as ready for review May 30, 2023 21:25
@dmgardiner25 dmgardiner25 requested a review from a team as a code owner May 30, 2023 21:25
@cliAutomation cliAutomation added the external pull request originating outside of the CLI core team label May 30, 2023
@cliAutomation cliAutomation added this to Needs review 🤔 in The GitHub CLI May 30, 2023
@dmgardiner25 dmgardiner25 merged commit 06e40df into cli:trunk May 30, 2023
7 checks passed
The GitHub CLI automation moved this from Needs review 🤔 to Pending Release 🥚 May 30, 2023
@dmgardiner25
Copy link
Contributor Author

@mislav I'm not sure what the team's policy is around new commands and I realized I probably should have had you take a look before merging. Are there any changes to the wording, style, etc., that you think should be updated before this gets shipped out in the next release?

Copy link
Contributor

@samcoe samcoe left a comment

Choose a reason for hiding this comment

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

@dmgardiner25 Thanks for pinging our team. I left a couple code comments based around the wording and style of the command so that it is a bit more consistent with other commands. Let me know if you have any questions about my comments.

internal/codespaces/api/api.go Show resolved Hide resolved
Long: heredoc.Doc(`
View details about a codespace.

For more fine-grained details, you can add the "--json" flag to view the full list of fields available.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure this is necessary since it is repeated in the examples section.


For more fine-grained details, you can add the "--json" flag to view the full list of fields available.

If this command doesn't provide enough information, you can use the "gh api" command to view the full JSON response:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know of anywhere else that we explicitly tell the user an endpoint to hit, I think that might be a bit too low level of detail that the user might not be concerned with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a suggestion from @cmbrose above. This command shows most of the useful information about a codespace but there are still some additional fields in the API response not shown so I think that's where the suggestion was coming from. This is all documented in the API docs so I don't mind removing it from the description.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove it for now. Almost none of our commands expose every field in the API response and when users want more fields added they tend to either create an issue or start digging in using the api command and looking at the API docs. Always easy enough to iterate if we receive feedback.

$ gh cs view

# view the details of a specific codespace
$ gh cs view -c <codespace-name>
Copy link
Contributor

Choose a reason for hiding this comment

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

In the examples we tend to fill in the arguments and flags with example values rather than leaving them as placeholder text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the way codespace names are formatted, doing something like username-laughing-meme-4qj7qpjrqxx2pr5 seemed like it might be more confusing than leaving the placeholder but if you would prefer the example value I can add it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think it would be best to change the placeholders to something like codespace-name-12345 something that is the similar/same format as a standard codespace name but clearly an example. This command has a pretty straight forward use so I don't anticipate this causing any confusion. If we receive feedback that it is we can iterate.

return opts.exporter.Write(a.io, selectedCodespace)
}

//nolint:staticcheck // SA1019: utils.NewTablePrinter is deprecated: use internal/tableprinter
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a new table printer utility in internal/tableprinter that would be good to use here instead of using the deprecated one.


for _, field := range fields {
// Only display the field if it has a value.
if field.value != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

For TTY output removing fields is fine but in scripting mode (non-TTY) it is nice to keep fields even if they are blank so that the output is consistent and easy to parse.

func formatGitStatus(codespace codespace) string {
branchWithGitStatus := codespace.branchWithGitStatus()
// u2193 and u2191 are the unicode arrows for down and up
return fmt.Sprintf("%s - %d\u2193 %d\u2191", branchWithGitStatus, codespace.GitStatus.Behind, codespace.GitStatus.Ahead)
Copy link
Contributor

Choose a reason for hiding this comment

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

I am slightly concerned at the unicode usage here because not all fonts support them. We tend to shy away from using them except in certain cases. I am also uncertain how accessible they are. @vilmibm do you have any opinions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was to keep some similarity with how we show this information in the VS Code extension and isn't necessary so I can remove them.

renovate bot added a commit to scottames/dots that referenced this pull request Jun 23, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [aquaproj/aqua-registry](https://togithub.com/aquaproj/aqua-registry)
| minor | `v4.20.0` -> `v4.21.1` |
| [cli/cli](https://togithub.com/cli/cli) | minor | `v2.30.0` ->
`v2.31.0` |
| [weaveworks/eksctl](https://togithub.com/weaveworks/eksctl) | minor |
`v0.145.0` -> `v0.146.0` |
| [zellij-org/zellij](https://togithub.com/zellij-org/zellij) | patch |
`v0.37.1` -> `v0.37.2` |

---

### Release Notes

<details>
<summary>aquaproj/aqua-registry</summary>

###
[`v4.21.1`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.21.1)

[Compare
Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.21.0...v4.21.1)


[Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.21.1)
| [Pull
Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.21.1)
| aquaproj/aqua-registry@v4.21.0...v4.21.1

#### Fixes


[#&#8203;13199](https://togithub.com/aquaproj/aqua-registry/issues/13199)
kubernetes-sigs/kustomize: Follow up changes of kustomize v5.1.0

-
[kubernetes-sigs/kustomize#5220

###
[`v4.21.0`](https://togithub.com/aquaproj/aqua-registry/releases/tag/v4.21.0)

[Compare
Source](https://togithub.com/aquaproj/aqua-registry/compare/v4.20.0...v4.21.0)


[Issues](https://togithub.com/aquaproj/aqua-registry/issues?q=is%3Aissue+milestone%3Av4.21.0)
| [Pull
Requests](https://togithub.com/aquaproj/aqua-registry/pulls?q=is%3Apr+milestone%3Av4.21.0)
| aquaproj/aqua-registry@v4.20.0...v4.21.0

#### 🎉 New Packages


[#&#8203;13173](https://togithub.com/aquaproj/aqua-registry/issues/13173)
[assetnote/surf](https://togithub.com/assetnote/surf): Escalate your
SSRF vulnerabilities on Modern Cloud Environments. `surf` allows you to
filter a list of hosts, returning a list of viable SSRF candidates

[#&#8203;13174](https://togithub.com/aquaproj/aqua-registry/issues/13174)
[iyear/tdl](https://togithub.com/iyear/tdl): A Telegram downloader
written in Golang

[#&#8203;13172](https://togithub.com/aquaproj/aqua-registry/issues/13172)
[nikolaydubina/go-cover-treemap](https://togithub.com/nikolaydubina/go-cover-treemap):
Go code coverage to SVG treemap
[@&#8203;iwata](https://togithub.com/iwata)

</details>

<details>
<summary>cli/cli</summary>

### [`v2.31.0`](https://togithub.com/cli/cli/releases/tag/v2.31.0):
GitHub CLI 2.31.0

[Compare Source](https://togithub.com/cli/cli/compare/v2.30.0...v2.31.0)

#### What's New

- New suite of `project` commands for interacting with and manipulating
projects. Huge shoutout 🥳 for the time and effort put into this work by
[@&#8203;mntlty](https://togithub.com/mntlty) in
[cli/cli#7375
[cli/cli#7578
- New `search code` command by
[@&#8203;joshkraft](https://togithub.com/joshkraft) in
[cli/cli#7376
- New `cs view` command by
[@&#8203;dmgardiner25](https://togithub.com/dmgardiner25) in
[cli/cli#7496
[cli/cli#7539

#### What's Changed

- `api`: output a single JSON array in REST pagination mode by
[@&#8203;mislav](https://togithub.com/mislav) in
[cli/cli#7190
- `api`: support array params in GET queries by
[@&#8203;mislav](https://togithub.com/mislav) in
[cli/cli#7513
- `api`: force method to uppercase by
[@&#8203;ffalor](https://togithub.com/ffalor) in
[cli/cli#7514
- `alias`: Allow aliases to recognize extended commands by
[@&#8203;srz-zumix](https://togithub.com/srz-zumix) in
[cli/cli#7523
- `alias import`: Fix `--clobber` flag by
[@&#8203;samcoe](https://togithub.com/samcoe) in
[cli/cli#7569
- `run rerun`: Improve docs around `--job` flag by
[@&#8203;williammartin](https://togithub.com/williammartin) in
[cli/cli#7527
- `run view`: Support viewing logs for jobs with composite actions by
[@&#8203;williammartin](https://togithub.com/williammartin) in
[cli/cli#7526
- `gist edit`: Add selector option to `gist edit` command by
[@&#8203;kousikmitra](https://togithub.com/kousikmitra) in
[cli/cli#7537
- `repo clone`: Set upstream remote to track all branches after initial
fetch by [@&#8203;samcoe](https://togithub.com/samcoe) in
[cli/cli#7542
- `extension`: Speed up listing extensions by lazy-loading extension
information when needed by [@&#8203;mislav](https://togithub.com/mislav)
in
[cli/cli#7493
- `auth`: Add timeouts to keyring operations by
[@&#8203;samcoe](https://togithub.com/samcoe) in
[cli/cli#7580
- `auth status`: write to stdout on success by
[@&#8203;rajhawaldar](https://togithub.com/rajhawaldar) in
[cli/cli#7540
- `completion`: Fix bash completions for extensions and aliases by
[@&#8203;mislav](https://togithub.com/mislav) in
[cli/cli#7525
- `issue/pr view`: alphabetically sort labels for `gh pr/issue view` by
[@&#8203;ffalor](https://togithub.com/ffalor) in
[cli/cli#7587
- Fix error handling for extension and shell alias commands by
[@&#8203;samcoe](https://togithub.com/samcoe) in
[cli/cli#7567
- Fix pkg imported more than once by
[@&#8203;testwill](https://togithub.com/testwill) in
[cli/cli#7591
- Refactor a nested if statement by
[@&#8203;yanskun](https://togithub.com/yanskun) in
[cli/cli#7596
- Fix a typo by
[@&#8203;lerocknrolla](https://togithub.com/lerocknrolla) in
[cli/cli#7557
- Fix flaky test by [@&#8203;samcoe](https://togithub.com/samcoe) in
[cli/cli#7515
- Credential rotations, renames and decouplings from Mislav by
[@&#8203;williammartin](https://togithub.com/williammartin) in
[cli/cli#7544
- build(deps): bump github.com/cli/go-gh/v2 from 2.0.0 to 2.0.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[cli/cli#7546
- build(deps): bump github.com/AlecAivazis/survey/v2 from 2.3.6 to 2.3.7
by [@&#8203;dependabot](https://togithub.com/dependabot) in
[cli/cli#7576

#### New Contributors

- [@&#8203;srz-zumix](https://togithub.com/srz-zumix) made their first
contribution in
[cli/cli#7523
- [@&#8203;lerocknrolla](https://togithub.com/lerocknrolla) made their
first contribution in
[cli/cli#7557
- [@&#8203;testwill](https://togithub.com/testwill) made their first
contribution in
[cli/cli#7591
- [@&#8203;rajhawaldar](https://togithub.com/rajhawaldar) made their
first contribution in
[cli/cli#7540

**Full Changelog**: cli/cli@v2.30.0...v2.31.0

</details>

<details>
<summary>weaveworks/eksctl</summary>

###
[`v0.146.0`](https://togithub.com/weaveworks/eksctl/releases/tag/v0.146.0):
eksctl 0.146.0 (permalink)

[Compare
Source](https://togithub.com/weaveworks/eksctl/compare/0.145.0...0.146.0)

### Release v0.146.0

#### 🎯 Improvements

- Update vpc-cni to 1.12.6
([#&#8203;6692](https://togithub.com/weaveworks/eksctl/issues/6692))

#### 🧰 Maintenance

- Bump dependencies
([#&#8203;6690](https://togithub.com/weaveworks/eksctl/issues/6690))

#### 📝 Documentation

- New eksctl channel
([#&#8203;6697](https://togithub.com/weaveworks/eksctl/issues/6697))

#### Acknowledgments

Weaveworks would like to sincerely thank:
[@&#8203;wind0r](https://togithub.com/wind0r)

</details>

<details>
<summary>zellij-org/zellij</summary>

###
[`v0.37.2`](https://togithub.com/zellij-org/zellij/releases/tag/v0.37.2)

[Compare
Source](https://togithub.com/zellij-org/zellij/compare/v0.37.1...v0.37.2)

This is a patch release to fix some minor issues in the previous
release.

#### What's Changed

- hotfix: include theme files into binary by
[@&#8203;jaeheonji](https://togithub.com/jaeheonji) in
[zellij-org/zellij#2566
- fix(plugins): make hide_self api idempotent by
[@&#8203;imsnif](https://togithub.com/imsnif) in
[zellij-org/zellij#2568

**Full Changelog**:
zellij-org/zellij@v0.37.1...v0.37.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get
[config help](https://togithub.com/renovatebot/renovate/discussions) if
that's undesired.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/scottames/dots).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMzEuMCIsInVwZGF0ZWRJblZlciI6IjM1LjEzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@Anemia88

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external pull request originating outside of the CLI core team
Projects
No open projects
The GitHub CLI
  
Pending Release 🥚
Development

Successfully merging this pull request may close these issues.

Add gh cs info command to get more fine-grained details about a specific codespace
6 participants