-
Notifications
You must be signed in to change notification settings - Fork 182
RUST-2157 Initial framework for autogenerated Atlas Search helpers #1451
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
Conversation
encode: EncodeType, | ||
description: String, | ||
arguments: Vec<Argument>, | ||
tests: Vec<serde_yaml::Value>, |
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.
The yaml file test definitions aren't terribly useful here, so I just cheated the parsing.
etc/gen_atlas_search/src/main.rs
Outdated
} | ||
|
||
impl Operator { | ||
fn clear_tests(mut self) -> Self { |
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.
This is to make debugging output easier to read.
[ArgumentType::SearchPath] => ArgumentRustType::StringOrArray, | ||
[ArgumentType::SearchOperator, ArgumentType::Array] => ArgumentRustType::Operator, | ||
[ArgumentType::Int] => ArgumentRustType::I32, | ||
_ => panic!("Unexpected argument types: {:?}", self.type_), |
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.
Since this is a standalone binary, I figure panic!
/ unwrap
is actually reasonable error handling.
} | ||
} | ||
|
||
impl<T> IntoIterator for AtlasSearch<T> { |
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.
This is handy for things like the methods on compound
which expect an IntoIterator<Item = AtlasSearch<T>>
, it means you can just pass in a single value without having to wrap it in vec![]
.
|
||
/// Order in which to search for tokens. | ||
#[derive(Debug, Clone, PartialEq)] | ||
pub enum TokenOrder { |
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.
Auxiliary types like this abstractly could fit better into the gen
module since that's where they're used but since these don't actually depend on the input files it seemed better to hoist them up.
proc-macro2 = "1.0.97" | ||
quote = "1.0.40" | ||
serde = { version = "1.0.219", features = ["derive"] } | ||
serde_yaml = "0.9.34" |
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.
This has been unmaintained for a while now but the alternatives aren't yet settled and it works fine for what's needed here.
} | ||
|
||
impl<T> AtlasSearch<T> { | ||
/// Erase the type of this builder. Not typically needed, but can be useful to include builders |
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.
This awkward little method is the downside of using a type parameter to differentiate the builders. However, this is only needed for situations like multiple subclauses of compound
that are themselves different types; if we used a fully distinct type for each builder they'd always need a .build()
tacked on at the end, which this avoids in all other circumstances.
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.
can you add a code example here of how this would be used?
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.
just a few doc suggestions!
} | ||
} | ||
|
||
let desc = &self.description; |
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 might be nice to add the link to the documentation as well since it's already in the yaml, i.e. something like:
let desc = format!("{self.description}\n\nFor more details, see {self.link}");
} | ||
|
||
impl<T> AtlasSearch<T> { | ||
/// Erase the type of this builder. Not typically needed, but can be useful to include builders |
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.
can you add a code example here of how this would be used?
parse_quote! { | ||
#[allow(missing_docs)] | ||
pub struct #name_ident; | ||
|
||
impl AtlasSearch<#name_ident> { | ||
#[doc = #desc] | ||
#[doc = ""] |
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.
Embedding the newline in a single docstring made rustdoc try to parse it as a code snippet for some reason 🤷
RUST-2157
This has the initial yaml file fetch and the sketch of the code generator, along with the output for three operators I've been using as a testbed to iterate on the API (
autocomplete
,text
, andcompound
). Codegen can quickly get impenetrable so I've tried to keep it reasonable; being able to useparse_quote!
rather than raw strings was a real help.Given that this is the same set of core Rust metaprogramming crates that macros use, I'm somewhat tempted to see if we can shift the action macro machinery to code generation, since that would likely knock off a chunk of compile time.