-
Notifications
You must be signed in to change notification settings - Fork 3
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
Cucumber Expressions AST into Regex expansion #2
Conversation
UPD: Waiting for JelteF/derive_more#175 |
Co-authored-by: Ilya Solovyiov <[email protected]>
# Conflicts: # Cargo.toml # README.md # src/ast.rs # src/combinator.rs # src/lib.rs # src/parse.rs
FCM
|
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.
@ilslv mostly OK, just bikeshedding is required.
/// | ||
/// [0]: Expression#parameter-types | ||
/// [`Error`]: ExpansionError | ||
pub fn regex_with_parameters<Input, Parameters>( |
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.
It's hard to grasp the whole idea here. Can we provide a meaningful example?
-> <SingleExpression<Input> as IntoRegexCharIter<Input>>::Iter, | ||
>, | ||
>, | ||
iter::Once<Result<char, UnknownParameterError<Input>>>, |
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.
Once we have TAIT we won't need this aliases anymore. It's worth mentioning with TODO comment for future.
src/expand/mod.rs
Outdated
// except according to those terms. | ||
|
||
//! [Cucumber Expressions][0] [AST] into [`Regex`] transformation | ||
//! definitions. |
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.
We also need to mention somewhere upstream production rules.
If we align with them 1-to-1, then they should be referred in transformation functions docs directly.
src/lib.rs
Outdated
#[cfg(feature = "expand-into-regex")] | ||
#[doc(inline)] | ||
pub use self::expand::{ | ||
ExpansionError as Error, IntoRegexCharIter, ParametersProvider, |
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.
I think we can omit re-exporting feature in root. They're quite specific to live in their on module.
src/expand/mod.rs
Outdated
SingleAlternation, SingleExpression, Spanned, | ||
}; | ||
|
||
pub use with_parameters::{ParametersProvider, WithParameters}; |
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.
Could it be?
pub mod parameters;
pub use parameters::{Provider as ParametersProvider, WithCustom as WithCustomParameters};
Moved from cucumber-rs/cucumber#154
Part of cucumber-rs/cucumber#124
Synopsis
This
PR
implementsCucumber expressions
AST
intoRegex
expansion.Solution
This is done with
IntoRegexCharIter
trait to avoid unnecessary overhead.Checklist
Draft:
prefixDraft:
prefix is removed