Enhance: Add more translations, remove unused ones and add linter for…

… unused (logseq#8568) * chore: remove unused dictionary keys * wip: add dictionary keys for hardcoded strings * resolve conflicts * Add linter to detect :en ui translation keys match used ones - Fix a couple entries caught by linter - Copy :command.editor/open-link-in-sidebar entries to :help/open-link-in-sidebar as translation keys shouldn't be reused in multiple contexts, especially when it's across ui and shortcut dicts * fix: remove dead keys * chore: reuse dict keys * chore: reintroduce useful keys --------- Co-authored-by: Gabriel Horner <[email protected]>
samuelbaizg · Feb 24, 2023 · 2cace88 · 2cace88
1 parent ec157b8
commit 2cace88
Show file tree

Hide file tree

Showing 15 changed files with 145 additions and 1,539 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -114,7 +114,7 @@ jobs:
         run: bb lint:ns-docstrings 2>/dev/null
 
       - name: Lint invalid translation entries
-        run: bb lang:invalid-translations
+        run: bb lang:validate-translations
 
   e2e-test:
     runs-on: ubuntu-latest

diff --git a/bb.edn b/bb.edn
@@ -99,8 +99,8 @@
   lang:duplicates
   logseq.tasks.lang/list-duplicates
 
-  lang:invalid-translations
-  logseq.tasks.lang/invalid-translations
+  lang:validate-translations
+  logseq.tasks.lang/validate-translations
 
   file-sync:integration-tests
   logseq.tasks.file-sync/integration-tests}

diff --git a/docs/contributing-to-translations.md b/docs/contributing-to-translations.md
@@ -84,9 +84,9 @@ Keys with duplicate values found:
 
 ## Fix Mistakes
 
-Sometimes, we typo the translation key. If that happens, the github CI step of
-`bb lang:invalid-translations` will detect this error and helpfully show you
-what was typoed.
+Sometimes, we typo a translation key or forget to use it. If this happens,
+the github CI step of `bb lang:validate-translations` will detect these errors
+and tell you what's wrong.
 
 ## Add a Language
 

diff --git a/scripts/src/logseq/tasks/dev.clj b/scripts/src/logseq/tasks/dev.clj
@@ -16,7 +16,7 @@
   (doseq [cmd ["clojure -M:clj-kondo --parallel --lint src --cache false"
                "bb lint:carve"
                "bb lint:large-vars"
-               "bb lang:invalid-translations"
+               "bb lang:validate-translations"
                "bb lint:ns-docstrings"]]
     (println cmd)
     (shell cmd)))

diff --git a/scripts/src/logseq/tasks/lang.clj b/scripts/src/logseq/tasks/lang.clj
@@ -1,9 +1,11 @@
 (ns logseq.tasks.lang
   "Tasks related to language translations"
   (:require [clojure.set :as set]
+            [clojure.string :as string]
             [frontend.dicts :as dicts]
             [frontend.modules.shortcut.dicts :as shortcut-dicts]
-            [logseq.tasks.util :as task-util]))
+            [logseq.tasks.util :as task-util]
+            [babashka.process :refer [shell]]))
 
 (defn- get-dicts
   []
@@ -69,8 +71,11 @@
            (sort-by (juxt :file :translation-key))
            task-util/print-table))))
 
-(defn invalid-translations
-  "Lists translation that don't exist in English"
+(defn- validate-non-default-languages
+  "This validation finds any translation keys that don't exist in the default
+  language English. Logseq needs to work out of the box with its default
+  language. This catches mistakes where another language has accidentally typoed
+  keys or added ones without updating :en"
   []
   (let [dicts (get-all-dicts)
         ;; For now defined as :en but clj-kondo analysis could be more thorough
@@ -83,12 +88,68 @@
                         (set/difference (set (keys get-dicts))
                                         valid-keys)))))]
     (if (empty? invalid-dicts)
-      (println "All translations have valid keys!")
+      (println "All non-default translations have valid keys!")
       (do
-        (println "Invalid translation keys found:")
+        (println "\nThese translation keys are invalid because they don't exist in English:")
         (task-util/print-table invalid-dicts)
         (System/exit 1)))))
 
+;; Command to check for manual entries:
+;; grep -E -oh  '\(t [^ ):]+' -r src/main
+(def manual-ui-dicts
+  "Manual list of ui translations because they are dynamic i.e. keyword isn't
+  first arg. Only map values are used in linter as keys are for easily scanning
+  grep result."
+
+  {"(t (shortcut-helper/decorate-namespace" [] ;; shortcuts related so can ignore
+   "(t (keyword" [:color/yellow :color/red :color/pink :color/green :color/blue
+                  :color/purple :color/gray]
+   ;; from 3 files
+   "(t (if" [:asset/show-in-folder :asset/open-in-browser
+             :search-item/whiteboard :search-item/page
+             :page/make-private :page/make-public]
+   "(t (name" [] ;; shortcuts related
+   "(t (dh/decorate-namespace" [] ;; shortcuts related
+   "(t prompt-key" [:select/default-prompt :select.graph/prompt]
+   ;; All args to ui/make-confirm-modal are not keywords
+   "(t title" []
+   "(t subtitle" [:asset/physical-delete]})
+
+(defn- validate-ui-translations-are-used
+  "This validation checks to see that translations done by (t ...) are equal to
+  the ones defined for the default :en lang. This catches translations that have
+  been added in UI but don't have an entry or translations no longer used in the UI"
+  []
+  (let [actual-dicts (->> (shell {:out :string}
+                                 ;; This currently assumes all ui translations
+                                 ;; use (t and src/main. This can easily be
+                                 ;; tweaked as needed
+                                 "grep -E -oh '\\(t :[^ )]+' -r src/main")
+                          :out
+                          string/split-lines
+                          (map #(keyword (subs % 4)))
+                          (concat (mapcat val manual-ui-dicts))
+                          set)
+        expected-dicts (set (keys (:en (get-dicts))))
+        actual-only (set/difference actual-dicts expected-dicts)
+        expected-only (set/difference expected-dicts actual-dicts)]
+    (if (and (empty? actual-only) (empty? expected-only))
+      (println "All defined :en translation keys match the ones that are used!")
+      (do
+        (when (seq actual-only)
+          (println "\nThese translation keys are invalid because they are used in the UI but not defined:")
+          (task-util/print-table (map #(hash-map :invalid-key %) actual-only)))
+        (when (seq expected-only)
+          (println "\nThese translation keys are invalid because they are not used in the UI:")
+          (task-util/print-table (map #(hash-map :invalid-key %) expected-only)))
+        (System/exit 1)))))
+
+(defn validate-translations
+  "Runs multiple translation validations that fail fast if one of them is invalid"
+  []
+  (validate-non-default-languages)
+  (validate-ui-translations-are-used))
+
 (defn list-duplicates
   "Lists translations that are the same as the one in English"
   [& args]

diff --git a/src/main/frontend/components/content.cljs b/src/main/frontend/components/content.cljs
@@ -33,7 +33,7 @@
    (ui/menu-link
     {:key "cut"
      :on-click #(editor-handler/cut-selection-blocks true)}
-    "Cut"
+    (t :content/cut)
     nil)
    (ui/menu-link
     {:key      "delete"
@@ -44,7 +44,7 @@
    (ui/menu-link
     {:key "copy"
      :on-click editor-handler/copy-selection-blocks}
-    "Copy"
+    (t :content/copy)
     nil)
    (ui/menu-link
     {:key "copy as"
@@ -236,7 +236,7 @@
           {:key      "Cut"
            :on-click (fn [_e]
                        (editor-handler/cut-block! block-id))}
-          "Cut"
+          (t :content/cut)
           nil)
 
          (ui/menu-link
@@ -400,7 +400,7 @@
   [:div {:id id}
    (if hiccup
      hiccup
-     [:div.cursor "Click to edit"])])
+     [:div.cursor (t :content/click-to-edit)])])
 
 (rum/defc non-hiccup-content < rum/reactive
   [id content on-click on-hide config format]
@@ -422,7 +422,7 @@
          {:id id
           :on-click on-click}
          (if (string/blank? content)
-           [:div.cursor "Click to edit"]
+           [:div.cursor (t :content/click-to-edit)]
            content)]))))
 
 (defn- set-draw-iframe-style!

diff --git a/src/main/frontend/components/export.cljs b/src/main/frontend/components/export.cljs
@@ -14,7 +14,7 @@
   []
   (when-let [current-repo (state/get-current-repo)]
     [:div.export
-     [:h1.title "Export"]
+     [:h1.title (t :export)]
      [:ul.mr-1
       [:li.mb-4
        [:a.font-medium {:on-click #(export/export-repo-as-edn-v2! current-repo)}

diff --git a/src/main/frontend/components/shortcut.cljs b/src/main/frontend/components/shortcut.cljs
@@ -104,7 +104,7 @@
      [:td.text-left (t :help/block-reference)]
      [:td.text-right [:code block-ref/left-and-right-parens]]]
     [:tr
-     [:td.text-left (t :command.editor/open-link-in-sidebar)]
+     [:td.text-left (t :help/open-link-in-sidebar)]
      [:td.text-right (ui/render-keyboard-shortcut ["shift" "click"])]]
     [:tr
      [:td.text-left (t :help/context-menu)]

diff --git a/src/main/frontend/components/theme.cljs b/src/main/frontend/components/theme.cljs
@@ -11,7 +11,8 @@
             [frontend.components.settings :as settings]
             [frontend.rum :refer [use-mounted]]
             [frontend.storage :as storage]
-            [rum.core :as rum]))
+            [rum.core :as rum]
+            [frontend.context.i18n :refer [t]]))
 
 (rum/defc container
   [{:keys [route theme on-click current-repo nfs-granted? db-restoring?
@@ -60,7 +61,7 @@
     (rum/use-effect!
      #(let [db-restored? (false? db-restoring?)]
         (if db-restoring?
-          (util/set-title! "Loading")
+          (util/set-title! (t :loading))
           (when (or nfs-granted? db-restored?)
             (route-handler/update-page-title! route))))
      [nfs-granted? db-restoring? route])

diff --git a/src/main/frontend/components/whiteboard.cljs b/src/main/frontend/components/whiteboard.cljs
@@ -214,7 +214,7 @@
           has-checked? (not-empty checked-page-names)]
       [:<>
        [:h1.select-none.flex.items-center.whiteboard-dashboard-title.title
-        [:div "All whiteboards"
+        [:div (t :all-whiteboards)
          [:span.opacity-50
           (str " · " total-whiteboards)]]
         [:div.flex-1]