diff --git a/docs/agent-guide/022-logseq-cli-help-show-output.md b/docs/agent-guide/022-logseq-cli-help-show-output.md new file mode 100644 index 0000000000..00d72911bb --- /dev/null +++ b/docs/agent-guide/022-logseq-cli-help-show-output.md @@ -0,0 +1,122 @@ +# Logseq CLI Help and Show Output Cleanup Implementation Plan + +Goal: Improve logseq-cli help readability and ensure show JSON and EDN outputs use :db/id without :block/uuid. + +Architecture: The CLI help text is assembled in logseq.cli.command.core and babashka.cli spec descriptions, while show output is built in logseq.cli.command.show and formatted in logseq.cli.format before returning JSON or EDN. + +Tech Stack: ClojureScript, logseq-cli, babashka.cli, db-worker-node. + +Related: Relates to 018-logseq-cli-add-tags-builtin-properties.md. + +## Problem statement + +The current help output lists [options] for nearly every command, which clutters the help display and reduces readability. + +The tags and properties option help text does not clearly state that identifiers can be id, :db/ident, or :block/title across all relevant help contexts. + +The show command includes :block/uuid in JSON and EDN output, even though :db/id is already present and preferred for programmatic consumers. + +``` +logseq-cli + | help text built from babashka.cli specs + v +logseq.cli.command.core + | parse args and build command payloads + v +logseq.cli.command.show + | fetch tree data from db-worker-node + v +logseq.cli.format + | render human, json, edn output + v +stdout +``` + +## Testing Plan + +I will add or update unit tests that assert help summaries do not include repeated [options] in the command list, and that tags and properties descriptions include the identifier guidance. + +I will add or update unit tests that verify show JSON and EDN outputs do not include :block/uuid and still include :db/id for root and child nodes. + +I will add or update integration tests for show JSON output to assert :db/id is present and :block/uuid is absent in root, tags, and linked references where applicable. + +NOTE: I will write all tests before I add any implementation behavior. + +## Requirements + +The top level help output and group summaries must avoid repeating [options] for each command listing while still documenting that options exist in the usage line. + +The help description for --tags and --properties must explicitly state that identifiers can be id, :db/ident, or :block/title. + +The show command must omit :block/uuid from JSON and EDN outputs while preserving :db/id for the same entities. + +The show command human output must be unchanged. + +## Non-goals + +Do not change CLI command behavior or supported flags beyond help text updates. + +Do not change db-worker-node behavior or its API surface. + +Do not change the structure of human output for show, list, add, or query commands. + +## Design decisions + +Limit the help output adjustment to formatting in logseq.cli.command.core so command behavior and parsing remain unchanged. + +Apply the identifier clarification to all --tags and --properties options in logseq.cli.command.add and any other specs that expose those flags. + +Strip :block/uuid only for show outputs in JSON and EDN formats by post-processing tree data just before returning payloads. + +## Implementation plan + +1. Follow @test-driven-development for every change in this plan. + +2. Add a unit test in src/test/logseq/cli/commands_test.cljs that asserts command list rows in top level and group help do not contain [options] after the command name. + +3. Add a unit test in src/test/logseq/cli/commands_test.cljs that asserts the --tags and --properties option descriptions include the text supporting id, :db/ident, and :block/title identifiers. + +4. Add a unit test in src/test/logseq/cli/format_test.cljs or src/test/logseq/cli/commands_test.cljs that asserts show JSON and EDN outputs strip :block/uuid while retaining :db/id in root and child nodes. + +5. Update integration tests in src/test/logseq/cli/integration_test.cljs that currently assert :uuid or :block/uuid in show JSON output to instead assert :db/id and absence of :block/uuid. + +6. Adjust logseq.cli.command.core command listing formatting so only the usage line includes [options], and the command listing uses the bare command path without the suffix. + +7. Update the --tags and --properties option descriptions in src/main/logseq/cli/command/add.cljs to include the identifier guidance sentence in a consistent phrasing. + +8. Add a helper in src/main/logseq/cli/command/show.cljs or src/main/logseq/cli/format.cljs that removes :block/uuid keys from show JSON and EDN payloads, and apply it in execute-show when output-format is :json or :edn. + +9. Run the updated unit tests and integration tests from the Testing Plan and confirm all pass. + +## Edge cases + +Command help should still show [options] in the usage line for commands that accept options, but not in the command list table. + +Multi-id show results should strip :block/uuid from each tree entry without changing the error map shape. + +Linked references and tag entities should keep :db/id in the output even when :block/uuid is removed. + +## Testing Details + +I will add tests that verify help summaries and option descriptions at the command summary level and not by matching the raw babashka.cli output. + +I will add tests that parse show JSON and EDN output and assert :block/uuid is missing while :db/id remains on block nodes. + +I will update integration tests that read show JSON output to match the new key expectations without changing the test setup logic. + +## Implementation Details + +- Update logseq.cli.command.core formatting to render command rows without [options]. +- Keep usage lines intact so users still see options availability in usage sections. +- Align help text for --tags and --properties to a single wording that mentions id, :db/ident, and :block/title. +- Add a show-specific sanitization step for json and edn output only. +- Keep the show tree data used for human output unchanged to avoid regressions. +- Ensure strip logic is recursive so :block/uuid is removed from nested children and linked references. +- Prefer clojure.walk/postwalk for key removal to minimize custom traversal code. +- Document the new behavior in tests rather than adding new user-facing docs. + +## Question + +This is resolved. Only add supports --tags and --properties today, so we will update help text there only. + +--- diff --git a/src/main/logseq/cli/command/add.cljs b/src/main/logseq/cli/command/add.cljs index cce1a9eab5..63985c5879 100644 --- a/src/main/logseq/cli/command/add.cljs +++ b/src/main/logseq/cli/command/add.cljs @@ -20,8 +20,8 @@ {:content {:desc "Block content for add"} :blocks {:desc "EDN vector of blocks for add"} :blocks-file {:desc "EDN file of blocks for add"} - :tags {:desc "EDN vector of tags (id, :db/ident, or :block/title)"} - :properties {:desc "EDN map of built-in properties (id, :db/ident, or :block/title)"} + :tags {:desc "EDN vector of tags. Identifiers can be id, :db/ident, or :block/title."} + :properties {:desc "EDN map of built-in properties. Identifiers can be id, :db/ident, or :block/title."} :target-id {:desc "Target block db/id" :coerce :long} :target-uuid {:desc "Target block UUID"} @@ -31,8 +31,8 @@ (def ^:private add-page-spec {:page {:desc "Page name"} - :tags {:desc "EDN vector of tags (id, :db/ident, or :block/title)"} - :properties {:desc "EDN map of built-in properties (id, :db/ident, or :block/title)"}}) + :tags {:desc "EDN vector of tags. Identifiers can be id, :db/ident, or :block/title."} + :properties {:desc "EDN map of built-in properties. Identifiers can be id, :db/ident, or :block/title."}}) (def entries [(core/command-entry ["add" "block"] :add-block "Add blocks" content-add-spec) diff --git a/src/main/logseq/cli/command/core.cljs b/src/main/logseq/cli/command/core.cljs index f718de38a0..5889881ada 100644 --- a/src/main/logseq/cli/command/core.cljs +++ b/src/main/logseq/cli/command/core.cljs @@ -47,12 +47,16 @@ (cond-> base has-options? (str " [options]")))) +(defn- command-label + [cmds] + (string/join " " cmds)) + (defn- format-commands [table] (let [rows (->> table (filter (comp seq :cmds)) - (map (fn [{:keys [cmds desc spec]}] - (let [command (command-usage cmds spec)] + (map (fn [{:keys [cmds desc]}] + (let [command (command-label cmds)] {:command command :desc desc})))) width (apply max 0 (map (comp count :command) rows))] diff --git a/src/main/logseq/cli/command/show.cljs b/src/main/logseq/cli/command/show.cljs index a5927d1f6f..7959bafff3 100644 --- a/src/main/logseq/cli/command/show.cljs +++ b/src/main/logseq/cli/command/show.cljs @@ -1,6 +1,7 @@ (ns logseq.cli.command.show "Show-related CLI commands." (:require [clojure.string :as string] + [clojure.walk :as walk] [logseq.cli.command.id :as id-command] [logseq.cli.command.core :as core] [logseq.cli.server :as cli-server] @@ -508,6 +509,15 @@ (walk root #{}) #{}))) +(defn- strip-block-uuid + [tree-data] + (walk/postwalk + (fn [entry] + (if (map? entry) + (dissoc entry :block/uuid) + entry)) + tree-data)) + (defn execute-show [action config] (-> (p/let [cfg (cli-server/ensure-server! config (:repo action)) @@ -539,12 +549,14 @@ results (vec (remove (fn [{:keys [ok? id]}] (and ok? (contained? id))) results)) + sanitize-tree (fn [tree] + (strip-block-uuid tree)) payload (case format :edn {:status :ok :data (mapv (fn [{:keys [ok? tree id error]}] (if ok? - tree + (sanitize-tree tree) (multi-id-error-entry id error))) results) :output-format :edn} @@ -553,7 +565,7 @@ {:status :ok :data (mapv (fn [{:keys [ok? tree id error]}] (if ok? - tree + (sanitize-tree tree) (multi-id-error-entry id error))) results) :output-format :json} @@ -568,15 +580,17 @@ payload) (p/let [tree-data (build-tree-data cfg action)] (case format - :edn - {:status :ok - :data tree-data - :output-format :edn} + :edn + (let [tree-data (strip-block-uuid tree-data)] + {:status :ok + :data tree-data + :output-format :edn}) - :json - {:status :ok - :data tree-data - :output-format :json} + :json + (let [tree-data (strip-block-uuid tree-data)] + {:status :ok + :data tree-data + :output-format :json}) {:status :ok :data {:message (tree->text-with-linked-refs tree-data)}})))))) diff --git a/src/test/logseq/cli/commands_test.cljs b/src/test/logseq/cli/commands_test.cljs index 2adad4054b..66cfc12bae 100644 --- a/src/test/logseq/cli/commands_test.cljs +++ b/src/test/logseq/cli/commands_test.cljs @@ -8,6 +8,25 @@ [logseq.cli.transport :as transport] [promesa.core :as p])) +(defn- command-lines + [summary] + (let [lines (string/split-lines summary) + section (if (some #{"Commands:"} lines) "Commands:" "Subcommands:") + start (inc (.indexOf lines section)) + end (.indexOf lines "Global options:") + entries (subvec (vec lines) start end)] + (->> entries + (filter #(string/starts-with? % " ")) + (remove string/blank?)))) + +(defn- contains-block-uuid? + [value] + (cond + (map? value) (or (contains? value :block/uuid) + (some contains-block-uuid? (vals value))) + (sequential? value) (some contains-block-uuid? value) + :else false)) + (deftest test-help-output (testing "top-level help lists command groups" (let [result (commands/parse-args ["--help"]) @@ -26,6 +45,12 @@ (is (string/includes? summary "graph")) (is (string/includes? summary "server")))) + (testing "top-level help command list omits [options]" + (let [summary (:summary (commands/parse-args ["--help"])) + lines (command-lines summary)] + (is (seq lines)) + (is (every? #(not (string/includes? % "[options]")) lines)))) + (testing "top-level help separates global and command options" (let [summary (:summary (commands/parse-args ["--help"]))] (is (string/includes? summary "Global options:")) @@ -84,7 +109,13 @@ summary (:summary result)] (is (true? (:help? result))) (is (string/includes? summary "query list")) - (is (string/includes? summary "query [options]"))))) + (is (string/includes? summary "query")))) + + (testing "group help command list omits [options]" + (let [summary (:summary (commands/parse-args ["list"])) + lines (command-lines summary)] + (is (seq lines)) + (is (every? #(not (string/includes? % "[options]")) lines))))) (deftest test-parse-args-help-alignment (testing "graph group aligns subcommand columns" @@ -302,6 +333,36 @@ (is (= (str "1 See [[Target [[Inner]]]]") (tree->text tree-data)))))) +(deftest test-help-tags-properties-identifiers + (testing "add help mentions tag and property identifiers" + (let [summary (:summary (commands/parse-args ["add" "block" "--help"]))] + (is (string/includes? summary "Identifiers can be id, :db/ident, or :block/title."))) + (let [summary (:summary (commands/parse-args ["add" "page" "--help"]))] + (is (string/includes? summary "Identifiers can be id, :db/ident, or :block/title."))))) + +(deftest test-show-json-edn-strips-block-uuid + (testing "show json/edn removes :block/uuid recursively while keeping :db/id" + (let [tree-data {:root {:db/id 1 + :block/uuid "root-uuid" + :block/title "Root" + :block/children [{:db/id 2 + :block/uuid "child-uuid" + :block/title "Child" + :block/tags [{:db/id 3 + :block/uuid "tag-uuid" + :block/title "Tag"}]}]} + :linked-references {:count 1 + :blocks [{:db/id 4 + :block/uuid "ref-uuid" + :block/page {:db/id 5 + :block/uuid "page-uuid" + :block/title "Page"}}]} + :uuid->label {"root-uuid" "Root"}} + stripped (#'show-command/strip-block-uuid tree-data)] + (is (not (contains-block-uuid? stripped))) + (is (= 1 (get-in stripped [:root :db/id]))) + (is (= 2 (get-in stripped [:root :block/children 0 :db/id])))))) + (deftest test-tree->text-uuid-ref-recursion-limit (testing "show tree text limits uuid ref replacement depth" (let [tree->text #'show-command/tree->text diff --git a/src/test/logseq/cli/integration_test.cljs b/src/test/logseq/cli/integration_test.cljs index bd97773b70..2993f6d998 100644 --- a/src/test/logseq/cli/integration_test.cljs +++ b/src/test/logseq/cli/integration_test.cljs @@ -48,10 +48,6 @@ [node] (or (:block/title node) (:block/content node) (:title node) (:content node))) -(defn- node-uuid - [node] - (or (:block/uuid node) (:uuid node))) - (defn- node-children [node] (or (:block/children node) (:children node))) @@ -108,6 +104,20 @@ (pr-str [title]))] (first (first (get-in payload [:data :result]))))) +(defn- query-block-id + [data-dir cfg-path repo title] + (p/let [payload (run-query data-dir cfg-path repo + "[:find ?id . :in $ ?title :where [?b :block/title ?title] [?b :db/id ?id]]" + (pr-str [title]))] + (get-in payload [:data :result]))) + +(defn- query-block-uuid-by-id + [data-dir cfg-path repo id] + (p/let [payload (run-query data-dir cfg-path repo + "[:find ?uuid . :in $ ?id :where [?b :db/id ?id] [?b :block/uuid ?uuid]]" + (pr-str [id]))] + (get-in payload [:data :result]))) + (defn- list-items [data-dir cfg-path repo list-type] (p/let [result (run-cli ["--repo" repo "list" list-type] data-dir cfg-path)] @@ -228,7 +238,8 @@ (is (= "ok" (:status list-property-payload))) (is (vector? (get-in list-property-payload [:data :items]))) (is (= "ok" (:status show-payload))) - (is (contains? (get-in show-payload [:data :root]) :uuid)) + (is (contains? (get-in show-payload [:data :root]) :db/id)) + (is (not (contains? (get-in show-payload [:data :root]) :block/uuid))) (is (= "ok" (:status remove-page-payload))) (is (= "ok" (:status stop-payload))) (done)) @@ -716,16 +727,13 @@ _ (run-cli ["graph" "create" "--repo" "nested-refs"] data-dir cfg-path) _ (run-cli ["--repo" "nested-refs" "add" "page" "--page" "NestedPage"] data-dir cfg-path) _ (run-cli ["--repo" "nested-refs" "add" "block" "--target-page-name" "NestedPage" "--content" "Inner"] data-dir cfg-path) - show-inner (run-cli ["--repo" "nested-refs" "show" "--page-name" "NestedPage" "--format" "json"] data-dir cfg-path) - show-inner-payload (parse-json-output show-inner) - inner-node (find-block-by-title (get-in show-inner-payload [:data :root]) "Inner") - inner-uuid (node-uuid inner-node) + inner-id (query-block-id data-dir cfg-path "nested-refs" "Inner") + inner-uuid (query-block-uuid-by-id data-dir cfg-path "nested-refs" inner-id) + middle-content (str "See [[" inner-uuid "]]") _ (run-cli ["--repo" "nested-refs" "add" "block" "--target-page-name" "NestedPage" - "--content" (str "See [[" inner-uuid "]]")] data-dir cfg-path) - show-middle (run-cli ["--repo" "nested-refs" "show" "--page-name" "NestedPage" "--format" "json"] data-dir cfg-path) - show-middle-payload (parse-json-output show-middle) - middle-node (find-block-by-title (get-in show-middle-payload [:data :root]) "See [[Inner]]") - middle-uuid (node-uuid middle-node) + "--content" middle-content] data-dir cfg-path) + middle-id (query-block-id data-dir cfg-path "nested-refs" middle-content) + middle-uuid (query-block-uuid-by-id data-dir cfg-path "nested-refs" middle-id) _ (run-cli ["--repo" "nested-refs" "add" "block" "--target-page-name" "NestedPage" "--content" (str "Outer [[" middle-uuid "]]")] data-dir cfg-path) show-outer (run-cli ["--repo" "nested-refs" "show" "--page-name" "NestedPage" "--format" "json"] data-dir cfg-path) @@ -751,27 +759,21 @@ _ (run-cli ["graph" "create" "--repo" "linked-refs-graph"] data-dir cfg-path) _ (run-cli ["--repo" "linked-refs-graph" "add" "page" "--page" "TargetPage"] data-dir cfg-path) _ (run-cli ["--repo" "linked-refs-graph" "add" "page" "--page" "SourcePage"] data-dir cfg-path) - target-show-before (run-cli ["--repo" "linked-refs-graph" "show" "--page-name" "TargetPage" "--format" "json"] data-dir cfg-path) - target-before-payload (parse-json-output target-show-before) - target-uuid (or (get-in target-before-payload [:data :root :block/uuid]) - (get-in target-before-payload [:data :root :uuid])) - target-title (or (get-in target-before-payload [:data :root :block/title]) - (get-in target-before-payload [:data :root :block/name]) - "TargetPage") + target-id (query-block-id data-dir cfg-path "linked-refs-graph" "TargetPage") + target-uuid (query-block-uuid-by-id data-dir cfg-path "linked-refs-graph" target-id) + target-title "TargetPage" ref-content (str "See [[" target-uuid "]]") ref-title (str "See [[" target-title "]]") _ (run-cli ["--repo" "linked-refs-graph" "add" "block" "--target-page-name" "SourcePage" "--content" ref-content] data-dir cfg-path) source-show (run-cli ["--repo" "linked-refs-graph" "show" "--page-name" "SourcePage" "--format" "json"] data-dir cfg-path) source-payload (parse-json-output source-show) ref-node (find-block-by-title (get-in source-payload [:data :root]) ref-title) - ref-uuid (or (:block/uuid ref-node) (:uuid ref-node)) + ref-id (:db/id ref-node) target-show (run-cli ["--repo" "linked-refs-graph" "show" "--page-name" "TargetPage" "--format" "json"] data-dir cfg-path) target-payload (parse-json-output target-show) linked-refs (get-in target-payload [:data :linked-references]) linked-blocks (:blocks linked-refs) - linked-uuids (set (map (fn [block] - (or (:block/uuid block) (:uuid block))) - linked-blocks)) + linked-ids (set (map :db/id linked-blocks)) linked-page-titles (set (keep (fn [block] (or (get-in block [:block/page :block/title]) (get-in block [:block/page :block/name]) @@ -782,8 +784,8 @@ stop-payload (parse-json-output stop-result)] (is (some? target-uuid)) (is (= "ok" (:status target-payload))) - (is (some? ref-uuid)) - (is (contains? linked-uuids ref-uuid)) + (is (some? ref-id)) + (is (contains? linked-ids ref-id)) (is (contains? linked-page-titles "SourcePage")) (is (= "ok" (:status stop-payload))) (done)) @@ -800,10 +802,8 @@ _ (run-cli ["--repo" "move-graph" "add" "page" "--page" "SourcePage"] data-dir cfg-path) _ (run-cli ["--repo" "move-graph" "add" "page" "--page" "TargetPage"] data-dir cfg-path) _ (run-cli ["--repo" "move-graph" "add" "block" "--target-page-name" "SourcePage" "--content" "Parent Block"] data-dir cfg-path) - source-show (run-cli ["--repo" "move-graph" "show" "--page-name" "SourcePage" "--format" "json"] data-dir cfg-path) - source-payload (parse-json-output source-show) - parent-node (find-block-by-title (get-in source-payload [:data :root]) "Parent Block") - parent-uuid (or (:block/uuid parent-node) (:uuid parent-node)) + parent-id (query-block-id data-dir cfg-path "move-graph" "Parent Block") + parent-uuid (query-block-uuid-by-id data-dir cfg-path "move-graph" parent-id) _ (run-cli ["--repo" "move-graph" "add" "block" "--target-uuid" (str parent-uuid) "--content" "Child Block"] data-dir cfg-path) move-result (run-cli ["--repo" "move-graph" "move" "--uuid" (str parent-uuid) "--target-page-name" "TargetPage"] data-dir cfg-path) move-payload (parse-json-output move-result) @@ -831,10 +831,8 @@ _ (run-cli ["graph" "create" "--repo" "add-pos-graph"] data-dir cfg-path) _ (run-cli ["--repo" "add-pos-graph" "add" "page" "--page" "PosPage"] data-dir cfg-path) _ (run-cli ["--repo" "add-pos-graph" "add" "block" "--target-page-name" "PosPage" "--content" "Parent"] data-dir cfg-path) - parent-show (run-cli ["--repo" "add-pos-graph" "show" "--page-name" "PosPage" "--format" "json"] data-dir cfg-path) - parent-payload (parse-json-output parent-show) - parent-node (find-block-by-title (get-in parent-payload [:data :root]) "Parent") - parent-uuid (or (:block/uuid parent-node) (:uuid parent-node)) + parent-id (query-block-id data-dir cfg-path "add-pos-graph" "Parent") + parent-uuid (query-block-uuid-by-id data-dir cfg-path "add-pos-graph" parent-id) _ (run-cli ["--repo" "add-pos-graph" "add" "block" "--target-uuid" (str parent-uuid) "--pos" "first-child" "--content" "First"] data-dir cfg-path) _ (run-cli ["--repo" "add-pos-graph" "add" "block" "--target-uuid" (str parent-uuid) "--pos" "last-child" "--content" "Last"] data-dir cfg-path) final-show (run-cli ["--repo" "add-pos-graph" "show" "--page-name" "PosPage" "--format" "json"] data-dir cfg-path) @@ -941,11 +939,11 @@ (is (some? page-id)) (is (some? page-uuid)) (is (= "ok" (:status show-by-id-payload))) - (is (= (str page-uuid) (str (or (get-in show-by-id-payload [:data :root :uuid]) - (get-in show-by-id-payload [:data :root :block/uuid]))))) + (is (= page-id (get-in show-by-id-payload [:data :root :db/id]))) + (is (not (contains? (get-in show-by-id-payload [:data :root]) :block/uuid))) (is (= "ok" (:status show-by-uuid-payload))) - (is (= (str page-uuid) (str (or (get-in show-by-uuid-payload [:data :root :uuid]) - (get-in show-by-uuid-payload [:data :root :block/uuid]))))) + (is (= page-id (get-in show-by-uuid-payload [:data :root :db/id]))) + (is (not (contains? (get-in show-by-uuid-payload [:data :root]) :block/uuid))) (is (= "ok" (:status stop-payload))) (done)) (p/catch (fn [e] @@ -1043,14 +1041,7 @@ data-dir cfg-path) parent-payload (parse-json-output parent-query) parent-id (get-in parent-payload [:data :result]) - show-parent (run-cli ["--repo" "show-multi-id-contained-graph" - "show" - "--page-name" "ParentPage" - "--format" "json"] - data-dir cfg-path) - show-parent-payload (parse-json-output show-parent) - parent-node (find-block-by-title (get-in show-parent-payload [:data :root]) "Parent Block") - parent-uuid (node-uuid parent-node) + parent-uuid (query-block-uuid-by-id data-dir cfg-path "show-multi-id-contained-graph" parent-id) _ (run-cli ["--repo" "show-multi-id-contained-graph" "add" "block" "--target-uuid" (str parent-uuid) "--content" "Child Block"] @@ -1174,7 +1165,7 @@ (is (pos? (:count linked))) (is (seq (:blocks linked))) (is (some? ref-block)) - (is (some? (or (:block/uuid ref-block) (:uuid ref-block)))) + (is (some? (:db/id ref-block))) (is (some? (or (get-in ref-block [:page :title]) (get-in ref-block [:page :name])))) (is (= "ok" (:status stop-payload)))