-
-
Notifications
You must be signed in to change notification settings - Fork 262
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
fix: Search for grammar file from CARGO_MANIFEST_DIR #702
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks! I think that works -- it seems both options are covered by tests, so it should be fine. CC @MarijnS95 this fix should be backwards-compatible, but you can have a quick look.
// | ||
// If we cannot find the expected file over there, fallback to the | ||
// `CARGO_MANIFEST_DIR/src`, which is the old default and kept for convenience | ||
// reasons. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe can add a TODO with a link to https://doc.rust-lang.org/std/path/fn.absolute.html ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done :)
1a0f5ef
to
782186b
Compare
Hey @tomtau :) I kind of need this fix for a feature I currently work on. It's a bit tricky to work with this, as Thanks in advance! |
Awesome, thanks for your swift response and for taking care of this project :) |
@tomtau Fwiw I haven't looked into |
@Nukesor it's out now: https://crates.io/crates/pest/2.3.1 @MarijnS95 I included this flow using |
@tomtau Indeed, those checks are nice to have but wouldn't have caught the build failure in external crates. That said, the recent update doesn't prevent any of my crates from building so we're all good here 🎉 |
This is a proposeal to fix #325.
The new logic looks for the
[grammar = "$path"]
path inCARGO_MANIFEST_DIR/
before using the fallback ofCARGO_MANIFEST_DIR/src
.This allows projects that don't have a
src
folder to use a grammar file, while staying backward compatible.Alternative implementation approach:
One could theoretically use the absolute function.
Sadly, this function is still nightly, which makes this approach unfeasible.
Current workarounds:
src
foldersrc
folder, so one can use relative pathsgrammar_inline