diff --git a/docs/agent-guide/026-logseq-cli-query-output.md b/docs/agent-guide/026-logseq-cli-query-output.md new file mode 100644 index 0000000000..3e6f3978bf --- /dev/null +++ b/docs/agent-guide/026-logseq-cli-query-output.md @@ -0,0 +1,74 @@ +# Logseq CLI Query Output Piping Implementation Plan + +Goal: Remove the space-to-comma transformation in CLI query human output while keeping query results usable in shell pipelines like xargs and direct stdin piping with logseq show. + +Architecture: Adjust human formatting for query results to preserve EDN output for general results and emit line-oriented output only for scalar id collections. +Architecture: Extend logseq show -id to read ids from stdin when no id argument is provided and update integration tests to validate both xargs and direct stdin pipelines. + +Tech Stack: ClojureScript, Logseq CLI, db-worker-node, Node-based integration tests. + +Related: Relates to docs/agent-guide/025-logseq-cli-builtin-status-priority-queries.md. + +## Testing Plan + +I will add an integration test that ensures a human-output query can be piped into xargs and then into logseq show for multiple ids. +I will add an integration test that ensures a human-output query can be piped directly into logseq show -id via stdin when no id value is passed. +I will update the existing integration test that currently pipes human query output directly into logseq show so it asserts the new line-oriented behavior and stdin ingestion. +I will add a focused unit test for format-query-results that covers scalar collections, non-scalar collections, and nil results. +I will add a unit test for show id parsing that covers missing id arg with stdin provided and missing id arg without stdin. + +NOTE: I will write all tests before I add any implementation behavior. + +## Problem statement + +The CLI currently replaces spaces with commas in format-query-results, which is a lossy transformation that makes output less readable. +Removing that transformation will reintroduce spaces in EDN vectors and lists, which breaks shell pipelines that rely on whitespace splitting. +We need to remove the space-to-comma logic while still ensuring the pipeline commands in the request work reliably. +The show command needs to accept ids from stdin when -id is present but no explicit id argument is provided. + +## Plan + +1. Read the existing formatting logic in /Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/format.cljs and document current behavior in a quick note. +2. Read the show command option parsing in /Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/show.cljs and /Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/id.cljs to understand current id validation. +3. Locate the current integration test that verifies human query output piping in /Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/integration_test.cljs and map the required changes. +4. Define the new output rule for format-query-results as a comment in the plan: if the result is a sequential collection of scalar ids, output one id per line, otherwise output the EDN string unchanged. +5. Define the new show -id stdin rule as a comment in the plan: when -id is present but no id value is provided, read stdin, trim it, and treat it as the id or id vector string for parsing. +6. Write the failing unit tests for format-query-results in a new test namespace under /Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/format_test.cljs that covers nil, scalar-id sequences, and nested maps. +7. Write failing unit tests for show id parsing in /Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/command/show_test.cljs that cover stdin provided, stdin blank, and explicit id values. +8. Run the unit tests to confirm the failure before implementation using bb dev:test with the new namespaces. +9. Write a failing integration test that uses the exact pipeline command from the request by invoking a shell command from the test harness in /Users/rcmerci/gh-repos/logseq/src/test/logseq/cli/integration_test.cljs for xargs usage. +10. Write a failing integration test that uses the exact pipeline command from the request for direct stdin piping into logseq show -id with no argument. +11. Run the integration tests to confirm the failure before implementation using bb dev:test with the specific test names. +12. Implement the new format-query-results behavior in /Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/format.cljs using a helper predicate for scalar ids and line-join logic. +13. Remove the string/replace space-to-comma logic from format-query-results in /Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/format.cljs. +14. Implement stdin ingestion for show -id in /Users/rcmerci/gh-repos/logseq/src/main/logseq/cli/command/show.cljs, using a helper to read from stdin only when -id is present and no id argument is provided. +15. Update the existing human-output pipeline integration test to align with the new output behavior and assert both xargs and direct stdin usage. +16. Re-run the unit tests and the integration tests to validate the new behavior. +17. Update deps/cli/README.md examples if they reference comma-transformed output, and add a short note describing the new xargs-friendly and stdin-friendly behavior for id lists. + +## Testing Details + +The unit tests will exercise behavior by calling format-query-results with representative results and asserting the exact emitted string for id lists and non-id data. +The integration tests will execute logseq query with a task-search or custom datalog query, pipe the output through xargs into logseq show, and pipe directly into logseq show -id via stdin with no id argument. +The show stdin behavior test will assert that missing -id input without stdin returns a clear error and that stdin is parsed the same as an id argument. + +## Implementation Details + +- Add a helper predicate for scalar ids that accepts integers and rejects maps, vectors, and strings. +- Detect sequential results that are entirely scalar ids and return a newline-joined string of ids. +- Preserve the existing safe-read-string validation to avoid changing behavior for invalid EDN strings. +- Keep non-scalar results as their original EDN string output. +- Maintain nil handling so that "nil" still prints as "nil". +- Ensure the output is stable for xargs by avoiding embedded spaces in the line-oriented mode. +- Add stdin reading to show -id when the option is present but its value is missing. +- Parse stdin through the same id parsing function used for explicit -id values. +- Keep existing errors when -id is missing and stdin is empty or whitespace. +- Update integration tests to use the exact pipeline command from the requirement. +- Use @test-driven-development for all implementation steps. + +## Question + +The line-oriented output applies only to vectors of numeric ids. +Logseq show -id reads stdin only when -id is present with no value. + +--- diff --git a/src/main/logseq/cli/command/id.cljs b/src/main/logseq/cli/command/id.cljs index 164ad37fec..1d5ed82c00 100644 --- a/src/main/logseq/cli/command/id.cljs +++ b/src/main/logseq/cli/command/id.cljs @@ -39,7 +39,7 @@ (every? valid-id? parsed) {:ok? true :value (vec parsed) :multi? true} :else (invalid "id vector must contain only integers"))) - (re-matches #"-?\\d+" text) + (re-matches #"-?\d+" text) {:ok? true :value [(js/parseInt text 10)] :multi? false} :else diff --git a/src/main/logseq/cli/command/show.cljs b/src/main/logseq/cli/command/show.cljs index 5ba7dc9d78..abed3d9227 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] + (:require ["fs" :as fs] + [clojure.string :as string] [clojure.walk :as walk] [logseq.cli.command.id :as id-command] [logseq.cli.command.core :as core] @@ -24,15 +25,47 @@ (def ^:private multi-id-delimiter "\n================================================================\n") +(defn read-stdin + [] + (.toString (fs/readFileSync 0) "utf8")) + +(defn- normalize-stdin-id + [value] + (let [text (string/trim (or value ""))] + (cond + (string/blank? text) text + (string/starts-with? text "[") text + (re-matches #"-?\d+" text) text + :else + (let [tokens (->> (string/split text #"\s+") + (remove string/blank?))] + (if (and (seq tokens) (every? #(re-matches #"-?\d+" %) tokens)) + (str "[" (string/join " " tokens) "]") + text))))) + +(defn- resolve-stdin-id + [options] + (if (:id-from-stdin? options) + (let [stdin (if (contains? options :stdin) + (:stdin options) + (read-stdin))] + (assoc options :id (normalize-stdin-id stdin))) + options)) + (defn invalid-options? [opts] (let [level (:level opts) - id-result (id-command/parse-id-option (:id opts))] + id-value (:id opts) + id-missing? (and (:id-from-stdin? opts) + (or (nil? id-value) + (and (string? id-value) (string/blank? id-value)))) + id-result (when-not id-missing? + (id-command/parse-id-option id-value))] (cond (and (some? level) (< level 1)) "level must be >= 1" - (and (some? (:id opts)) (not (:ok? id-result))) + (and (some? id-value) (not id-missing?) (not (:ok? id-result))) (:message id-result) :else @@ -469,7 +502,8 @@ {:ok? false :error {:code :missing-repo :message "repo is required for show"}} - (let [id-result (id-command/parse-id-option (:id options)) + (let [options (resolve-stdin-id options) + id-result (id-command/parse-id-option (:id options)) ids (:value id-result) multi-id? (:multi? id-result) targets (filter some? [(:id options) (:uuid options) (:page options)])] diff --git a/src/main/logseq/cli/commands.cljs b/src/main/logseq/cli/commands.cljs index 0bedcd411f..ac38c6063e 100644 --- a/src/main/logseq/cli/commands.cljs +++ b/src/main/logseq/cli/commands.cljs @@ -105,6 +105,28 @@ ;; Global option parsing lives in logseq.cli.command.core. +(defn- index-of + [coll value] + (first (keep-indexed (fn [idx item] + (when (= item value) idx)) + coll))) + +(defn- inject-stdin-id-arg + [args] + (if (and (seq args) (= "show" (first args))) + (if-let [idx (index-of args "--id")] + (let [next-token (nth args (inc idx) nil) + missing-value? (or (nil? next-token) + (string/starts-with? next-token "-"))] + (if missing-value? + {:args (vec (concat (subvec args 0 (inc idx)) + [""] + (subvec args (inc idx)))) + :id-from-stdin? true} + {:args args :id-from-stdin? false})) + {:args args :id-from-stdin? false}) + {:args args :id-from-stdin? false})) + (defn- unknown-command-message [{:keys [dispatch wrong-input]}] (string/join " " (cond-> (vec dispatch) @@ -223,44 +245,53 @@ [raw-args] (let [summary (command-core/top-level-summary table) legacy-graph-opt? (command-core/legacy-graph-opt? raw-args) - {:keys [opts args]} (command-core/parse-leading-global-opts raw-args)] - (if legacy-graph-opt? + {:keys [opts args]} (command-core/parse-leading-global-opts raw-args) + {:keys [args id-from-stdin?]} (inject-stdin-id-arg (vec args))] + (cond + legacy-graph-opt? (command-core/invalid-options-result summary "unknown option: --graph") - (if (:version opts) + + (:version opts) (command-core/ok-result :version opts [] summary) - (if (empty? args) + + (empty? args) (if (:help opts) (command-core/help-result summary) {:ok? false :error {:code :missing-command :message "missing command"} - :summary summary}) - (if (and (= 1 (count args)) (#{"graph" "server" "list" "add" "query"} (first args))) - (command-core/help-result (command-core/group-summary (first args) table)) - (try - (let [result (cli/dispatch table args {:spec global-spec})] - (if (nil? result) - (command-core/unknown-command-result summary (str "unknown command: " (string/join " " args))) - (finalize-command summary (update result :opts #(merge opts (or % {})))))) - (catch :default e - (let [{:keys [cause] :as data} (ex-data e)] - (cond - (= cause :input-exhausted) - (if (:help opts) - (command-core/help-result summary) - {:ok? false - :error {:code :missing-command - :message "missing command"} - :summary summary}) + :summary summary}) - (= cause :no-match) - (command-core/unknown-command-result summary (str "unknown command: " (unknown-command-message data))) + (and (= 1 (count args)) (#{"graph" "server" "list" "add" "query"} (first args))) + (command-core/help-result (command-core/group-summary (first args) table)) - (some? data) - (command-core/cli-error->result summary data) + :else + (try + (let [result (cli/dispatch table args {:spec global-spec})] + (if (nil? result) + (command-core/unknown-command-result summary (str "unknown command: " (string/join " " args))) + (finalize-command summary + (update result :opts #(cond-> (merge opts (or % {})) + id-from-stdin? (assoc :id-from-stdin? true)))))) + (catch :default e + (let [{:keys [cause] :as data} (ex-data e)] + (cond + (= cause :input-exhausted) + (if (:help opts) + (command-core/help-result summary) + {:ok? false + :error {:code :missing-command + :message "missing command"} + :summary summary}) - :else - (command-core/unknown-command-result summary (str "unknown command: " (string/join " " args))))))))))))) + (= cause :no-match) + (command-core/unknown-command-result summary (str "unknown command: " (unknown-command-message data))) + + (some? data) + (command-core/cli-error->result summary data) + + :else + (command-core/unknown-command-result summary (str "unknown command: " (string/join " " args)))))))))) ;; Repo/graph helpers live in logseq.cli.command.core. @@ -296,8 +327,6 @@ ;; Show helpers live in logseq.cli.command.show. - - ;; Show helpers live in logseq.cli.command.show. ;; Repo normalization lives in logseq.cli.command.core. diff --git a/src/main/logseq/cli/format.cljs b/src/main/logseq/cli/format.cljs index 65125bdf09..f4c8daf9ce 100644 --- a/src/main/logseq/cli/format.cljs +++ b/src/main/logseq/cli/format.cljs @@ -182,13 +182,25 @@ (:pid server)]) (or servers [])))) +(defn- scalar-id? + [value] + (and (number? value) (integer? value))) + +(defn- scalar-id-seq? + [value] + (and (sequential? value) + (seq value) + (every? scalar-id? value))) + (defn- format-query-results [result] (let [edn-str (pr-str result) parsed (common-util/safe-read-string {:log-error? false} edn-str) valid? (or (some? parsed) (= "nil" (string/trim edn-str)))] (if valid? - (string/replace edn-str " " ",") + (if (scalar-id-seq? result) + (string/join "\n" (map str result)) + edn-str) edn-str))) (defn- format-query-list diff --git a/src/test/logseq/cli/command/show_test.cljs b/src/test/logseq/cli/command/show_test.cljs new file mode 100644 index 0000000000..a36f749769 --- /dev/null +++ b/src/test/logseq/cli/command/show_test.cljs @@ -0,0 +1,33 @@ +(ns logseq.cli.command.show-test + (:require [cljs.test :refer [deftest is testing]] + [clojure.string :as string] + [logseq.cli.command.show :as show-command])) + +(deftest test-build-action-stdin-id + (testing "reads id from stdin when id flag is present without a value" + (let [result (show-command/build-action {:id "" + :id-from-stdin? true + :stdin "42"} + "logseq_db_demo")] + (is (true? (:ok? result))) + (is (= 42 (get-in result [:action :id]))) + (is (= [42] (get-in result [:action :ids]))) + (is (false? (get-in result [:action :multi-id?]))))) + + (testing "reads multi-id vector from stdin" + (let [result (show-command/build-action {:id "" + :id-from-stdin? true + :stdin "[1 2 3]"} + "logseq_db_demo")] + (is (true? (:ok? result))) + (is (= [1 2 3] (get-in result [:action :ids]))) + (is (true? (get-in result [:action :multi-id?]))))) + + (testing "blank stdin returns invalid options" + (let [result (show-command/build-action {:id "" + :id-from-stdin? true + :stdin " "} + "logseq_db_demo")] + (is (false? (:ok? result))) + (is (= :invalid-options (get-in result [:error :code]))) + (is (string/includes? (get-in result [:error :message]) "id"))))) diff --git a/src/test/logseq/cli/format_test.cljs b/src/test/logseq/cli/format_test.cljs index 4532743395..c733b050d7 100644 --- a/src/test/logseq/cli/format_test.cljs +++ b/src/test/logseq/cli/format_test.cljs @@ -101,7 +101,7 @@ (testing "remove page renders a succinct success line" (let [result (format/format-result {:status :ok - :command :remove-page + :command :remove :context {:repo "demo-repo" :page "Home"} :data {:result {:ok true}}} @@ -170,6 +170,38 @@ {:output-format nil})] (is (= "Line 1\nLine 2" result))))) +(deftest test-human-output-query-results + (testing "scalar id collections render one id per line" + (let [result (format/format-result {:status :ok + :command :query + :data {:result [1 2 3]}} + {:output-format nil})] + (is (= "1\n2\n3" result)))) + + (testing "non-scalar collections preserve EDN formatting" + (let [value [{:db/id 1 :block/title "Alpha"} + {:db/id 2 :block/title "Beta"}] + result (format/format-result {:status :ok + :command :query + :data {:result value}} + {:output-format nil})] + (is (= (pr-str value) result)))) + + (testing "mixed scalar collections preserve EDN formatting" + (let [value [1 "two" 3] + result (format/format-result {:status :ok + :command :query + :data {:result value}} + {:output-format nil})] + (is (= (pr-str value) result)))) + + (testing "nil results render as nil" + (let [result (format/format-result {:status :ok + :command :query + :data {:result nil}} + {:output-format nil})] + (is (= "nil" result))))) + (deftest test-human-output-show-styled-prefixes (testing "show preserves styled status and tags in human output" (let [tree->text #'show-command/tree->text @@ -231,7 +263,7 @@ :command :query :data {:result [[1] [2] [3]]}} {:output-format nil})] - (is (= "[[1],[2],[3]]" result))))) + (is (= "[[1] [2] [3]]" result))))) (deftest test-human-output-query-list (testing "query list renders a table with count" diff --git a/src/test/logseq/cli/integration_test.cljs b/src/test/logseq/cli/integration_test.cljs index 23131a9fad..a804234eb6 100644 --- a/src/test/logseq/cli/integration_test.cljs +++ b/src/test/logseq/cli/integration_test.cljs @@ -1,11 +1,13 @@ (ns logseq.cli.integration-test - (:require ["fs" :as fs] + (:require ["child_process" :as child-process] + ["fs" :as fs] ["path" :as node-path] [cljs.reader :as reader] [cljs.test :refer [deftest is async]] [clojure.string :as string] [frontend.worker-common.util :as worker-util] [frontend.test.node-helper :as node-helper] + [logseq.cli.command.show :as show-command] [logseq.cli.command.core :as command-core] [logseq.cli.main :as cli-main] [logseq.common.util :as common-util] @@ -44,6 +46,28 @@ [result] (reader/read-string (:output result))) +(defn- shell-escape + [value] + (let [text (str value)] + (str "'" (string/replace text #"'" "'\"'\"'") "'"))) + +(defn- run-shell + [command] + (try + (child-process/execSync command #js {:encoding "utf8" + :shell "/bin/bash"}) + (catch :default e + (let [err ^js e + stdout (some-> (.-stdout err) (.toString "utf8")) + stderr (some-> (.-stderr err) (.toString "utf8"))] + (throw (ex-info (str "shell command failed: " command + "\nstdout: " (or stdout "") + "\nstderr: " (or stderr "")) + {:command command + :stdout stdout + :stderr stderr} + e)))))) + (defn- node-title [node] (or (:block/title node) (:block/content node) (:title node) (:content node))) @@ -1156,30 +1180,91 @@ " :where" " [?e :block/title ?title]" " [(clojure.string/includes? ?title ?q)]]") - query-result (run-cli ["--repo" "query-pipe-graph" + node-bin (shell-escape (.-execPath js/process)) + cli-bin (shell-escape (node-path/resolve "static/logseq-cli.js")) + data-arg (shell-escape data-dir) + cfg-arg (shell-escape cfg-path) + repo-arg (shell-escape "query-pipe-graph") + query-arg (shell-escape query-text) + inputs-arg (shell-escape (pr-str ["Pipe"])) + query-cmd (string/join " " + [node-bin cli-bin + "--data-dir" data-arg + "--config" cfg-arg + "--repo" repo-arg + "--output" "human" + "query" + "--query" query-arg + "--inputs" inputs-arg]) + show-cmd (string/join " " + [node-bin cli-bin + "--data-dir" data-arg + "--config" cfg-arg + "--repo" repo-arg + "--output" "human" + "show" + "--id"]) + pipeline (str query-cmd " | xargs -I{} " show-cmd " {}") + output (run-shell pipeline) + stop-result (run-cli ["server" "stop" "--repo" "query-pipe-graph"] + data-dir cfg-path) + stop-payload (parse-json-output stop-result)] + (is (string/includes? output "Pipe One")) + (is (string/includes? output "Pipe Two")) + (is (= "ok" (:status stop-payload))) + (done)) + (p/catch (fn [e] + (is false (str "unexpected error: " e)) + (done))))))) + +(deftest test-cli-query-human-output-pipes-to-show-stdin + (async done + (let [data-dir (node-helper/create-tmp-dir "db-worker-query-stdin")] + (-> (p/let [cfg-path (node-path/join (node-helper/create-tmp-dir "cli") "cli.edn") + _ (fs/writeFileSync cfg-path "{:output-format :json}") + _ (run-cli ["graph" "create" "--repo" "query-stdin-graph"] data-dir cfg-path) + _ (run-cli ["--repo" "query-stdin-graph" "add" "page" "--page" "PipePage"] + data-dir cfg-path) + _ (run-cli ["--repo" "query-stdin-graph" "add" "block" + "--target-page-name" "PipePage" + "--content" "Pipe One"] + data-dir cfg-path) + _ (run-cli ["--repo" "query-stdin-graph" "add" "block" + "--target-page-name" "PipePage" + "--content" "Pipe Two"] + data-dir cfg-path) + _ (p/delay 100) + query-text (str "[:find [?e ...]" + " :in $ ?q" + " :where" + " [?e :block/title ?title]" + " [(clojure.string/includes? ?title ?q)]]") + query-result (run-cli ["--repo" "query-stdin-graph" "--output" "human" "query" "--query" query-text "--inputs" (pr-str ["Pipe"])] data-dir cfg-path) - ids-edn (string/trim (:output query-result)) - show-json-result (run-cli ["--repo" "query-pipe-graph" "show" - "--id" ids-edn - "--format" "json"] - data-dir cfg-path) - show-json-payload (parse-json-output show-json-result) - show-data (:data show-json-payload) + ids-text (string/trim (:output query-result)) + show-result (with-redefs [show-command/read-stdin (fn [] ids-text)] + (run-cli ["--repo" "query-stdin-graph" + "--output" "json" + "show" + "--id"] + data-dir cfg-path)) + show-payload (parse-json-output show-result) + show-data (:data show-payload) + root (some-> show-data first :root) root-titles (set (map (comp node-title :root) show-data)) - stop-result (run-cli ["server" "stop" "--repo" "query-pipe-graph"] + pipe-one (find-block-by-title root "Pipe One") + pipe-two (find-block-by-title root "Pipe Two") + stop-result (run-cli ["server" "stop" "--repo" "query-stdin-graph"] data-dir cfg-path) stop-payload (parse-json-output stop-result)] - (is (= 0 (:exit-code query-result))) - (is (seq ids-edn)) - (is (= 0 (:exit-code show-json-result))) - (is (= "ok" (:status show-json-payload))) - (is (vector? show-data)) - (is (contains? root-titles "Pipe One")) - (is (contains? root-titles "Pipe Two")) + (is (= "ok" (:status show-payload))) + (is (contains? root-titles "PipePage")) + (is (some? pipe-one)) + (is (some? pipe-two)) (is (= "ok" (:status stop-payload))) (done)) (p/catch (fn [e]