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

Return empty list on invalid globs #824

Merged
merged 1 commit into from
Apr 16, 2023
Merged

Conversation

vimpostor
Copy link
Contributor

@vimpostor vimpostor commented Apr 6, 2023

As discussed in #819, this restores the previous behaviour to not fail on invalid globs, while still keeping the fix from #799.

Also see getzola/zola#2150 and several other downstream repositories, where that change caused regressions.

@vimpostor vimpostor force-pushed the emptyglob branch 2 times, most recently from 22ce96d to ce9a467 Compare April 6, 2023 07:05
@giraffate
Copy link
Contributor

This need to do a git rebase or git merge master to get CI working because of #825.

Regression was introduced in 9479c28.

Since that commit, providing an invalid glob to tera will make it fail
to create an instance, first (in that commit) by making it panic and
since 1f95878 by making it return an
error.

While returning an error is not entirely unlogical, it doesn't match
what most languages do with invalid globs.

- Bash will by default return an empty set on invalid globs, as the
  `failglob` option is off by default
- Python will likewise return the empty set instead of throwing an
  exception, when doing something like `glob.glob("/dev/null/*")`
- Rust's `globwalk` will also not error, but return an empty set

In fact, we use globwalk in tera and the only reason we panic is by
accident, because `std::fs::canonicalize()` checks the path.

It is better to match other language's glob behaviour, therefore we
resort back to the original path if `canonicalize()` fails.

We should especially restore the previous behaviour, as this caused a
lot of regressions already, including in zola.

Fixes getzola/zola#2150

ref: Keats#819
ref: Keats#820
ref: Keats#799
@Keats Keats merged commit 170e891 into Keats:master Apr 16, 2023
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