Browse Source

Additional polish and fixes

- fix two bugs with subs - didn't have global config and missing merge
- explicit re-watch for global config
- more docs
Gabriel Horner 3 years ago
parent
commit
7ade955df7

+ 1 - 2
deps/graph-parser/src/logseq/graph_parser/cli.cljs

@@ -34,8 +34,7 @@ TODO: Fail fast when process exits 1"
     (mapv #(assoc % :file/content (slurp (:file/path %))) files)))
 
 (defn- read-config
-  "Commandline version of frontend.handler.common/read-config without graceful
-  handling of broken config. Config is assumed to be at $dir/logseq/config.edn "
+  "Reads repo-specific config from logseq/config.edn"
   [dir]
   (let [config-file (str dir "/" gp-config/app-name "/config.edn")]
     (if (fs/existsSync config-file)

+ 3 - 1
src/electron/electron/fs_watcher.cljs

@@ -1,4 +1,7 @@
 (ns electron.fs-watcher
+  "This ns is a wrapper around the chokidar file watcher,
+  https://www.npmjs.com/package/chokidar. File watcher events are sent to the
+  `file-watcher` ipc channel"
   (:require [cljs-bean.core :as bean]
             ["fs" :as fs]
             ["chokidar" :as watcher]
@@ -52,7 +55,6 @@
   "Watch a directory if no such file watcher exists. Has the following options:
 * :current-repo-dir - Provides current repo-dir for global directories. Needed as watch events need to take place in a repo context in order for window and db to function correctly"
   [dir options]
-  (prn :WATCH dir options)
   (when-not (get @*file-watcher dir)
     (if (fs/existsSync dir)
       (let [watcher-opts (clj->js

+ 2 - 0
src/electron/electron/handler.cljs

@@ -1,4 +1,6 @@
 (ns electron.handler
+  "This ns starts the event handling for the electron main process and defines
+  all the application-specific event types"
   (:require ["electron" :refer [ipcMain dialog app autoUpdater shell]]
             [cljs-bean.core :as bean]
             ["fs" :as fs]

+ 2 - 4
src/main/frontend/components/block.cljs

@@ -2874,8 +2874,7 @@
 
 (defn built-in-custom-query?
   [title]
-  (let [repo (state/get-current-repo)
-        queries (state/sub [:config repo :default-queries :journals])]
+  (let [queries (get-in (state/sub-config) [:default-queries :journals])]
     (when (seq queries)
       (boolean (some #(= % title) (map :title queries))))))
 
@@ -2951,10 +2950,9 @@
   [state config {:keys [title query view collapsed? children? breadcrumb-show? table-view?] :as q}]
   (let [dsl-query? (:dsl-query? config)
         query-atom (:query-atom state)
-        repo (state/get-current-repo)
         query-time (or (react/get-query-time query)
                        (react/get-query-time q))
-        view-fn (if (keyword? view) (state/sub [:config repo :query/views view]) view)
+        view-fn (if (keyword? view) (get-in (state/sub-config) [:query/views view]) view)
         current-block-uuid (or (:block/uuid (:block config))
                                (:block/uuid config))
         current-block (db/entity [:block/uuid current-block-uuid])

+ 1 - 1
src/main/frontend/components/page.cljs

@@ -157,7 +157,7 @@
 (rum/defc today-queries < rum/reactive
   [repo today? sidebar?]
   (when (and today? (not sidebar?))
-    (let [queries (state/sub [:config repo :default-queries :journals])]
+    (let [queries (get-in (state/sub-config repo) [:default-queries :journals])]
       (when (seq queries)
         [:div#today-queries.mt-10
          (for [query queries]

+ 4 - 3
src/main/frontend/db/query_react.cljs

@@ -65,9 +65,10 @@
                                   result)]
                      (model/with-pages result))
                    result)
-          result-transform-fn (:result-transform q)
-          repo (state/get-current-repo)]
-      (if-let [result-transform (if (keyword? result-transform-fn) (state/sub [:config repo :query/result-transforms result-transform-fn]) result-transform-fn)]
+          result-transform-fn (:result-transform q)]
+      (if-let [result-transform (if (keyword? result-transform-fn)
+                                  (get-in (state/sub-config) [:query/result-transforms result-transform-fn])
+                                  result-transform-fn)]
         (if-let [f (sci/eval-string (pr-str result-transform))]
           (try
             (sci/call-fn f result)

+ 1 - 0
src/main/frontend/handler/config.cljs

@@ -30,6 +30,7 @@
         (file-handler/set-file-content! repo path new-content)))))
 
 (defn set-config!
+  "Sets config state for repo-specific config"
   [k v]
   (let [path (config/get-repo-config-path)]
     (repo-config-set-key-value path k v)))

+ 1 - 1
src/main/frontend/handler/events.cljs

@@ -121,7 +121,7 @@
          (route-handler/redirect-to-home!))
        (when-let [dir-name (config/get-repo-dir graph)]
          (fs/watch-dir! dir-name))
-       (global-config-handler/watch-for-global-config-dir! graph)
+       (global-config-handler/re-watch-dir! graph)
        (srs/update-cards-due-count!)
        (state/pub-event! [:graph/ready graph])
        (repo-handler/refresh-repos!)

+ 2 - 0
src/main/frontend/handler/file.cljs

@@ -194,6 +194,8 @@
   []
   (when-let [repo (state/get-current-repo)]
     (when-let [dir (config/get-repo-dir repo)]
+      ;; An unwatch shouldn't be needed on startup. However not having this
+      ;; after an app refresh can cause stale page data to load
       (fs/unwatch-dir! dir)
       (fs/watch-dir! dir))))
 

+ 16 - 12
src/main/frontend/handler/global_config.cljs

@@ -1,7 +1,7 @@
 (ns frontend.handler.global-config
-  "This ns is a system component that encapsulates global config functionality
-  and defines how to start and stop it. Unlike repo config, this manages a directory
-for configuration. This app component depends on a repo."
+  "This ns is a system component that encapsulates global config functionality.
+  Unlike repo config, this also manages a directory for configuration. This app
+  component depends on a repo."
   (:require [frontend.config :as config]
             [frontend.fs :as fs]
             [frontend.handler.common.file :as file-common-handler]
@@ -54,16 +54,20 @@ for configuration. This app component depends on a repo."
   (let [config-content (get-global-config-content repo-url)]
     (set-global-config-state! config-content)))
 
-(defn watch-for-global-config-dir!
+(defn- watch-dir!
   "Watches global config dir for given repo/db"
   [repo]
-  (let [dir (global-config-dir)
-        repo-dir (config/get-repo-dir repo)]
-    ;; Don't want multiple file watchers, especially when switching graphs
-    (fs/unwatch-dir! dir)
-    ;; Even a global dir needs to know it's current graph in order to send
-    ;; change events to the right window and graph db
-    (fs/watch-dir! dir {:current-repo-dir repo-dir})))
+  (fs/watch-dir! (global-config-dir)
+                 ;; Global dir needs to know it's current graph in order to send
+                 ;; change events to the right window and graph db
+                 {:current-repo-dir (config/get-repo-dir repo)}))
+
+(defn re-watch-dir!
+  "Rewatch global config dir for given repo/db. Unwatches dir first as we don't
+  want multiple file watchers, especially when switching graphs"
+  [repo]
+  (fs/unwatch-dir! (global-config-dir))
+  (watch-dir! repo))
 
 (defn start
   "This component has four responsibilities on start:
@@ -76,4 +80,4 @@ for configuration. This app component depends on a repo."
           _ (reset! root-dir root-dir')
           _ (restore-global-config! repo)
           _ (create-global-config-file-if-not-exists repo)
-          _ (watch-for-global-config-dir! repo)]))
+          _ (watch-dir! repo)]))

+ 8 - 4
src/main/frontend/handler/repo_config.cljs

@@ -1,8 +1,8 @@
 (ns frontend.handler.repo-config
-  "This ns is a system component that encapsulates repo config functionality and
-  defines how to start it. This component only concerns itself with one
-  user-facing repo config, logseq/config.edn. In the future it may manage more.
-  This app component depends on a repo."
+  "This ns is a system component that encapsulates repo config functionality.
+  This component only concerns itself with one user-facing repo config file,
+  logseq/config.edn. In the future it may manage more files. This app component
+  depends on a repo."
   (:require [frontend.db :as db]
             [frontend.config :as config]
             [frontend.state :as state]
@@ -18,6 +18,8 @@
   (db/get-file repo-url (config/get-repo-config-path)))
 
 (defn read-repo-config
+  "Converts file content to edn and handles read failure by backing up file and
+  reverting to a default file"
   [repo content]
   (common-handler/safe-read-string
    content
@@ -26,12 +28,14 @@
      (reader/read-string config/config-default-content))))
 
 (defn set-repo-config-state!
+  "Sets repo config state using given file content"
   [repo-url content]
   (let [config (read-repo-config repo-url content)]
     (state/set-config! repo-url config)
     config))
 
 (defn create-config-file-if-not-exists
+  "Creates a default logseq/config.edn if it doesn't exist"
   [repo-url]
   (spec/validate :repos/url repo-url)
   (let [repo-dir (config/get-repo-dir repo-url)

+ 6 - 3
src/main/frontend/state.cljs

@@ -299,7 +299,8 @@
     configs))
 
 (defn get-config
-  "User config for the given repo or current repo if none given"
+  "User config for the given repo or current repo if none given. All config fetching
+should be done through this fn in order to get global config and config defaults"
   ([]
    (get-config (get-current-repo)))
   ([repo-url]
@@ -499,11 +500,13 @@ Similar to re-frame subscriptions"
     (util/react (rum/cursor state ks))))
 
 (defn sub-config
-  "Sub equivalent to get-config"
+  "Sub equivalent to get-config which should handle all sub user-config access"
   ([] (sub-config (get-current-repo)))
   ([repo]
    (let [config (sub :config)]
-     (merge (get config ::global-config) (get config repo)))))
+     (merge-configs default-config
+                    (get config ::global-config)
+                    (get config repo)))))
 
 (defn enable-grammarly?
   []