-
-
Notifications
You must be signed in to change notification settings - Fork 176
fix(rust) :: format the files cargo fmt could not see
#1378
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| # ignore commits from showing up on git diffs. | ||
|
|
||
| # === large formatting commits === | ||
| # TODO :: add commit once merged into mainline | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,53 @@ | ||
| //! Built-in `SQLPage` SQL functions. | ||
| //! | ||
| //! Every function is a plain `async fn` in its own module under [`functions/`](self). To add one, | ||
| //! create `functions/<name>.rs` with an `async fn <name>` and add it to the | ||
| //! [`sqlpage_functions!`](super::function_traits::sqlpage_functions) call below. The macro declares | ||
| //! the module and adds it to the dispatch enum. Argument conversion and | ||
| //! dispatch are handled generically in [`super::function_traits`]. | ||
| //! create `functions/<name>.rs` with an `async fn <name>`, declare the module below and add it to | ||
| //! the [`sqlpage_functions!`](super::function_traits::sqlpage_functions) call. Argument conversion | ||
| //! and dispatch are handled generically in [`super::function_traits`]. | ||
|
|
||
| use std::fmt::Write; | ||
|
|
||
| use super::function_traits::sqlpage_functions; | ||
|
|
||
| mod basic_auth_password; | ||
| mod basic_auth_username; | ||
| mod client_ip; | ||
| mod configuration_directory; | ||
| mod cookie; | ||
| mod current_working_directory; | ||
| mod environment_variable; | ||
| mod exec; | ||
| mod fetch; | ||
| mod fetch_with_meta; | ||
| mod hash_password; | ||
| mod header; | ||
| mod headers; | ||
| mod hmac; | ||
| mod link; | ||
| mod oidc_logout_url; | ||
| mod path; | ||
| mod persist_uploaded_file; | ||
| mod protocol; | ||
| mod random_string; | ||
| mod read_file_as_data_url; | ||
| mod read_file_as_text; | ||
| mod regex_match; | ||
| mod request_body; | ||
| mod request_body_base64; | ||
| mod request_method; | ||
| mod run_sql; | ||
| mod send_mail; | ||
| mod set_variable; | ||
| mod uploaded_file_mime_type; | ||
| mod uploaded_file_name; | ||
| mod uploaded_file_path; | ||
| mod url_encode; | ||
| mod user_info; | ||
| mod user_info_token; | ||
| mod variables; | ||
| mod version; | ||
| mod web_root; | ||
|
Comment on lines
+12
to
+49
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, we need the macro to be able to keep the files in the functions folder and the actual functions in sync. this is documented, we can't change the behavior without changing the instructions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is some prior art to doing it this way :: apache/datafusion#9281 that said I have added a test to check for this assumption so that it pragmatically holds. While I do believe in my engineering skills, I do not consider myself an expert in Rust yet so not sure if there is a better approach here. lmk if you have any ideas, happy to iterate and learn along the way 😄 |
||
|
|
||
| sqlpage_functions! { | ||
| basic_auth_password, | ||
| basic_auth_username, | ||
|
|
@@ -82,3 +120,33 @@ fn supported_function_list() -> String { | |
| } | ||
| supported | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::SqlPageFunctionName; | ||
| use std::collections::BTreeSet; | ||
|
|
||
| #[test] | ||
| fn functions_directory_matches_registered_functions() { | ||
| let directory = concat!( | ||
| env!("CARGO_MANIFEST_DIR"), | ||
| "/src/webserver/database/sqlpage_functions/functions" | ||
| ); | ||
| let files: BTreeSet<String> = std::fs::read_dir(directory) | ||
| .expect("functions directory") | ||
| .map(|entry| entry.expect("directory entry").path()) | ||
| .filter(|path| path.extension().is_some_and(|extension| extension == "rs")) | ||
| .map(|path| { | ||
| path.file_stem() | ||
| .expect("file stem") | ||
| .to_string_lossy() | ||
| .into_owned() | ||
| }) | ||
| .collect(); | ||
| let registered: BTreeSet<String> = SqlPageFunctionName::ALL | ||
| .iter() | ||
| .map(|function| function.name().to_owned()) | ||
| .collect(); | ||
| assert_eq!(files, registered); | ||
| } | ||
| } | ||
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.
do we need this?
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.
yes, this is to ignore the formatting commit so that the blame correctly attributes the person who wrote it rather than the person who formatted it. if you don't care about that I can take it out