KEMBAR78
Fix: `project` commands use shared progress indicator by BagToad · Pull Request #10817 · cli/cli · GitHub
Skip to content

Conversation

@BagToad
Copy link
Member

@BagToad BagToad commented Apr 17, 2025

Description

project commands have their own spinner implementation instead of relying on the shared iostreams package's progress indicator. This is a problem because we now have a GH_SPINNER_DISABLED textual progress indicator which is not being respected by project commands.

This changes project commands to use the iostreams package's progress indicator instead of their own spinner implementation.

Demo

Note

Leveraging the GH_SPINNER_DISABLED environment variable and observing a textual progress indicator proves that project commands are leveraging the proper iostreams progress indicator.

❯ GH_SPINNER_DISABLED=true ./bin/gh project item-list
Fetching ViewerLoginAndOrgs...
? Which owner would you like to use? BagToad
Fetching ViewerProjects...
? Which project would you like to use? @BagToad's untitled project (#1)
Fetching ViewerProjectWithItems...
Project 1 for owner BagToad has no items

Commentary

At first, I felt that the project queries package was too strongly coupled to user interface mechanisms... Worded differently: I thought the queries package shouldn't be responsible for displaying the progress indicator at all.

This turned out to be more involved than I realized with even prompting being handled by the queries package in many places. This strong coupling of queries to UI elements convinced me that the right solution right now is not to "fix" that coupling, but rather to correct the spinner implementation.

Thus, the proposed solution here is essentially to remove the spinner boolean from the Client and replace it with an iostreams.IOStreams instance which was already provided to the Client's constructor function, meaning this doesn't require a function signature change or changes to locations where the Client constructor is called.

You'll notice that we're now getting some messages (AKA labels) printed in the progress indicator now as well. I've made this change because, without it, the GH_SPINNER_DISABLED progress indicator would show a general Working... message that doesn't feel all that descriptive.

@Copilot Copilot AI review requested due to automatic review settings April 17, 2025 17:20
@BagToad BagToad requested a review from a team as a code owner April 17, 2025 17:20
@BagToad BagToad requested a review from babakks April 17, 2025 17:20
@BagToad BagToad temporarily deployed to cli-automation April 17, 2025 17:20 — with GitHub Actions Inactive
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the project commands to use the shared progress indicator provided by the iostreams package instead of a custom spinner implementation. Key changes include removing the spinner dependency and time package, adding a new progress indicator function (doQueryWithProgressIndicator), and updating all query calls accordingly.

Comments suppressed due to low confidence (1)

pkg/cmd/project/shared/queries/queries.go:25

  • [nitpick] The field name 'io' might be too terse; consider renaming it to 'ioStreams' or a similarly descriptive name to improve clarity.
io:        ios,

@BagToad BagToad requested a review from andyfeller April 17, 2025 17:25
Copy link
Member

@andyfeller andyfeller left a comment

Choose a reason for hiding this comment

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

✨ So much nicer to have a unified spinner experience! 🙌

@BagToad BagToad merged commit 41f9450 into trunk Apr 23, 2025
17 checks passed
@BagToad BagToad deleted the kw/project-query-uses-iostreams-progress-indicator branch April 23, 2025 13:15
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request May 10, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cli/cli](https://github.com/cli/cli) | minor | `v2.69.0` -> `v2.72.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>cli/cli (cli/cli)</summary>

### [`v2.72.0`](https://github.com/cli/cli/releases/tag/v2.72.0): GitHub CLI 2.72.0

[Compare Source](cli/cli@v2.71.2...v2.72.0)

#### :accessibility: Accessibility public preview

This release marks the public preview of several accessibility improvements to the GitHub CLI that have been under development over the past year in partnership with our friends at [Charm](https://github.com/charmbracelet) including:

-   customizable and contrasting colors
-   non-interactive user input prompting
-   text-based spinners

These new experiences are captured in a new `gh a11y` help topic command, which goes into greater detail into the motivation behind each of them as well as opt-in configuration settings / environment variables.

We would like you to share your feedback and join us on this journey through one of [GitHub Accessibility feedback channels](https://accessibility.github.com/feedback)! 🙌

#### What's Changed

##### ✨ Features

-   Introduce `gh accessibility` help topic highlighting GitHub CLI accessibility experiences by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#10890
-   \[gh pr view] Support `closingIssuesReferences` JSON field by [@&#8203;iamazeem](https://github.com/iamazeem) in cli/cli#10544

##### 🐛 Fixes

-   Fix expected error output of `TestRepo/repo-set-default` by [@&#8203;aconsuegra](https://github.com/aconsuegra) in cli/cli#10884
-   Ensure accessible password and auth token prompters disable echo mode by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#10885
-   Fix: Accessible multiselect prompt respects default selections by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10901

#### New Contributors

-   [@&#8203;aconsuegra](https://github.com/aconsuegra) made their first contribution in cli/cli#10884

**Full Changelog**: cli/cli@v2.71.2...v2.72.0

### [`v2.71.2`](https://github.com/cli/cli/releases/tag/v2.71.2): GitHub CLI 2.71.2

[Compare Source](cli/cli@v2.71.1...v2.71.2)

#### What's Changed

-   Fix pr create when push.default tracking and no merge ref by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10863

**Full Changelog**: cli/cli@v2.71.1...v2.71.2

### [`v2.71.1`](https://github.com/cli/cli/releases/tag/v2.71.1): GitHub CLI 2.71.1

[Compare Source](cli/cli@v2.71.0...v2.71.1)

#### What's Changed

-   Fix pr create when branch name contains slashes by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10859

**Full Changelog**: cli/cli@v2.71.0...v2.71.1

### [`v2.71.0`](https://github.com/cli/cli/releases/tag/v2.71.0): GitHub CLI 2.71.0

[Compare Source](cli/cli@v2.70.0...v2.71.0)

#### What's Changed

##### ✨ Features

-   `gh pr create`: Support Git's `@{push}` revision syntax for determining head ref by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10513
-   Introduce option to opt-out of spinners by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10773
-   Update configuration support for accessible colors by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#10820
-   `gh config`: add config settings for accessible prompter and disabling spinner by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10846

##### 🐛 Fixes

-   Fix multi pages search for gh search by [@&#8203;leudz](https://github.com/leudz) in cli/cli#10767
-   Fix: `project` commands use shared progress indicator by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10817
-   Issue commands should parse args early by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10811
-   Feature detect v1 projects on `issue view` by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10813
-   Feature detect v1 projects on non web-mode `issue create` by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10815
-   Feature detect v1 projects on web mode issue create by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10818
-   Feature detect v1 projects on issue edit by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10819

##### 📚 Docs & Chores

-   Refactor Sigstore verifier logic by [@&#8203;malancas](https://github.com/malancas) in cli/cli#10750

##### :dependabot: Dependencies

-   chore(deps): bump github.com/sigstore/sigstore-go from 0.7.1 to 0.7.2 by [@&#8203;dependabot](https://github.com/dependabot) in cli/cli#10787
-   Bump google.golang.org/grpc from 1.71.0 to 1.71.1 by [@&#8203;dependabot](https://github.com/dependabot) in cli/cli#10758

#### New Contributors

-   [@&#8203;leudz](https://github.com/leudz) made their first contribution in cli/cli#10767

**Full Changelog**: cli/cli@v2.70.0...v2.71.0

### [`v2.70.0`](https://github.com/cli/cli/releases/tag/v2.70.0): GitHub CLI 2.70.0

[Compare Source](cli/cli@v2.69.0...v2.70.0)

#### Accessibility

This release contains dark shipped changes that are part of a larger GitHub CLI accessibility preview still under development.  More information about these will be announced later this month including various channels to work with GitHub and GitHub CLI maintainers on shaping these experiences.

##### Ensure table headers are thematically contrasting

[#&#8203;8292](cli/cli#8292) is a long time issue where table headers were difficult to see in terminals with light background.  Ahead of the aforementioned preview, `v2.70.0` has shipped changes that improve the out-of-the-box experience based on terminal background detection.

The following screenshots demonstrate the Mac Terminal using the Basic profile, which responds to user's appearance preferences:

<img width="1512" alt="Screenshot of gh repo list in light background terminal" src="https://github.com/user-attachments/assets/87413dde-eec8-43eb-9c16-dc84f8249ddf" />

<img width="1512" alt="Screenshot of gh repo list in dark background terminal" src="https://github.com/user-attachments/assets/7430b42c-7267-402b-b565-a296beb4d5ea" />

For more information including demos from various official distributions, see [#&#8203;10649](cli/cli#10649).

#### What's Changed

##### ✨ Features

-   Update go-gh and document available sprig funcs by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10680
-   Introducing experimental support for rendering markdown with customizable, accessible colors by [@&#8203;andyfeller](https://github.com/andyfeller) [@&#8203;jtmcg](https://github.com/jtmcg) in cli/cli#10680
-   Ensure table datetime columns have thematic, customizable muted text by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#10709
-   Ensure table headers are thematically contrasting by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#10649
-   Introduce configuration setting for displaying issue and pull request labels in rich truecolor by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#10720
-   Ensure muted text is thematic and customizable by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#10737
-   \[gh repo create] Show host name in repo creation prompts by [@&#8203;iamazeem](https://github.com/iamazeem) in cli/cli#10516
-   Introduce accessible prompter for screen readers (preview) by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10710

##### 🐛 Fixes

-   `run list`: do not fail on organization/enterprise ruleset imposed workflows by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10660
-   Implement safeguard for `gh alias delete` test, prevent wiping out GitHub CLI configuration by [@&#8203;andyfeller](https://github.com/andyfeller) in cli/cli#10683
-   Pin third party actions to commit sha by [@&#8203;BagToad](https://github.com/BagToad) in cli/cli#10731
-   Fallback to job run logs when step logs are missing by [@&#8203;babakks](https://github.com/babakks) in cli/cli#10740
-   \[gh ext] Fix `GitKind` extension directory path by [@&#8203;iamazeem](https://github.com/iamazeem) in cli/cli#10609
-   Fix job log resolution to skip legacy logs in favour of normal/new ones by [@&#8203;babakks](https://github.com/babakks) in cli/cli#10769

##### 📚 Docs & Chores

-   `./script/sign` cleanup by [@&#8203;iamazeem](https://github.com/iamazeem) in cli/cli#10599
-   Fix typos in CONTRIBUTING.md by [@&#8203;rylwin](https://github.com/rylwin) in cli/cli#10657
-   Improve `gh at verify --help`, document json output by [@&#8203;phillmv](https://github.com/phillmv) in cli/cli#10685
-   Acceptance test issue/pr create/edit with project by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10707
-   Escape dots in regexp pattern in `README.md` by [@&#8203;babakks](https://github.com/babakks) in cli/cli#10742
-   Simplify cosign verification example by not using a regex. by [@&#8203;kommendorkapten](https://github.com/kommendorkapten) in cli/cli#10759
-   Document UNKNOWN STEP in run view by [@&#8203;williammartin](https://github.com/williammartin) in cli/cli#10770

##### :dependabot: Dependencies

-   Update github.com/sigstore/sigstore-go to 0.7.1 and fix breaking function change by [@&#8203;malancas](https://github.com/malancas) in cli/cli#10749

#### New Contributors

-   [@&#8203;rylwin](https://github.com/rylwin) made their first contribution in cli/cli#10657

**Full Changelog**: cli/cli@v2.69.0...v2.70.0

</details>

---

### Configuration

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

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

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

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4yNTkuMCIsInVwZGF0ZWRJblZlciI6IjM5LjI2NC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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.

3 participants