Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Inspector] Configure truncation limits #694

Merged
merged 2 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ defaults: &defaults
working_directory: ~/repo
environment:
LEIN_ROOT: "true" # we intended to run lein as root
JVM_OPTS: -Xmx3200m # limit the maximum heap size to prevent out of memory errors
# JVM_OPTS:
# - limit the maximum heap size to prevent out of memory errors
# - print stacktraces to console
JVM_OPTS: >
-Xmx3200m
-Dclojure.main.report=stderr

# Runners for OpenJDK 8 and 11

Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## master (unreleased)

### New features

* [#694](https://github.com/clojure-emacs/cider-nrepl/pull/694): [Inspector] Configure truncation limits

## 0.25.11 (2021-04-12)

### Bugs Fixed
Expand Down
4 changes: 2 additions & 2 deletions project.clj
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
(defproject cider/cider-nrepl "0.25.11"
(defproject cider/cider-nrepl "0.26.0-SNAPSHOT"
:description "A collection of nREPL middlewares designed to enhance Clojure editors."
:url "https://github.com/clojure-emacs/cider-nrepl"
:license {:name "Eclipse Public License"
:url "http://www.eclipse.org/legal/epl-v10.html"}
:scm {:name "git" :url "https://github.com/clojure-emacs/cider-nrepl"}
:dependencies [[nrepl "0.8.3"]
^:inline-dep [cider/orchard "0.6.5"]
^:inline-dep [cider/orchard "0.7.0"]
^:inline-dep [thunknyc/profile "0.5.2"]
^:inline-dep [mvxcvi/puget "1.3.1"]
^:inline-dep [fipp "0.6.23"] ; can be removed in unresolved-tree mode
Expand Down
10 changes: 10 additions & 0 deletions src/cider/nrepl.clj
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,16 @@
:requires {"page-size" "New page size."
"session" "The current session"}
:returns {"status" "\"done\""}}
"inspect-set-max-atom-length"
{:doc "Set the max length of nested atoms to specified value."
:requires {"max-atom-length" "New max length."
"session" "The current session"}
:returns {"status" "\"done\""}}
"inspect-set-max-coll-size"
{:doc "Set the number of nested collection members to display before truncating."
:requires {"max-coll-size" "New collection size."
"session" "The current session"}
:returns {"status" "\"done\""}}
"inspect-clear"
{:doc "Clears the state state of the inspector."
:requires {"session" "The current session"}
Expand Down
17 changes: 15 additions & 2 deletions src/cider/nrepl/middleware/inspect.clj
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@
(response-for msg resp {:value value}))))

(defn inspect-reply
[{:keys [page-size transport] :as msg} eval-response]
[{:keys [page-size transport max-atom-length max-coll-size] :as msg} eval-response]
(let [value (cljs/response-value msg eval-response)
page-size (or page-size 32)
inspector (swap-inspector! msg #(-> (assoc % :page-size page-size)
inspector (swap-inspector! msg #(-> %
(assoc :page-size page-size
:max-atom-length max-atom-length
:max-coll-size max-coll-size)
(inspect/start value)))]
(transport/send transport (inspector-response msg inspector {}))))

Expand Down Expand Up @@ -84,6 +87,14 @@
(defn set-page-size-reply [msg]
(inspector-response msg (swap-inspector! msg inspect/set-page-size (:page-size msg))))

(defn set-max-atom-length-reply [msg]
(inspector-response msg (swap-inspector! msg inspect/set-max-atom-length
(:max-atom-length msg))))

(defn set-max-coll-size-reply [msg]
(inspector-response msg (swap-inspector! msg inspect/set-max-coll-size
(:max-coll-size msg))))

(defn clear-reply [msg]
(inspector-response msg (swap-inspector! msg (constantly (inspect/fresh)))))

Expand All @@ -102,5 +113,7 @@
"inspect-next-page" next-page-reply
"inspect-prev-page" prev-page-reply
"inspect-set-page-size" set-page-size-reply
"inspect-set-max-atom-length" set-max-atom-length-reply
"inspect-set-max-coll-size" set-max-coll-size-reply
"inspect-clear" clear-reply
"inspect-def-current-value" def-current-value)))
120 changes: 120 additions & 0 deletions test/clj/cider/nrepl/middleware/inspect_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
(:require
[cider.nrepl.middleware.inspect :as i]
[cider.nrepl.test-session :as session]
[clojure.string :as string]
[clojure.test :refer :all]))

(def nil-result ["(\"nil\" (:newline))"])
Expand Down Expand Up @@ -129,6 +130,22 @@
(is (= (:status response) #{"inspect-set-page-size-error" "done"}))
(is (= (:ex response) "class java.lang.Exception"))
(is (.startsWith (:err response) "java.lang.Exception: page-size exception"))
(is (:pp-stacktrace response)))))

(testing "inspect-set-max-atom-length error handling"
(with-redefs [i/swap-inspector! (fn [& _] (throw (Exception. "max-atom-length exception")))]
(let [response (session/message {:op "inspect-set-max-atom-length" :max-atom-length 10})]
(is (= (:status response) #{"inspect-set-max-atom-length-error" "done"}))
(is (= (:ex response) "class java.lang.Exception"))
(is (.startsWith (:err response) "java.lang.Exception: max-atom-length exception"))
(is (:pp-stacktrace response)))))

(testing "inspect-set-max-coll-size error handling"
(with-redefs [i/swap-inspector! (fn [& _] (throw (Exception. "max-coll-size exception")))]
(let [response (session/message {:op "inspect-set-max-coll-size" :max-coll-size 10})]
(is (= (:status response) #{"inspect-set-max-coll-size-error" "done"}))
(is (= (:ex response) "class java.lang.Exception"))
(is (.startsWith (:err response) "java.lang.Exception: max-coll-size exception"))
(is (:pp-stacktrace response))))))

(deftest inspect-var-integration-test
Expand Down Expand Up @@ -273,6 +290,109 @@
(is (re-find #"Page size: 5, showing page: 2 of 20"
(extract-text small-page-2))))))

(deftest max-atom-length-integration-test
;; Default max length is 150, so 150 - 3 = 147 chars is too long because we
;; need to leave room for an opening quote and trailing ellipsis "xxxxxxxx...
;; NOTE: We'd prefer to use `orchard.inspect/*max-atom-length*` vs.
;; hard-coding `max-len` to 150 in case the default ever changes, but test
;; code isn't processed by mranderson and so can't refer to inlined deps
;; including orchard.
;; See also https://github.com/benedekfazekas/mranderson/issues/5
(let [max-len 150 #_(var-get #'orchard.inspect/*max-atom-length*)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other new change is an attempt to avoid hard-coding orchard's defaults here, but I couldn't get mranderson to munge the test code requires. I gave up after a few stabs at editing project.clj with-profile +test/:source-paths/:test-paths entries, and reverted to hard-coding. I think benedekfazekas/mranderson#5 is related.

_ (assert (> max-len 3) "inspect/*max-atom-length* is too short for this test.")
too-long (pr-str [(string/join (repeat (- max-len 3) "x"))]) ;; 147
trunc-len (- max-len 4) ;; 146
x-pattern #(re-pattern (format "\"\\\\\"%s...\"" (string/join (repeat % "x"))))
extract-text #(-> % :value first)]

(testing "max atom length can be set for the eval op"
(let [default-atom-length (session/message {:op "eval"
:inspect "true"
:code too-long})
short-atom-length (session/message {:op "eval"
:inspect "true"
:code too-long
:max-atom-length 10})
unchanged-default-atom-length (session/message {:op "eval"
:inspect "true"
:code too-long})]
(is (re-find (x-pattern trunc-len)
(extract-text default-atom-length)))
(is (re-find (x-pattern 6)
(extract-text short-atom-length)))
(is (re-find (x-pattern trunc-len)
(extract-text unchanged-default-atom-length)))))

(testing "max atom length can be changed without re-eval'ing last form"
(let [default-atom-length (session/message {:op "eval"
:inspect "true"
:code too-long})
shorten-atom-length (session/message {:op "inspect-set-max-atom-length"
:max-atom-length 10})
longer-atom-length (session/message {:op "inspect-set-max-atom-length"
:max-atom-length 20})
unchanged-default-atom-length (session/message {:op "eval"
:inspect "true"
:code too-long})]
(is (re-find (x-pattern trunc-len)
(extract-text default-atom-length)))
(is (re-find (x-pattern 6)
(extract-text shorten-atom-length)))
(is (re-find (x-pattern 16)
(extract-text longer-atom-length)))
(is (re-find (x-pattern trunc-len)
(extract-text unchanged-default-atom-length)))))))

(deftest max-coll-size-integration-test
;; See NOTE in `max-atom-length-integration-test` about hard-coding
;; `size-limit` here.
(let [size-limit 5 #_(var-get #'orchard.inspect/*max-coll-size*)
big-size (* 2 size-limit) ;; 10
big-coll (format "[(range %d)]" big-size)
coll-pattern (fn [len & [truncate]]
(re-pattern (format "\\( %s%s \\)"
(string/join " " (range len))
(if truncate " ..." ""))))
extract-text #(-> % :value first)]

(testing "max coll size can be set for the eval op"
(let [default-coll-size (session/message {:op "eval"
:inspect "true"
:code big-coll})
large-coll-size (session/message {:op "eval"
:inspect "true"
:code big-coll
:max-coll-size big-size})
unchanged-default-coll-size (session/message {:op "eval"
:inspect "true"
:code big-coll})]
(is (re-find (coll-pattern size-limit :truncate) ;; #"\( 0 1 2 3 4 ... \)"
(extract-text default-coll-size)))
(is (re-find (coll-pattern big-size) ;; #"\( 0 1 2 3 4 5 6 7 8 9 \)"
(extract-text large-coll-size)))
(is (re-find (coll-pattern size-limit :truncate)
(extract-text unchanged-default-coll-size)))))

(testing "max coll size can be changed without re-eval'ing last form"
(let [default-coll-size (session/message {:op "eval"
:inspect "true"
:code big-coll})
large-coll-size (session/message {:op "inspect-set-max-coll-size"
:max-coll-size big-size})
smaller-coll-size (session/message {:op "inspect-set-max-coll-size"
:max-coll-size (dec big-size)})
unchanged-default-coll-size (session/message {:op "eval"
:inspect "true"
:code big-coll})]
(is (re-find (coll-pattern size-limit :truncate)
(extract-text default-coll-size)))
(is (re-find (coll-pattern big-size)
(extract-text large-coll-size)))
(is (re-find (coll-pattern (dec big-size) :truncate)
(extract-text smaller-coll-size)))
(is (re-find (coll-pattern size-limit :truncate)
(extract-text unchanged-default-coll-size)))))))

(deftest print-length-independence-test
(testing "*print-length* doesn't break rendering of long collections"
(is (re-find #"showing page: \d+ of \d+"
Expand Down