[gh release create/upload] Expand glob patterns for all platforms by iamazeem · Pull Request #10413 · cli/cli

iamazeem · Feb 9, 2025

Verified that this issue is only applicable to Windows Command Prompt and Powershell.
See this relevant SO thread for more details.

GitBash behaves same as a Linux shell.

This issue affects gh release create and gh release upload commands.
In this PR, the glob patterns are expanded internally for Windows.

Tests (same for Windows Command Prompt and Powershell)

# Create test files e.g. test.{1,2,3} and log.{1,2,3}
> echo "test.1" > test.1
> echo "test.2" > test.2
> echo "test.3" > test.3
> echo "log.1" > log.1
> echo "log.2" > log.2
> echo "log.3" > log.3

# Create release with test.{1,2,3} files with glob pattern test.*
> .\gh.exe release create --repo iamazeem/test --draft --notes "Test" v1.2.3 test.*
https://github.com/iamazeem/test/releases/tag/untagged-d72acf4185040741cacb

# Verify release
> .\gh.exe release view --repo iamazeem/test v1.2.3
v1.2.3
Draft • iamazeem created this less than a minute ago

  Test


Assets
test.1  18 B
test.2  18 B
test.3  18 B

View on GitHub: https://github.com/iamazeem/test/releases/tag/untagged-d72acf4185040741cacb


# Upload log.{1,2,3} files to release with glob pattern log.*
> .\gh.exe release upload --repo iamazeem/test v1.2.3 log.*
Successfully uploaded 3 assets to v1.2.3

# Verify release again
> .\gh.exe release view --repo iamazeem/test v1.2.3
v1.2.3
Draft • iamazeem created this about 2 minutes ago

  Test


Assets
log.1   16 B
log.2   16 B
log.3   16 B
test.1  18 B
test.2  18 B
test.3  18 B

View on GitHub: https://github.com/iamazeem/test/releases/tag/untagged-d72acf4185040741cacb

Invalid patterns

# Create release with a glob pattern which doesn't expand e.g. test123.*
> .\gh.exe release create --repo iamazeem/test --draft --notes "Test" v1.2.3 test123.*
CreateFile test123.*: The filename, directory name, or volume label syntax is incorrect.

Notes

In Powershell, quoting (single or double) glob patterns work the same and the quotes are not passed to gh.
In Command Prompt, double quotes work same as Powershell but single quotes become part of the pattern itself.
- For example, 'test.*' is passed as-is and cannot be expanded.

Error on Command Prompt

> .\gh.exe release create --repo iamazeem/test --draft --notes "Test" v1.2.3 'test.*'
CreateFile 'test.*': The filename, directory name, or volume label syntax is incorrect.

UPDATE

After discussions, the support for glob patterns has been added for all the platforms.

jtmcg

Coming in with another blast from the past! We appreciate all the work you're putting in, @iamazeem 🙌

I reviewed the old issue and I think there was just a bit more to do from there than is delivered in this PR. I've added Acceptance Criteria to it so we can more easily track what needs doing. Let me know if you have any questions.

tl;dr: We want to expand wildcards for ALL platforms using gh and we want this to happen in gist create and release create in addition to what you've done here for release upload. I suspect its not too much more effort for those, given the solution you've outlined here 🙂

Also, if you haven't already come to expect it from me by now, I'm going to ask for some tests 😅

iamazeem · Feb 10, 2025

@jtmcg:

We want to expand wildcards for ALL platforms using gh

As mentioned in the PR description, it's only needed for Windows.
The shells on Linux and Mac perform their own expansion.
That's why I targeted only Windows after verification.

we want this to happen in gist create

Sure, I'll add its support for gist create.

Also, if you haven't already come to expect it from me by now, I'm going to ask for some tests 😅

Actually, I thought about how to approach these tests and then decided not to.
Apparently, it's a shell-specific thing and didn't feel like that it should be covered.
If you could share some examples that'd be very helpful. Thanks!

iamazeem · Feb 11, 2025

`gh gist create`

Working fine both on Linux and Windows.

Tests (Windows)

> .\gh.exe gist create -d "test" test.*
- Creating gist with multiple files
✓ Created secret gist test.1
https://gist.github.com/iamazeem/4d5a636d1c66f58be21440efd37247f0

>.\gh.exe gist list -L1
ID                                DESCRIPTION  FILES    VISIBILITY  UPDATED
4d5a636d1c66f58be21440efd37247f0  test         3 files  secret      less than a minute ago

> .\gh.exe gist view 4d5a636d1c66f58be21440efd37247f0
test

test.1

test.1

test.2

test.1

test.3

test.1

> .\gh.exe gist delete --yes 4d5a636d1c66f58be21440efd37247f0
✓ Gist "test.1" deleted

iamazeem · Feb 11, 2025

@jtmcg: Please review. Thanks!

…atterns-on-windows

jtmcg · Feb 13, 2025

As mentioned in the PR description, it's only needed for Windows.
The shells on Linux and Mac perform their own expansion.
That's why I targeted only Windows after verification.

Totally. There may be a good reason to also handle other os's explicitly in the CLI, though... Not sure what they are at the moment, but I'll think on this and get back to you on it 👍 If you can think of any reasons for or against it, I'm all ears, too

Actually, I thought about how to approach these tests and then decided not to.
Apparently, it's a shell-specific thing and didn't feel like that it should be covered.
If you could share some examples that'd be very helpful. Thanks!

Sure thing! I'll take a look and add a PR into your feature branch for it if I can find a solution

jtmcg · Feb 13, 2025

@iamazeem, here's the PR with the testing example. Note, I moved the os check out of the helper function and into the commands. I'm not sure if that's exactly right, but it seemed weird to have a function named GlobWindowsPaths that was also making decisions about mac and ubuntu 🤷

iamazeem · Feb 14, 2025

@jtmcg

There may be a good reason to also handle other os's explicitly in the CLI, though... Not sure what they are at the moment, but I'll think on this and get back to you on it 👍 If you can think of any reasons for or against it, I'm all ears, too

On nix shells, the expansions happen when args are in double quotes e.g. "test.*".
In single quotes, the glob patterns are not expanded e.g. 'test.*'.
If gh were to start expanding everything internally then the behavior of single quotes would be altered.
That was my only concern while implementing this i.e. not to break the default behavior of nix shell expansions and the UX should remain consistent on all platforms where gh works.

jtmcg

Great, so here's where we're at:

We want to handle all globbing ourselves, regardless of shell
We want to error if a glob pattern doesn't find any matches. This error should contain ALL patterns that don't have matches and not just the first that didn't match. Perhaps this suggests a new bespoke error type?

Does this make sense?

jtmcg

Looking good. I like how clean this reads, now! Have a couple of questions, though

jtmcg

The requested changes are described in this comment

…atterns-on-windows

…mers (#3) * Separate partitioning from globbing in cmdutil/args package and consumers In the previous commit, GlobPaths was overloaded, containing logic specific to command use-cases. This commit removes that functionality from GlobPaths and back into the commands that have the special use-cases. To do this, I've introduced a new Partition util in cmdutil/args.go that will separate a slice into two slices given a predicate. This functionality is leveraged by both the special use-cases described above to separate the command-specific syntax from the globable filepaths. * Add test to validate that the order of '-' in gh gist create args doesn't matter

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.68.1` -> `v2.69.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.69.0`](https://github.com/cli/cli/releases/tag/v2.69.0): GitHub CLI 2.69.0 [Compare Source](cli/cli@v2.68.1...v2.69.0) #### What's Changed ##### Features - Commands that accept filepath arguments will do glob expansion for `*` characters, by [@iamazeem](https://github.com/iamazeem) in cli/cli#10413 ##### Bug Fixes - `gh issue/pr comment --edit-last` no longer creates a comment in non-interactive mode if there weren't one. A new flag `--create-if-none` provides this behaviour, by [@andyfeller](https://github.com/andyfeller) in cli/cli#10625 - `gh repo sync` provides a more informative error for missing workflow permissions when the token is provided by a GitHub app, by [@wata727](https://github.com/wata727) in cli/cli#10574 - `gh api` no longer tries to encode URLs incorrectly, by [@williammartin](https://github.com/williammartin) in cli/cli#10630 ##### Other - Add cli-discuss-automation environment to triage.md by [@jtmcg](https://github.com/jtmcg) in cli/cli#10552 - chore: remove redundant word in comment by [@kevincatty](https://github.com/kevincatty) in cli/cli#10586 - Bump golang.org/x/net from 0.34.0 to 0.36.0 by [@dependabot](https://github.com/dependabot) in cli/cli#10593 #### New Contributors - [@kevincatty](https://github.com/kevincatty) made their first contribution in cli/cli#10586 - [@wata727](https://github.com/wata727) made their first contribution in cli/cli#10574 **Full Changelog**: cli/cli@v2.68.1...v2.69.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. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

[gh release create/upload] Expand glob patterns on Windows

161ad92

iamazeem requested a review from a team as a code owner February 9, 2025 17:55

iamazeem requested a review from andyfeller February 9, 2025 17:55

cliAutomation added the external pull request originating outside of the CLI core team label Feb 9, 2025

jtmcg suggested changes Feb 10, 2025

View reviewed changes

Comment thread pkg/cmd/release/shared/upload.go Outdated

jtmcg requested review from jtmcg and removed request for andyfeller February 10, 2025 16:48

Fix gh gist create for multiple filenames and glob patterns

d9bfa60

Merge branch 'trunk' into 5099-gh-release-create-upload-expand-glob-p…

f0ac3bc

…atterns-on-windows

SahibYar reviewed Feb 13, 2025

View reviewed changes

Comment thread pkg/cmdutil/args.go Outdated

SahibYar reviewed Feb 13, 2025

View reviewed changes

Comment thread pkg/cmdutil/args.go Outdated

jtmcg mentioned this pull request Feb 19, 2025

Add testing to GlobWindowsPaths iamazeem/cli#2

Merged

Add testing to GlobWindowsPaths (#2)

8748bb0

jtmcg reviewed Feb 21, 2025

View reviewed changes

Comment thread pkg/cmdutil/args.go Outdated

Support globbing for all platforms

092eb7e

iamazeem changed the title ~~[gh release create/upload] Expand glob patterns on Windows (Command Prompt and Powershell)~~ [gh release create/upload] Expand glob patterns for all platforms Feb 22, 2025

iamazeem requested a review from jtmcg February 22, 2025 07:28

jtmcg reviewed Feb 24, 2025

View reviewed changes

Comment thread pkg/cmd/gist/create/create.go Outdated

Comment thread pkg/cmdutil/args.go Outdated

iamazeem added 2 commits February 25, 2025 10:49

Remove slicing

8ace163

Add tests for - and label args

3d726c9

iamazeem requested a review from jtmcg February 25, 2025 13:00

jtmcg suggested changes Feb 25, 2025

View reviewed changes

Add exclude callback function

f4b65b7

iamazeem requested a review from jtmcg February 26, 2025 07:50

iamazeem and others added 2 commits February 27, 2025 19:20

Merge branch 'trunk' into 5099-gh-release-create-upload-expand-glob-p…

db3df04

…atterns-on-windows

jtmcg approved these changes Mar 12, 2025

View reviewed changes

jtmcg merged commit 4a8ecee into cli:trunk Mar 12, 2025

iamazeem deleted the 5099-gh-release-create-upload-expand-glob-patterns-on-windows branch March 12, 2025 16:23

This comment was marked as spam.

Sign in to view

coderabbitai Bot mentioned this pull request May 12, 2026

fix(ci): download release artifacts to isolated _artifacts/ tree sentrix-labs/sentrix#591

Merged

Search code, repositories, users, issues, pull requests...

Uh oh!

Conversation

iamazeem commented Feb 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests (same for Windows Command Prompt and Powershell)

Invalid patterns

UPDATE

Uh oh!

jtmcg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iamazeem commented Feb 10, 2025

Uh oh!

iamazeem commented Feb 11, 2025

gh gist create

Tests (Windows)

Uh oh!

iamazeem commented Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jtmcg commented Feb 13, 2025

Uh oh!

jtmcg commented Feb 13, 2025

Uh oh!

iamazeem commented Feb 14, 2025

Uh oh!

jtmcg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jtmcg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jtmcg left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

This comment was marked as spam.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

iamazeem commented Feb 9, 2025 •

edited

Loading

`gh gist create`

iamazeem commented Feb 11, 2025 •

edited

Loading

jtmcg left a comment •

edited

Loading