Add validation for global config

- Handles inside and outside logseq editing
- Removed temporary catch's that are no longer needed since invalid global
  config is not allowed
- Add validation for a couple more documented config options

src/main/frontend/handler/file.cljs

@@ -85,6 +85,36 @@
     :else
     nil))
 
+(defn- validate-file
+  "Returns true if valid and if false validator displays error message. Files
+  that are not validated just return true"
+  [path content]
+  (if (and
+       (config/global-config-enabled?)
+       (= (path/dirname path) (global-config-handler/global-config-dir)))
+    (global-config-handler/validate-config-edn path content)
+    true))
+
+(defn- validate-and-write-file
+  "Validates and if valid writes file. Returns boolean indicating if file content was valid"
+  [repo path content write-file-options]
+  (let [original-content (db/get-file repo path)
+        path-dir (if (and
+                      (config/global-config-enabled?)
+                      ;; Hack until we better understand failure in error handler
+                      (global-config-handler/global-config-dir-exists?)
+                      (= (path/dirname path) (global-config-handler/global-config-dir)))
+                   (global-config-handler/global-config-dir)
+                   (config/get-repo-dir repo))
+        write-file-options' (merge write-file-options
+                                   (when original-content {:old-content original-content}))]
+    (p/do!
+     (if (validate-file path content)
+       (do
+         (fs/write-file! repo path-dir path content write-file-options')
+         true)
+       false))))
+
 ;; TODO: Remove this function in favor of `alter-files`
 (defn alter-file
   [repo path content {:keys [reset? re-render-root? from-disk? skip-compare? new-graph? verbose
@@ -94,19 +124,9 @@
                            from-disk? false
                            skip-compare? false}}]
   (let [path (gp-util/path-normalize path)
-        original-content (db/get-file repo path)
         write-file! (if from-disk?
-                      #(p/resolved nil)
-                      #(let [path-dir (if (and
-                                           (config/global-config-enabled?)
-                                           ;; Hack until we better understand failure in error handler
-                                           (global-config-handler/global-config-dir-exists?)
-                                           (= (path/dirname path) (global-config-handler/global-config-dir)))
-                                        (global-config-handler/global-config-dir)
-                                        (config/get-repo-dir repo))]
-                         (fs/write-file! repo path-dir path content
-                                        (assoc (when original-content {:old-content original-content})
-                                               :skip-compare? skip-compare?))))
+                      #(p/promise (validate-file path content))
+                      #(validate-and-write-file repo path content {:skip-compare? skip-compare?}))
         opts {:new-graph? new-graph?
               :from-disk? from-disk?
               :skip-db-transact? skip-db-transact?}
@@ -115,14 +135,14 @@
                    (when-not skip-db-transact?
                      (when-let [page-id (db/get-file-page-id path)]
                        (db/transact! repo
-                         [[:db/retract page-id :block/alias]
-                          [:db/retract page-id :block/tags]]
-                         opts)))
+                                     [[:db/retract page-id :block/alias]
+                                      [:db/retract page-id :block/tags]]
+                                     opts)))
                    (file-common-handler/reset-file! repo path content (merge opts
                                                                              (when (some? verbose) {:verbose verbose}))))
                  (db/set-file-content! repo path content opts))]
     (util/p-handle (write-file!)
-                   (fn [_]
+                   (fn [valid-result?]
                      (when re-render-root? (ui-handler/re-render-root!))
 
                      (cond
@@ -131,11 +151,13 @@
 
                        (= path (config/get-repo-config-path repo))
                        (p/let [_ (repo-config-handler/restore-repo-config! repo content)]
-                         (state/pub-event! [:shortcut/refresh]))
+                              (state/pub-event! [:shortcut/refresh]))
 
-                       (and (config/global-config-enabled?) (= path (global-config-handler/global-config-path)))
+                       (and (config/global-config-enabled?)
+                            (= path (global-config-handler/global-config-path))
+                            valid-result?)
                        (p/let [_ (global-config-handler/restore-global-config!)]
-                         (state/pub-event! [:shortcut/refresh]))))
+                              (state/pub-event! [:shortcut/refresh]))))
                    (fn [error]
                      (when (and (config/global-config-enabled?)
                                 ;; Global-config not started correctly but don't

src/main/frontend/handler/global_config.cljs

@@ -4,10 +4,17 @@
   component depends on a repo."
   (:require [frontend.fs :as fs]
             [frontend.handler.common.file :as file-common-handler]
+            [frontend.handler.notification :as notification]
+            [frontend.schema.handler.global-config :as global-config-schema]
             [frontend.state :as state]
             [cljs.reader :as reader]
             [promesa.core :as p]
             [shadow.resource :as rc]
+            [malli.error :as me]
+            [malli.core :as m]
+            [goog.string :as gstring]
+            [clojure.edn :as edn]
+            [clojure.string :as string]
             [electron.ipc :as ipc]
             ["path" :as path]))
 
@@ -56,6 +63,63 @@
     (p/let [config-content (fs/read-file config-dir config-path)]
            (set-global-config-state! config-content))))
 
+(defn- humanize-more
+  "Make error maps from me/humanize more readable for users. Doesn't try to handle
+nested keys or positional errors e.g. tuples"
+  [errors]
+  (map
+   (fn [[k v]]
+     (if (map? v)
+       [k (str "Has errors in the following keys - " (string/join ", " (keys v)))]
+       ;; Only show first error since we don't have a use case yet for multiple yet
+       [k (->> v flatten (remove nil?) first)]))
+   errors))
+
+(defn- validate-config-map
+  [m path]
+  (if-let [errors (->> m
+                       (m/explain global-config-schema/Config-edn)
+                       me/humanize
+                       seq)]
+    (do
+      (notification/show! (gstring/format "The file '%s' has the following errors:\n%s"
+                                          path
+                                          (->> errors
+                                               humanize-more
+                                               (map (fn [[k v]]
+                                                      (str k " - " v)))
+                                               (string/join "\n")))
+                          :error)
+      false)
+    true))
+
+(defn validate-config-edn
+  "Validates a global config.edn file for correctness and pops up an error
+  notification if invalid. Returns a boolean indicating if file is invalid.
+  Error messages are written with consideration that this validation is called
+  regardless of whether a file is written outside or inside Logseq."
+  [path file-body]
+  (let [parsed-body (try
+                      (edn/read-string file-body)
+                      (catch :default _ ::failed-to-read))]
+    (cond
+      (= ::failed-to-read parsed-body)
+      (do
+        (notification/show! (gstring/format "Failed to read file '%s'. Make sure your config is wrapped
+in {}. Also make sure that the characters '( { [' have their corresponding closing character ') } ]'."
+                                            path)
+                            :error)
+        false)
+      ;; Custom error message is better than malli's "invalid type" error
+      (not (map? parsed-body))
+      (do
+        (notification/show! (gstring/format "The file '%s' is not valid. Make sure the config is wrapped in {}."
+                                            path)
+                            :error)
+        false)
+      :else
+      (validate-config-map parsed-body path))))
+
 (defn start
   "This component has four responsibilities on start:
 - Fetch root-dir for later use with config paths

src/main/frontend/schema/handler/common_config.cljc

@@ -14,8 +14,11 @@
                          [:enum :journals]
                          :string]]
     [:ui/enable-tooltip? :boolean]
+    [:ui/show-brackets? :boolean]
     [:feature/enable-block-timestamps? :boolean]
     [:feature/enable-search-remove-accents? :boolean]
+    [:feature/enable-journals? :boolean]
+    [:feature/enable-flashcards? :boolean]
     [:feature/disable-scheduled-and-deadline-query? :boolean]
     [:start-of-week [:enum 0 1 2 3 4 5 6]]
     [:custom-css-url :string]
@@ -59,6 +62,7 @@
     [:property-pages/enabled? :boolean]
     [:property-pages/excludelist [:set :keyword]]
     [:property/separated-by-commas [:set :keyword]]
+    [:ignored-page-references-keywords [:set :keyword]]
     [:logbook/settings :map]
     [:mobile/photo [:map
                     [:allow-editing? {:optional true} :boolean]
@@ -66,7 +70,8 @@
     [:mobile [:map
               [:gestures/disabled-in-block-with-tags {:optional true} [:vector :string]]]]
     [:editor/extra-codemirror-options :map]
-    [:ignored-page-references-keywords [:set :keyword]]
+    [:editor/logical-outdenting? :boolean]
+    [:editor/preferred-pasting-file? :boolean]
     [:quick-capture-templates [:map
                                [:text {:optional true} :string]
                                [:media {:optional true} :string]]]

src/main/frontend/state.cljs

@@ -17,8 +17,7 @@
             [logseq.graph-parser.config :as gp-config]
             [medley.core :as medley]
             [promesa.core :as p]
-            [rum.core :as rum]
-            [logseq.graph-parser.log :as log]))
+            [rum.core :as rum]))
 
 ;; Stores main application state
 (defonce ^:large-vars/data-var state
@@ -335,19 +334,10 @@ should be done through this fn in order to get global config and config defaults
   ([]
    (get-config (get-current-repo)))
   ([repo-url]
-   (try
-     (merge-configs
-      default-config
-      (get-in @state [:config ::global-config])
-      (get-in @state [:config repo-url]))
-     (catch :default e
-       (do
-         (log/error "Cannot parse global config file" e)
-         (log/error "Restore repo config")
-         ;; NOTE: Since repo config is guarded by a try-catch, we can safely failback to it
-         (merge-configs
-          default-config
-          (get-in @state [:config repo-url])))))))
+   (merge-configs
+    default-config
+    (get-in @state [:config ::global-config])
+    (get-in @state [:config repo-url]))))
 
 (defonce publishing? (atom nil))
 
@@ -560,15 +550,9 @@ Similar to re-frame subscriptions"
   ([] (sub-config (get-current-repo)))
   ([repo]
    (let [config (sub :config)]
-     (try
-       (merge-configs default-config
-                      (get config ::global-config)
-                      (get config repo))
-       (catch :default e
-         (do
-           (log/error "Cannot parse config files" e)
-           (log/error "Restore default config")
-           default-config))))))
+     (merge-configs default-config
+                    (get config ::global-config)
+                    (get config repo)))))
 
 (defn enable-grammarly?
   []

src/test/frontend/handler/global_config_test.cljs

@@ -0,0 +1,32 @@
+(ns frontend.handler.global-config-test
+  (:require [clojure.test :refer [is testing deftest]]
+            [frontend.handler.global-config :as global-config-handler]
+            [clojure.string :as string]
+            [frontend.handler.notification :as notification]))
+
+(defn- validation-config-error-for
+  [config-body]
+  (let [error-message (atom nil)]
+      (with-redefs [notification/show! (fn [msg _] (reset! error-message msg))]
+        (is (= false
+               (global-config-handler/validate-config-edn "config.edn" config-body)))
+        (str @error-message))))
+
+(deftest validate-config-edn
+  (testing "Valid cases"
+    (is (= true
+           (global-config-handler/validate-config-edn
+            "config.edn" "{:preferred-workflow :todo}"))))
+
+  (testing "Invalid cases"
+    (is (string/includes?
+         (validation-config-error-for ":export/bullet-indentation :two-spaces")
+         "wrapped in {}"))
+
+    (is (string/includes?
+         (validation-config-error-for "{:preferred-workflow :todo")
+         "Failed to read"))
+
+    (is (string/includes?
+         (validation-config-error-for "{:start-of-week 7}")
+         "has the following errors"))))