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

file.Glob: Replace stdlib Glob to support double star syntax #1919

Open
fionera opened this issue Sep 9, 2022 · 3 comments · May be fixed by #1920
Open

file.Glob: Replace stdlib Glob to support double star syntax #1919

fionera opened this issue Sep 9, 2022 · 3 comments · May be fixed by #1920
Labels
FeatureRequest New feature or request

Comments

@fionera
Copy link

fionera commented Sep 9, 2022

Is your feature request related to a problem? Please describe.
The Go stdlib implements Glob with Match as its base and does not support the double star syntax. To allow recursive lists via file.Glob it should be supported.

Describe the solution you'd like
Replace the usage of the stdlib Glob with an implementation that supports double stars like https://github.com/bmatcuk/doublestar

Describe alternatives you've considered
I didnt found any good way to list all files of an extension recursively

Additional context
https://stackoverflow.com/questions/26809484/how-to-use-double-star-glob-in-go
golang/go#11862

@fionera fionera added the Triage Requires triage/attention label Sep 9, 2022
@myitcv myitcv added FeatureRequest New feature or request and removed Triage Requires triage/attention labels Sep 10, 2022
@myitcv
Copy link
Member

myitcv commented Sep 10, 2022

Thanks for raising. Please can you edit the description above, filling out the template for a feature request:

**Is your feature request related to a problem? Please describe.**
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

**Describe the solution you'd like**
A clear and concise description of what you want to happen.

**Describe alternatives you've considered**
A clear and concise description of any alternative solutions or features you've considered.

**Additional context**
Add any other context or screenshots about the feature request here.

The "Feature Request" issue type is linked from https://github.com/cue-lang/cue/issues/new/choose for convenience.

@fionera
Copy link
Author

fionera commented Sep 10, 2022

Sure thing :) I wasnt sure if this is a feature request or a bug since CUE already has a Glob feature, but doesnt implement the full spec.

@myitcv
Copy link
Member

myitcv commented Sep 14, 2022

but doesnt implement the full spec.

The history behind that decision is covered in golang/go#11862. I think we need to first reach agreement that we want to implement/use something different for CUE. And then discuss how that should be implemented.

@myitcv myitcv added the zGarden label Jun 13, 2023
@mvdan mvdan removed the zGarden label Feb 8, 2024
cueckoo pushed a commit that referenced this issue Jul 31, 2024
Currently the double-star pattern term `**` behaves like a star `*`,
as one star matches any sequence of non-separator characters
and the second star matches nothing.

This is confusing, as one may write the pattern `**/*.json` and it can
match strings like `foo/bar.json`, making it seem like `**` works as
a double star that would also match `foo.json` or `foo/bar/baz.json`
when in fact it does not.

If we choose to support `**` patterns in the future to match separators,
this would mean changing the behavior for existing users in a subtle way
which could lead to unintended behavior or latent bugs.
It is clearer for existing users to treat `**` as an invalid pattern,
which would allow us to add the feature in the future without concern
about existing users already using such patterns without error.

We also mirror Go's path.Match behavior to consume the entire pattern
to validate that it is always syntactically valid, so that we may
use it to check for `**` even when the matching has otherwise ended.
See https://go.dev/issue/28614 for details; we largely copied these
semantics from Go 1.15 already, but we omitted a bit of code that was
necessary for trailing double-stars to be caught as an error.

This change is driven by the embed proposal, where recursively embedding
entire directory trees is a relatively common use case that we want
to support somehow, and users might be misled by `**` silently working
in a way that doesn't match the user's expectation. We want path.Match,
tool/file.Glob, and embed patterns to behave consistently, so for that
reason, start consistently treating `**` as an error.

Note that this is a breaking change for any users whose pattern
includes `**` as two consecutive wildcards, and this is on purpose
for the reasons outlined above. Users who run into this problem
may not have been aware that it behaved like a single wildcard,
and in the majority of cases they should be able to use `*` instead.

For #1919.
Fixes #3315.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: Ifeb81f4818a863cd0e4c5c13bc0fc2ac54a0f22f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198636
Unity-Result: CUE porcuepine <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants