Misc cleanup and test improvements

- Remove incorrect carve entry
- Move test-only code for extract-properties to test
- Remove unused if branches in extract-properties
- Move json parsing of get-references into mldoc to be consistent about
  json encapsulation like other mldoc fns
- reorganize relevant unit tests
- add tests for alias, tags and comma separated config properties
- Update incorrect docstring

deps/graph-parser/.carve/ignore

@@ -25,8 +25,6 @@ logseq.graph-parser.config/remove-asset-protocol
 ;; API
 logseq.graph-parser.util/unquote-string
 ;; API
-logseq.graph-parser.util/wrapped-by-quotes?
-;; API
 logseq.graph-parser.util.page-ref/page-ref-re
 ;; API
 logseq.graph-parser.property/->block-content

deps/graph-parser/src/logseq/graph_parser/block.cljs

@@ -182,18 +182,9 @@
          distinct)))
 
 (defn extract-properties
-  [properties user-config & {:keys [format]
-                             :or {format :markdown}}]
+  [properties user-config]
   (when (seq properties)
     (let [properties (seq properties)
-          properties (if (= 2 (count (first properties))) ; property value not parsed yet
-                       (map
-                         (fn [[k v]]
-                           (let [mldoc-ast (-> (gp-mldoc/get-references v (gp-mldoc/default-config format))
-                                               gp-util/json->clj)]
-                             [k v mldoc-ast]))
-                         properties)
-                       properties)
           *invalid-properties (atom #{})
           properties (->> properties
                           (map (fn [[k v mldoc-ast]]
@@ -205,15 +196,12 @@
                                              (string/replace "_" "-")
                                              (string/replace #"[\"|^|(|)|{|}]+" ""))]
                                    (if (gp-property/valid-property-name? (str ":" k))
-                                     (let [k (if (contains? #{"custom_id" "custom-id"} k)
-                                               "id"
-                                               k)
-                                           v (if (coll? v)
-                                               (remove string/blank? v)
-                                               (text/parse-property k v mldoc-ast user-config))
-                                           k (keyword k)
-                                           v (if (coll? v) (set v) v)]
-                                       [k v mldoc-ast])
+                                     (let [k' (keyword
+                                               (if (contains? #{"custom_id" "custom-id"} k)
+                                                 "id"
+                                                 k))
+                                           v' (text/parse-property k v mldoc-ast user-config)]
+                                       [k' v' mldoc-ast])
                                      (do (swap! *invalid-properties conj k)
                                          nil)))))
                           (remove #(nil? (second %))))
@@ -627,7 +615,7 @@
                   (recur headings (rest blocks) timestamps' properties body))
 
                 (gp-property/properties-ast? block)
-                (let [properties (extract-properties (second block) user-config :format format)]
+                (let [properties (extract-properties (second block) user-config)]
                   (recur headings (rest blocks) timestamps properties body))
 
                 (heading-block? block)

deps/graph-parser/src/logseq/graph_parser/mldoc.cljc

@@ -43,7 +43,7 @@
 (defn get-references
   [text config]
   (when-not (string/blank? text)
-    (getReferences text config)))
+    (gp-util/json->clj (getReferences text config))))
 
 (defn ast-export-markdown
   [ast config references]
@@ -110,7 +110,7 @@
           properties (map (fn [[_directive k v]]
                             (let [kname (string/lower-case k)
                                   k (keyword kname)
-                                  mldoc-ast (-> (get-references v config) gp-util/json->clj)]
+                                  mldoc-ast (get-references v config)]
                               [k v mldoc-ast]))
                        properties-ast)]
       (if (seq properties)

deps/graph-parser/src/logseq/graph_parser/text.cljs

@@ -92,7 +92,7 @@
     (re-find #"^\d+$" v)
     (parse-long v)))
 
-(defn get-ref-from-ast
+(defn- get-ref-from-ast
   [[typ data]]
   (case typ
     "Link"
@@ -147,9 +147,10 @@
         property-separated-by-commas? (and (separated-by-commas? config-state k)
                                            (empty? refs))
         refs' (if property-separated-by-commas?
-                (if (string/includes? v ",")
-                  (distinct (sep-by-comma v))
-                  [(string/trim v)])
+                (set
+                 (if (string/includes? v ",")
+                   (distinct (sep-by-comma v))
+                   [(string/trim v)]))
                 refs)
         k (if (or (symbol? k) (keyword? k)) (subs (str k) 1) k)
         v (if (or (symbol? v) (keyword? v))

deps/graph-parser/test/logseq/graph_parser/block_test.cljs

@@ -1,9 +1,20 @@
 (ns logseq.graph-parser.block-test
   (:require [logseq.graph-parser.block :as gp-block]
+            [logseq.graph-parser.mldoc :as gp-mldoc]
             [cljs.test :refer [deftest are testing is]]))
 
+(defn- extract-properties
+  [properties user-config]
+  (gp-block/extract-properties
+   (map
+    (fn [[k v]]
+      (let [mldoc-ast (gp-mldoc/get-references v (gp-mldoc/default-config :markdown))]
+        [k v mldoc-ast]))
+    properties)
+   user-config))
+
 (deftest test-extract-properties
-  (are [x y] (= (:properties (gp-block/extract-properties x {})) y)
+  (are [x y] (= (:properties (extract-properties x {})) y)
        ;; Built-in properties
        [["background-color" "#000000"]] {:background-color "#000000"}
        [["alias" "[[name/with space]]"]] {:alias #{"name/with space"}}
@@ -32,7 +43,7 @@
 
   (testing "page-refs"
     (are [x y] (= (vec (:page-refs
-                        (gp-block/extract-properties x {:property-pages/enabled? true}))) y)
+                        (extract-properties x {:property-pages/enabled? true}))) y)
          [["year" "1000"]] ["year"]
          [["year" "\"1000\""]] ["year"]
          [["year" "1000"] ["month" "12"]] ["year" "month"]
@@ -44,27 +55,27 @@
 
 
     (are [x y] (= (vec (:page-refs
-                        (gp-block/extract-properties x {:property-pages/enabled? false}))) y)
+                        (extract-properties x {:property-pages/enabled? false}))) y)
          [["year" "1000"]] []
          [["year" "1000"] ["month" "12"]] []
          [["foo" "[[bar]] test"]] ["bar"])
 
     (is (= ["year"]
            (:page-refs
-            (gp-block/extract-properties [["year" "1000"] ["month" "12"]]
+            (extract-properties [["year" "1000"] ["month" "12"]]
                                          {:property-pages/enabled? true
                                           :property-pages/excludelist #{:month :day}})))
         ":property-pages/exclude-list excludes specified properties")
 
     (is (= ["year"]
            (:page-refs
-            (gp-block/extract-properties [["year" "1000"]]
+            (extract-properties [["year" "1000"]]
                                          {})))
         "Default to enabled when :property-pages/enabled? is not in config")
 
     (is (= ["foo" "bar"]
            (:page-refs
-            (gp-block/extract-properties
+            (extract-properties
              ;; tags is linkable and background-color is not
              [["tags" "[[foo]], [[bar]]"] ["background-color" "#008000"]]
                                          {:property-pages/enabled? true})))

deps/graph-parser/test/logseq/graph_parser/mldoc_test.cljs

@@ -79,21 +79,13 @@
 (deftest md-properties-test
   (are [x y] (= y (get-properties x))
 
-    ;; comma separates values
+    ;; reference values
     "property:: [[foo]], [[bar]]"
     {:property #{"foo" "bar"}}
 
-    ;; alias property
-    "alias:: [[foo]], #bar"
-    {:alias #{"foo" "bar"}}
-
-    ;; tags property
-    "tags:: #foo,#bar,#foo"
-    {:tags #{"foo" "bar"}}
-
-    ;; title property
-    "title:: comma, is ok"
-    {:title "comma, is ok"}))
+    ;; comma separated
+    "tags:: foo, bar, foo"
+    {:tags #{"foo" "bar"}}))
 
 (deftest name-definition-test
   (is (= [["List"

deps/graph-parser/test/logseq/graph_parser/text_test.cljs

@@ -1,8 +1,7 @@
 (ns logseq.graph-parser.text-test
   (:require [cljs.test :refer [are deftest testing]]
             [logseq.graph-parser.text :as text]
-            [logseq.graph-parser.mldoc :as gp-mldoc]
-            [logseq.graph-parser.util :as gp-util]))
+            [logseq.graph-parser.mldoc :as gp-mldoc]))
 
 (deftest test-get-page-name
   []
@@ -62,27 +61,41 @@
       "*********************foobar" "foobar")))
 
 (defn- parse-property
-  [k v]
-  (let [references (-> (gp-mldoc/get-references v (gp-mldoc/default-config :markdown))
-                       (gp-util/json->clj))]
-    (text/parse-property k v references {})))
+  [k v user-config]
+  (let [references (gp-mldoc/get-references v (gp-mldoc/default-config :markdown))]
+    (text/parse-property k v references user-config)))
 
 (deftest test-parse-property
-  (testing "parse-property"
-    (are [k v y] (= (parse-property k v) y)
-      :tags "foo" ["foo"]
-      :tags "[[foo]], [[bar]]" #{"foo" "bar"}
-      :tags "[[foo]],[[bar]]" #{"foo" "bar"}
-      :tags "[[foo]]" #{"foo"}
-      :tags "[[foo]] [[bar]]" #{"foo" "bar"}
-      :tags "[[foo]], [[bar]]" #{"foo" "bar"}
-      :tags "[[foo]], [[bar]], #baz" #{"foo" "bar" "baz"}
-      :tags "#baz, [[foo]], [[bar]]" #{"foo" "bar" "baz"}
-      :tags "[[foo [[bar]]]]" #{"foo [[bar]]"}
-      :tags "[[foo [[bar]]]], [[baz]]" #{"baz" "foo [[bar]]"}))
+  (testing "for default comma separated properties"
+    (are [k v y] (= (parse-property k v {}) y)
+         :tags "foo" #{"foo"}
+         :tags "comma, separated" #{"comma" "separated"}
+         :alias "one, two, one" #{"one" "two"}))
+
+  (testing "for user comma separated properties"
+    (are [k v y] (= (parse-property k v {:property/separated-by-commas #{:comma-prop}}) y)
+         :comma-prop "foo" #{"foo"}
+         :comma-prop "comma, separated" #{"comma" "separated"}
+         :comma-prop "one, two, one" #{"one" "two"}))
+
+  (testing "for normal properties"
+    (are [k v y] (= (parse-property k v {}) y)
+         :normal "[[foo]] [[bar]]" #{"foo" "bar"}
+         :normal "[[foo]], [[bar]]" #{"foo" "bar"}
+         :normal "[[foo]]" #{"foo"}
+         :normal "[[foo]], [[bar]], #baz" #{"foo" "bar" "baz"}
+         :normal "[[foo [[bar]]]]" #{"foo [[bar]]"}
+         :normal "[[foo [[bar]]]], [[baz]]" #{"baz" "foo [[bar]]"}
+         :title "comma, is ok" "comma, is ok"))
+
+  (testing "for tags in properties with punctuation"
+    (are [k v y] (= (parse-property k v {}) y)
+         :prop "#foo, #bar. #baz!" #{"foo" "bar" "baz"}
+         :prop "#foo: '#bar'" #{"foo" "bar"}))
+
   (testing "parse-property with quoted strings"
-    (are [k v y] (= (parse-property k v) y)
-      :tags "\"foo, bar\"" "\"foo, bar\""
-      :tags "\"[[foo]], [[bar]]\"" "\"[[foo]], [[bar]]\"")))
+    (are [k v y] (= (parse-property k v {}) y)
+         :tags "\"foo, bar\"" "\"foo, bar\""
+         :tags "\"[[foo]], [[bar]]\"" "\"[[foo]], [[bar]]\"")))
 
 #_(cljs.test/test-ns 'logseq.graph-parser.text-test)

deps/graph-parser/test/logseq/graph_parser_test.cljs

@@ -70,13 +70,12 @@
   (testing "Sort order and persistence of 10 properties"
     (test-property-order 10)))
 
-(defn- quoted-property-values-test
-  [user-config]
+(deftest quoted-property-values
   (let [conn (ldb/start-conn)
         _ (graph-parser/parse-file conn
                                    "foo.md"
                                    "- desc:: \"#foo is not a ref\""
-                                   {:extract-options {:user-config user-config}})
+                                   {:extract-options {:user-config {}}})
         block (->> (d/q '[:find (pull ?b [* {:block/refs [*]}])
                        :in $
                        :where [?b :block/properties]]
@@ -90,45 +89,55 @@
            (map :block/original-name (:block/refs block)))
         "No refs from property value")))
 
-(deftest quoted-property-values
-  (testing "With default config"
-    (quoted-property-values-test {})))
-
-(deftest page-properties-persistence
-  (testing "Non-string property values"
-    (let [conn (ldb/start-conn)]
-      (graph-parser/parse-file conn
-                               "lythe-of-heaven.md"
-                               "rating:: 8\nrecommend:: true\narchive:: false"
-                               {})
-      (is (= {:rating 8 :recommend true :archive false}
-             (->> (d/q '[:find (pull ?b [*])
-                         :in $
-                         :where [?b :block/properties]]
-                       @conn)
-                  (map (comp :block/properties first))
-                  first)))))
+(deftest non-string-property-values
+  (let [conn (ldb/start-conn)]
+    (graph-parser/parse-file conn
+                             "lythe-of-heaven.md"
+                             "rating:: 8\nrecommend:: true\narchive:: false"
+                             {})
+    (is (= {:rating 8 :recommend true :archive false}
+           (->> (d/q '[:find (pull ?b [*])
+                       :in $
+                       :where [?b :block/properties]]
+                     @conn)
+                (map (comp :block/properties first))
+                first)))))
 
-  (testing "Linkable built-in properties"
-    (let [conn (ldb/start-conn)
-          _ (graph-parser/parse-file conn
-                                     "lol.md"
-                                     "alias:: 233\ntags:: fun, facts"
-                                     {})
-          block (->> (d/q '[:find (pull ?b [:block/properties {:block/alias [:block/name]} {:block/tags [:block/name]}])
-                            :in $
-                            :where [?b :block/name "lol"]]
-                          @conn)
-                     (map first)
-                     first)]
-
-      (is (= {:block/alias [{:block/name "233"}]
-              :block/tags [{:block/name "fun"} {:block/name "facts"}]
-              :block/properties {:alias #{"233"} :tags #{"fun" "facts"}}}
-             block))
-
-      (is (every? set? (vals (:block/properties block)))
-          "Linked built-in property values as sets provides for easier transforms"))))
+(deftest linkable-built-in-properties
+  (let [conn (ldb/start-conn)
+        _ (graph-parser/parse-file conn
+                                   "lol.md"
+                                   (str "alias:: 233\ntags:: fun, facts"
+                                        "\n- "
+                                        "alias:: 666\ntags:: block, facts")
+                                   {})
+        page-block (->> (d/q '[:find (pull ?b [:block/properties {:block/alias [:block/name]} {:block/tags [:block/name]}])
+                               :in $
+                               :where [?b :block/name "lol"]]
+                             @conn)
+                        (map first)
+                        first)
+        block (->> (d/q '[:find (pull ?b [:block/properties])
+                          :in $
+                          :where
+                          [?b :block/properties]
+                          [(missing? $ ?b :block/pre-block?)]
+                          [(missing? $ ?b :block/name)]]
+                        @conn)
+                   (map first)
+                   first)]
+
+    (is (= {:block/alias [{:block/name "233"}]
+            :block/tags [{:block/name "fun"} {:block/name "facts"}]
+            :block/properties {:alias #{"233"} :tags #{"fun" "facts"}}}
+           page-block)
+        "page properties, alias and tags are correct")
+    (is (every? set? (vals (:block/properties page-block)))
+        "Linked built-in property values as sets provides for easier transforms")
+
+    (is (= {:block/properties {:alias #{"666"} :tags #{"block" "facts"}}}
+           block)
+        "block properties are correct")))
 
 (defn- property-relationships-test
   "Runs tests on page properties and block properties. file-properties is what is

src/main/frontend/format/mldoc.cljs

@@ -50,7 +50,8 @@
       [])))
 
 (defn ->edn
-  "Wrapper around gp-mldoc/->edn which provides config state"
+  "Alias to gp-mldoc/->edn but could serve as a wrapper e.g. handle
+  gp-mldoc/default-config"
   [content config]
   (gp-mldoc/->edn content config))