Review comments.

core/src/lib.rs

@@ -46,8 +46,9 @@ pub use edn::{
     Utc,
     ValueRc,
 };
+
 pub use edn::parse::{
-    query as parse_query,
+    parse_query,
     ParseError as EdnParseError,
 };
 

edn/src/edn.rustpeg

@@ -471,8 +471,8 @@ query_part -> query::QueryPart
     / __ ":where" ws:where_clause+ { query::QueryPart::WhereClauses(ws) }
     / __ ":with" with_vars:variable+ { query::QueryPart::WithVars(with_vars) }
 
-pub query -> query::ParsedFindQuery
-    = __ "[" qps:query_part+ "]" __ {? query::ParsedFindQuery::from_parts(qps) }
+pub parse_query -> query::ParsedQuery
+    = __ "[" qps:query_part+ "]" __ {? query::ParsedQuery::from_parts(qps) }
 
 variable -> query::Variable
     = v:value {? query::Variable::from_value(&v).ok_or("expected variable") }

edn/src/query.rs

@@ -978,7 +978,7 @@ pub enum WhereClause {
 
 #[allow(dead_code)]
 #[derive(Debug, Eq, PartialEq)]
-pub struct ParsedFindQuery {
+pub struct ParsedQuery {
     pub find_spec: FindSpec,
     pub default_source: SrcVar,
     pub with: Vec<Variable>,
@@ -987,7 +987,6 @@ pub struct ParsedFindQuery {
     pub limit: Limit,
     pub where_clauses: Vec<WhereClause>,
     pub order: Option<Vec<Order>>,
-    // TODO: in_rules;
 }
 
 pub(crate) enum QueryPart {
@@ -999,14 +998,14 @@ pub(crate) enum QueryPart {
     Order(Vec<Order>),
 }
 
-/// A `ParsedFindQuery` represents a parsed but potentially invalid query to the query algebrizer.
+/// A `ParsedQuery` represents a parsed but potentially invalid query to the query algebrizer.
 /// Such a query is syntactically valid but might be semantically invalid, for example because
 /// constraints on the set of variables are not respected.
 ///
-/// We split `ParsedFindQuery` from `FindQuery` because it's not easy to generalize over containers
+/// We split `ParsedQuery` from `FindQuery` because it's not easy to generalize over containers
 /// (here, `Vec` and `BTreeSet`) in Rust.
-impl ParsedFindQuery {
-    pub(crate) fn from_parts(parts: Vec<QueryPart>) -> std::result::Result<ParsedFindQuery, &'static str> {
+impl ParsedQuery {
+    pub(crate) fn from_parts(parts: Vec<QueryPart>) -> std::result::Result<ParsedQuery, &'static str> {
         let mut find_spec: Option<FindSpec> = None;
         let mut with: Option<Vec<Variable>> = None;
         let mut in_vars: Option<Vec<Variable>> = None;
@@ -1055,7 +1054,7 @@ impl ParsedFindQuery {
             }
         }
 
-        Ok(ParsedFindQuery {
+        Ok(ParsedQuery {
             find_spec: find_spec.ok_or("expected :find")?,
             default_source: SrcVar::DefaultSrc,
             with: with.unwrap_or(vec![]),

edn/tests/query_tests.rs

@@ -35,7 +35,7 @@ use edn::query::{
 };
 
 use edn::parse::{
-    query as parse_query,
+    parse_query,
 };
 
 ///! N.B., parsing a query can be done without reference to a DB.
@@ -279,3 +279,22 @@ fn can_parse_uuid() {
                                 PatternNonValuePlace::Placeholder)
                        .expect("valid pattern")));
 }
+
+#[test]
+fn can_parse_exotic_whitespace() {
+    let expected = edn::Uuid::parse_str("4cb3f828-752d-497a-90c9-b1fd516d5644").expect("valid uuid");
+    // The query string from `can_parse_uuid`, with newlines, commas, and line comments interspersed.
+    let s = r#"[:find
+?x ,, :where,   ;atest
+[?x :foo/baz #uuid
+   "4cb3f828-752d-497a-90c9-b1fd516d5644", ;testa
+,],,  ,],;"#;
+    assert_eq!(parse_query(s).expect("parsed").where_clauses.pop().expect("a where clause"),
+               WhereClause::Pattern(
+                   Pattern::new(None,
+                                PatternNonValuePlace::Variable(Variable::from_valid_name("?x")),
+                                Keyword::namespaced("foo", "baz").into(),
+                                PatternValuePlace::Constant(NonIntegerConstant::Uuid(expected)),
+                                PatternNonValuePlace::Placeholder)
+                       .expect("valid pattern")));
+}

query-algebrizer/src/lib.rs

@@ -41,7 +41,7 @@ use mentat_query::{
     FindSpec,
     Limit,
     Order,
-    ParsedFindQuery,
+    ParsedQuery,
     SrcVar,
     Variable,
     WhereClause,
@@ -351,7 +351,7 @@ impl FindQuery {
         }
     }
 
-    pub fn from_parsed_find_query(parsed: ParsedFindQuery) -> Result<FindQuery> {
+    pub fn from_parsed_query(parsed: ParsedQuery) -> Result<FindQuery> {
         let in_vars = {
             let mut set: BTreeSet<Variable> = BTreeSet::default();
 
@@ -398,7 +398,6 @@ impl FindQuery {
 
 pub fn parse_find_string(string: &str) -> Result<FindQuery> {
     parse_query(string)
-        // .and_then(
         .map_err(|e| e.into())
-        .and_then(|parsed| FindQuery::from_parsed_find_query(parsed)) // .map_err(|e| e.into()))
+        .and_then(|parsed| FindQuery::from_parsed_query(parsed))
 }

query-algebrizer/src/types.rs

@@ -695,6 +695,9 @@ impl Debug for EmptyBecause {
 
 
 /// A `FindQuery` represents a valid query to the query algebrizer.
+///
+/// We split `FindQuery` from `ParsedQuery` because it's not easy to generalize over containers
+/// (here, `Vec` and `BTreeSet`) in Rust.
 #[allow(dead_code)]
 #[derive(Debug, Eq, PartialEq)]
 pub struct FindQuery {
@@ -706,7 +709,6 @@ pub struct FindQuery {
     pub limit: Limit,
     pub where_clauses: Vec<WhereClause>,
     pub order: Option<Vec<Order>>,
-    // TODO: in_rules;
 }
 
 // Intermediate data structures for resolving patterns.