Browse Source

Minor clean up and help with feedback

- basic clj-kondo config and cleanup unused binding
- Add some unit tests
- Add a basic readme
Gabriel Horner 2 years ago
parent
commit
06fa65861f

+ 9 - 0
deps/common/.clj-kondo/config.edn

@@ -0,0 +1,9 @@
+{:linters
+ {:aliased-namespace-symbol {:level :warning}
+  :namespace-name-mismatch {:level :warning}
+  :used-underscored-binding {:level :warning}
+
+  :consistent-alias
+  {:aliases {clojure.string string}}}
+ :skip-comments true
+ :output {:progress true}}

+ 1 - 0
deps/common/.gitignore

@@ -0,0 +1 @@
+.clj-kondo/.cache

+ 29 - 0
deps/common/README.md

@@ -0,0 +1,29 @@
+## Description
+
+This library provides common util namespaces to share between the frontend and
+other non-frontend namespaces. This library is not supposed to depend on other logseq
+libraries.
+
+## API
+
+This library is under the parent namespace `logseq.common`.
+
+## Dev
+
+This follows the practices that [the Logseq frontend
+follows](/docs/dev-practices.md). Most of the same linters are used, with
+configurations that are specific to this library. See [this library's CI
+file](/.github/workflows/logseq-common.yml) for linting examples.
+
+### Testing
+
+To run ClojureScript tests:
+```
+clojure -M:test
+```
+
+To auto-run tests while writing tests:
+
+```
+clojure -M:test -w src
+```

+ 4 - 3
deps/common/src/logseq/common/path.cljs

@@ -23,7 +23,7 @@
 
 (defn filename
   "File name of a path or URL.
-   Returns nil when it's a directory."
+   Returns nil when it's a directory that ends with '/'."
   [path]
   (let [fname (if (string/ends-with? path "/")
                 nil
@@ -159,8 +159,9 @@
   [base & segments]
 
   (cond
-    (nil? base)
-    (js/console.log "path join with nil global directory" segments)
+    ;; For debugging
+    ; (nil? base)
+    ; (js/console.log "path join with nil global directory" segments)
     (= base "")
     (js/console.error "BUG: should not join with empty dir" segments)
     :else

+ 14 - 2
deps/common/test/logseq/common/path_test.cljs

@@ -2,7 +2,17 @@
   (:require [cljs.test :refer [deftest is testing]]
             [logseq.common.path :as path]))
 
-(deftest test-safe-file-name?
+(deftest filename
+  (is (= nil (path/filename "/path/to/dir/")))
+  (is (= "file-name" (path/filename "/path/to/dir/file-name")))
+  (is (= "file-name" (path/filename "dir/file-name"))))
+
+(deftest split-ext
+  (is (= ["some-song" "mp3"] (path/split-ext "some-song.MP3")))
+  (is (= ["some-song" ""] (path/split-ext "some-song")))
+  (is (= ["some-file.edn" "txt"] (path/split-ext "some-file.edn.txt"))))
+
+(deftest safe-file-name?
   (testing "safe-file-name"
     (is (path/safe-filename? "foo"))
     (is (path/safe-filename? "foo bar"))
@@ -18,9 +28,11 @@
 
 
 (deftest path-join
-  (testing "join-path"
+  (testing "path-join"
     (is (= "foo/bar" (path/path-join "foo" "bar")))
     (is (= "foo/bar" (path/path-join "foo/" "bar")))
+    (is (= "foo/bar" (path/path-join nil "foo" "bar"))
+        "global dir")
     (is (= "/foo/bar/baz/asdf" (path/path-join "/foo/bar//baz/asdf/quux/..")))
     (is (= "assets:///foo.bar/baz" (path/path-join "assets:///foo.bar" "baz")))
     (is (= "assets:///foo.bar/baz" (path/path-join "assets:///foo.bar/" "baz")))))

+ 7 - 0
docs/dev-practices.md

@@ -246,6 +246,13 @@ page](https://github.com/metosin/malli/blob/master/docs/clojurescript-function-i
 
 Currently the codebase is not formatted/indented consistently. We loosely follow https://github.com/bbatsov/clojure-style-guide. [cljfmt](https://cljdoc.org/d/cljfmt/) is a common formatter used for Clojure, analogous to Prettier for other languages. You can do so easily with the [Calva](https://marketplace.visualstudio.com/items?itemName=betterthantomorrow.calva) extension in [VSCode](https://code.visualstudio.com/): It will (mostly) indent your code correctly as you type, and you can move your cursor to the start of the line(s) you've written and press `Tab` to auto-indent all Clojure forms nested under the one starting on the current line.
 
+## Naming
+
+We strive to use explicit names that are self explanatory so that our codebase is readable and maintainable. Sometimes we use abbreviations for frequently occurring concepts. Some common abbreviations:
+
+* `rpath` - Relative path e.g. `logseq/config.edn`
+* `fpath` -  Full path e.g. `/full/path/to/logseq/config.edn`
+
 ## Development Tools
 
 ### Babashka tasks

+ 2 - 4
src/main/frontend/config.cljs

@@ -432,10 +432,8 @@
     (util/node-path.join (get-repo-dir repo-url) path)))
 
 (defn get-repo-config-path
-  ([]
-   (get-repo-config-path (state/get-current-repo)))
-  ([_repo]
-   (path/path-join app-name config-file)))
+  []
+  (path/path-join app-name config-file))
 
 (defn get-custom-css-path
   ([]

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

@@ -286,7 +286,7 @@
   (spec/validate :repos/url repo-url)
   (route-handler/redirect-to-home!)
   (state/set-parsing-state! {:graph-loading? true})
-  (let [config (or (when-let [content (some-> (first (filter #(= (config/get-repo-config-path repo-url) (:file/path %)) file-objs))
+  (let [config (or (when-let [content (some-> (first (filter #(= (config/get-repo-config-path) (:file/path %)) file-objs))
                                               :file/content)]
                      (repo-config-handler/read-repo-config content))
                    (state/get-config repo-url))