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

Suggestion to improve "glob" expansion in documentation #210

Open
2 tasks done
sebastien-rosset opened this issue Jun 28, 2022 · 2 comments · May be fixed by #211
Open
2 tasks done

Suggestion to improve "glob" expansion in documentation #210

sebastien-rosset opened this issue Jun 28, 2022 · 2 comments · May be fixed by #211
Labels
bug Something isn't working

Comments

@sebastien-rosset
Copy link
Contributor

sebastien-rosset commented Jun 28, 2022

Thank you for creating the issue!

  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc). This is a doc-only issue.

In the documentation at https://github.com/get-woke/woke/blame/main/docs/usage.md#L30-L33:

To change this, supply a space-separated list of globs as the first argument.

I suggest removing "as the first argument". Since the patterns are space-separated, they are interpreted by the shell as multiple arguments. woke parses all arguments, not just the first argument.

This can be something like **/*.go, or a space-separated list of filenames.

It would be worth clarifying the glob expansion is performed by the shell. woke itself is not doing any globbing expansion. In particular, given the ambiguity of the documentation, one might expect the pattern below to be expanded by woke, but it's not.

woke '**/*.go'

This is a bit confusing because different shells have different globbing behaviors. Some shells do not support the double star glob pattern. In bash >= 4, the double star is used as an extended file match globbing operator, meaning it matches filenames and directories recursively. However, the exact behavior depends on the globstar value:

shopt -s globstar ; woke **/*.go
shopt -u globstar ; woke **/*.go

How about adding support for glob expansion in woke? This would provide consistent and deterministic results that do not depend on the shell glob expansion.

Please include the following information:

Version of woke
$ woke --version
# paste output here
woke version 0.18.2
Config file
$ cat .woke.yml
# paste output here
N/A
Go environment
$ go version && go env
# paste output here
N/A
Verbose output of running
$ woke --debug
# paste output here
N/A
@sebastien-rosset sebastien-rosset added the bug Something isn't working label Jun 28, 2022
@github-actions
Copy link
Contributor

👋 Thanks for submitting your first issue!

Please be sure to read and follow our Code of Conduct and Contributing guide.

⭐️ Is your org or open source project using woke? If so, we'd love for you to be included in the 'Who uses woke' list at https://github.com/get-woke/woke/blob/main/docs/about.md#who-uses-woke.

@sebastien-rosset
Copy link
Contributor Author

What do you think about supporting ** glob expansion using one of the libraries listed here: https://www.client9.com/golang-globs-and-the-double-star-glob-operator/?

  1. mattn/go-zglob looks very optimized for filewalking
  2. bmatcuk/doublestar appears to be the most mature
  3. godo/glob.go is an implementation embedded inside godo

@sebastien-rosset sebastien-rosset linked a pull request Jun 28, 2022 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant