From bfb0c263dbfaaa83badfa67426f59d13d8a54435 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sun, 15 Dec 2019 12:33:27 -0700 Subject: [PATCH 01/19] Adds draft of ADR --- doc/arch/adr-003.md | 94 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 doc/arch/adr-003.md diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md new file mode 100644 index 00000000..e0cf70e1 --- /dev/null +++ b/doc/arch/adr-003.md @@ -0,0 +1,94 @@ +# ADR 003: Easier editing of error messages + +## Problem Statement + +Besides a fixed set of [printer options](https://github.com/bhb/expound#printer-options), users can only manipulate error messages by parsing and manipulating the string returned by `expound-str`. + +Expound does not consider the format of the error messages to be part of the API and so changes that break client string-parsing code is likely to break at some point. + +Why might a user want to manipulate the error message? + +* Minimizing [large vectors](https://github.com/bhb/expound/issues/108) or [maps](https://github.com/bhb/expound/issues/129) when printing out the invalid value +* [Adding additional information](https://github.com/bhb/expound/issues/148) + +## Context + +Currently Expound can only return a single string for the error message. If a user wants to add, omit, or change information, they must do so using the string. + +It's not clear that users would use an solution that allowed for easier editing of error messages: they might instead just want more fixes or features that solve the problem automatically. + +If Expound tries to do the right thing in all cases, it will get complext to maintain and to use since users will have to guess how/why it's showing or omitting certain information. + +## Prior Art + +### [Hiccup](https://github.com/weavejester/hiccup) + +Hiccup is a data format for representing HTML + +**Tradeoffs in the context of this problem:** + +* \+ Optional second element (map) is a place to add classes and IDs, which add semantic meaning +* \- Not intended to cover aspects like indentation and whitespace, since HTML/CSS is responsible for that + +### [Fipp](https://github.com/brandonbloom/fipp/blob/master/doc/primitives.md)
 +Fipp is a better pretty printer for Clojure and ClojureScript + +**Tradeoffs in the context of this problem:** + +* \+ Focused on pretty printing, so primitives reflect that +* \- No place to add meaning which would allow users to find content based on ID or class +* ? Potentially more general than this particular problem requires +* ? Fipp is very fast, but Expound doesn't require that much speed + +## Possible solutions + +### Include Fipp + +Expound could depend on Fipp for pretty-printing. All functions that generate a string would return Fipp data structures. + +**Tradeoffs** + +* \+ Don't need to reimplement algorithm +* \+ Can rely on existing documentation for data primitives +* ? Does Fipp provide code to extract/manipulate the primitives? Do we want to provide this so Expound clients can manipulate data structure? +* \- Take on two new deps: Fipp and `org.clojure/core.rrb-vector`. + * Fipp is well-maintained, but there are complex issues like [this](https://github.com/brandonbloom/fipp/issues/60) +* \- Doesn't provide way to add semantic information about elements beyond formatting +* \- Requires a wrapping layer if we don't want clients to be exposed to Fipp internals + +### Write a custom data description + +I could come up with my own custom data language to describe the errors. + +* \- Need to write more code +* \+ Can make it custom to error messages +* \- Need to write more documentation for format +* \+ Creates a more stable API for changes (format is stable, although content of strings are not) +* \- Creates a more complex API (more functions) +* \- Requires more testing for backwards compatibility +* \+ Speeds up building internal features +* \+ Provides solution for advanced users that doesn't rely on string manipulation + * Encourages libraries to use Expound as dependency + * Provides temporary solution for advanced use cases (can still build configuration if I want, but not necessary for all cases) + * May provide long-term fix for use cases I don't want to move into Expound + +### Don't expose data description + +I could make an internal data API that is not exposed to users. + +**Tradeoffs** + +* \+ No need to add new functions (which won't be used my most users anyway) +* \+ Still allows faster iteration on internal features +* \- If I want to solve the issues linked above, I'll need to either add complexity to the Expound rules OR add more configuration + * Additional configuration adds potential for bugs as number of combinations grows + * I'm not sure users use the configuration I expose now +* \+ No additional API to test for breaking changes +* \- String format becomes an API that users cannot rely on +* \- Does not encourage tools to use Expound internally + +## Decision + +## Status + +Draft From 86468accae072b0166b86d4583ec78297a4abdd3 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Thu, 26 Dec 2019 16:37:41 -0700 Subject: [PATCH 02/19] c --- doc/arch/adr-003.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md index e0cf70e1..644d50cd 100644 --- a/doc/arch/adr-003.md +++ b/doc/arch/adr-003.md @@ -11,6 +11,20 @@ Why might a user want to manipulate the error message? * Minimizing [large vectors](https://github.com/bhb/expound/issues/108) or [maps](https://github.com/bhb/expound/issues/129) when printing out the invalid value * [Adding additional information](https://github.com/bhb/expound/issues/148) +### Specific problems + +#### [#129](https://github.com/bhb/expound/issues/129) + +* When a map matches some user-defined pattern (e.g. contains some keys) and/or when the problem is of a certain problem type (e.g. the "missing keys" type, which currently shows a table of missing keys), hide most of the map +* Note that for some problem type, this could hide info - e.g. a predicate that checked length of map + + +#### [#129](https://github.com/bhb/expound/issues/129) + +### Non-problems + +* Minor formatting tweaks to error messages e.g. changes in indentation + ## Context Currently Expound can only return a single string for the error message. If a user wants to add, omit, or change information, they must do so using the string. From 4b9562e03f228b62aba724890a5d4bee3a835611 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Thu, 26 Dec 2019 16:47:10 -0700 Subject: [PATCH 03/19] cp --- doc/arch/adr-003.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md index 644d50cd..f9959be4 100644 --- a/doc/arch/adr-003.md +++ b/doc/arch/adr-003.md @@ -18,8 +18,22 @@ Why might a user want to manipulate the error message? * When a map matches some user-defined pattern (e.g. contains some keys) and/or when the problem is of a certain problem type (e.g. the "missing keys" type, which currently shows a table of missing keys), hide most of the map * Note that for some problem type, this could hide info - e.g. a predicate that checked length of map +#### [#110](https://github.com/bhb/expound/issues/110) -#### [#129](https://github.com/bhb/expound/issues/129) +* When sequences are long, shrink output by omitting repeated "..." values +* This depends on value, not necessarily on type of problem +* For certain values with many values that are similar, only showing the failing value is insufficient, but `print-valid-values` shows too much data. +* Perhaps whitelist values to show? +* Perhaps add notion of "distance" from problem? Does this depend entirely on type of data? + +#### [#108](https://github.com/bhb/expound/issues/108) + +* When value is very large (especially nested values), hide higher levels of values +* Does not generally depend on problem type + +#### [#148](https://github.com/bhb/expound/issues/148) + +* Given the values and the error, allow user to inject content between value nad the error (or really, anywhere) ### Non-problems From 63dbcbf3aba37fb15991d2f57988a471d616702b Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sun, 29 Dec 2019 11:59:22 -0700 Subject: [PATCH 04/19] co --- doc/arch/adr-003.md | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md index f9959be4..49ec877b 100644 --- a/doc/arch/adr-003.md +++ b/doc/arch/adr-003.md @@ -35,6 +35,47 @@ Why might a user want to manipulate the error message? * Given the values and the error, allow user to inject content between value nad the error (or really, anywhere) +#### "Embed spec errors" (No issue filed) + +> My case: user creates a router, a lot of rules are run (syntax, conflicts, dependencies, route data specs etc.). Spec validation is just one part and… I would like to return enought context information what and where it failed. Currently: + +``` +(require '[reitit.core :as r]) +(require '[clojure.spec.alpha :as s]) +(require '[expound.alpha :as e]) + +(s/def ::role #{:admin :manager}) +(s/def ::roles (s/coll-of ::role :into #{})) + +(r/router + ["/api" {:handler identity + ::roles #{:adminz}}] + {::rs/explain e/expound-str + :validate rs/validate-spec!}) +; CompilerException clojure.lang.ExceptionInfo: Invalid route data: +; +; -- On route ----------------------- +; +; "/api" +; +; -- Spec failed -------------------- +; +; {:handler ..., :user/roles #{:adminz}} +; ^^^^^^^ +; +; should be one of: `:admin`,`:manager` +; +``` + +Given some context, add additional context around the spec. + +#### Customize printing for record or value (No issue filed) + +> Can I prevent Expound from printing a certain record? It generates something like 140k lines of output which is not helpful at all. I defined a `print-method` so that prints something simple with `prn` but that does not seem to affect Expound. + +Given some value (or properties of value), allow user to specify custom output (presumably to shrink size of output). + + ### Non-problems * Minor formatting tweaks to error messages e.g. changes in indentation From b284558a8b0ac1af9a06b94eb444088606003e37 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Thu, 2 Jan 2020 20:32:57 -0700 Subject: [PATCH 05/19] cp --- src/expound/alpha.cljc | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 5de91050..4220c920 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1120,3 +1120,31 @@ returned an invalid value. "Given a sequence of results from `clojure.spec.test.alpha/check`, returns a string summarizing the results." [check-results] (with-out-str (explain-results check-results))) + +(comment + + ;; value + ["one" "two" 33] + ;; [... ... 33] + ;; ^^ + + ;; maybe a map of paths to meta, including distance + { + [0] {:distance 1 :type :peer} + [1] {:distance 1 :type :peer} + [2] {:distance 0 :type :value} ; distance zero is highlighted value + [] {:distance 2} ; not sure how to compute distance, but it does seem "further away" + } + + + + ;; what could you do here? maybe over values to find distance x + ;; and change it? + + + + + + + + ) From 2413cb52b04fc1c3e3dad315d42beedd4267c463 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 4 Jan 2020 09:39:32 -0700 Subject: [PATCH 06/19] cp --- src/expound/alpha.cljc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 4220c920..1340011a 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1126,7 +1126,7 @@ returned an invalid value. ;; value ["one" "two" 33] ;; [... ... 33] - ;; ^^ + ;; ^^ ;; maybe a map of paths to meta, including distance { @@ -1141,6 +1141,12 @@ returned an invalid value. ;; what could you do here? maybe over values to find distance x ;; and change it? + ;; every value is a container, or a key, or a value + ;; I don't think 'peer' is useful becaue it only applies to + ;; elements that are peers of distance 0 value + + + From 291ce2e2c399b71bd43d1998cd9991aa7f245973 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 4 Jan 2020 09:58:13 -0700 Subject: [PATCH 07/19] cp --- doc/arch/adr-003.md | 11 ++++++++++- src/expound/alpha.cljc | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md index 49ec877b..97011e79 100644 --- a/doc/arch/adr-003.md +++ b/doc/arch/adr-003.md @@ -79,6 +79,7 @@ Given some value (or properties of value), allow user to specify custom output ( ### Non-problems * Minor formatting tweaks to error messages e.g. changes in indentation +* No requests to change the specific characters i.e. hide values with ",,," instead of "." or underline with "---" instead of "^^^" ## Context @@ -109,7 +110,7 @@ Fipp is a better pretty printer for Clojure and ClojureScript * ? Potentially more general than this particular problem requires * ? Fipp is very fast, but Expound doesn't require that much speed -## Possible solutions +## Possible solutions: general approach ### Include Fipp @@ -156,6 +157,14 @@ I could make an internal data API that is not exposed to users. * \- String format becomes an API that users cannot rely on * \- Does not encourage tools to use Expound internally +## Possible solutions: data language + +### Return original value plus mapping from `:in` path to value + +It's too tricky to embed metadata within the original data, so we provide a lookup table that provides information to assist with formatting. + +We also need to pass along the category of failure because we might alter the output. + ## Decision ## Status diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 1340011a..56fa1ae8 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1145,6 +1145,39 @@ returned an invalid value. ;; I don't think 'peer' is useful becaue it only applies to ;; elements that are peers of distance 0 value + ;; Expound - a predicate like nil only needs location vs value. Location is where to find it, value is what it is. + + + + :kw + + ;; { [] {:distance 0, :type :value}} + + ;; need to design own pretty print algorithm to layout values + + + + [:foo 1 :bar :baz] + + + + {:age 1 :keys-spec/name :bob} +;; {:age ..., :keys-spec/name :bob} +;; ^^^^ + + { + ;; or whatever path incantation is necessary for finding keys + ;; are the keys further away than the value? + [:age 0] {:distance 2 :role :key :hidden? false} + [:age 1] {:distance 1 :role :value :hidden? true} + [:keys-spec/name 0] {:distance 1 :role :value, :hidden? true} + [:keys-spec/name 1] {:distance 0 :role :value, :hidden? false, :highlight? true} + ;; Is the containers further distance than the keys? + [] {:distance 3 :role :container} + + + } + From 36242d532cd6896cd7fd0811936a951de40ab543 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 4 Jan 2020 10:50:51 -0700 Subject: [PATCH 08/19] cp --- doc/arch/adr-003.md | 17 +++++++++++++++++ src/expound/alpha.cljc | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md index 97011e79..148727b8 100644 --- a/doc/arch/adr-003.md +++ b/doc/arch/adr-003.md @@ -110,6 +110,23 @@ Fipp is a better pretty printer for Clojure and ClojureScript * ? Potentially more general than this particular problem requires * ? Fipp is very fast, but Expound doesn't require that much speed +### [Clojure pretty printer](https://clojure.github.io/clojure/doc/clojure/pprint/PrettyPrinting.html) + +Expound currently uses Clojure's built-in pretty-print to display values. + +> More advanced formats, +including formats that don't look like Clojure data at all like XML and +JSON, can be rendered by creating custom dispatch functions. + +- + +Flowchart of how to use + +* \- Need slightly different implementation for ClojureScript and Clojure +* \- Seems to only work on types, need a new type for each role in data structure +* \+ Wouldn't have to worry about writing layout engine +* \+ Configurable by users if they didn't like how I formatted tree + ## Possible solutions: general approach ### Include Fipp diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 56fa1ae8..12b9b86d 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1174,10 +1174,39 @@ returned an invalid value. [:keys-spec/name 1] {:distance 0 :role :value, :hidden? false, :highlight? true} ;; Is the containers further distance than the keys? [] {:distance 3 :role :container} - - } + (write-out {}) + + + clojure.pprint/*print-pretty* + *print-length* + + (println (range 0 30)) + + clojure.pprint/*print-pprint-dispatch* + + + + (binding [clojure.pprint/*print-pretty* true] + (clojure.pprint/with-pretty-writer + + (clojure.pprint/write-out (range 0 30))) + + ) + ;; how dispatching works on multimethod + ;; https://github.com/clojure/clojurescript/blob/23cedecbf4f704f9fee672e395bbfa1e3fe3ee1a/src/main/cljs/cljs/pprint.cljs#L2893-L2909 + ;; vs + ;; https://github.com/clojure/clojure/blob/4ef4b1ed7a2e8bb0aaaacfb0942729252c2c3091/src/clj/clojure/pprint/dispatch.clj#L151-L157 + ;; in CLJ + + + + + + + + From 8d9a954a535b559eba7ca83fd940904037b49508 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 4 Jan 2020 12:03:39 -0700 Subject: [PATCH 09/19] cp --- src/expound/alpha.cljc | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 12b9b86d..d7f85b09 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1200,6 +1200,46 @@ returned an invalid value. ;; https://github.com/clojure/clojure/blob/4ef4b1ed7a2e8bb0aaaacfb0942729252c2c3091/src/clj/clojure/pprint/dispatch.clj#L151-L157 ;; in CLJ + (require '[clojure.pprint :as pprint]) + + ;; hack to redefine method + (def new-dispatch nil) + + (defmulti + new-dispatch + "The pretty print dispatch function for simple data structure format." + (fn [m] + (:role m) + ) + ) + + + (defmethod new-dispatch :default [x] + (pprint/simple-dispatch x) + ) + + (defmethod new-dispatch :value [m] + (pprint/pprint-logical-block :prefix "[" :suffix "]" + (pprint/print-length-loop [aseq (seq (:x m))] + (when aseq + (pprint/write-out (first aseq)) + (when (next aseq) + ;; This will have to change for CLJS + #_ (-write *out* " ") + (.write ^java.io.Writer *out* " ") + (pprint/pprint-newline :linear) + (recur (next aseq)))))) + + ) + + + (clojure.pprint/with-pprint-dispatch new-dispatch + (clojure.pprint/pprint {:role :value :x (range 0 30)}) + ) + + + + From dddbe49d3332202b1822c289810eee95a64d7ee1 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 4 Jan 2020 12:41:15 -0700 Subject: [PATCH 10/19] cp --- src/expound/alpha.cljc | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index d7f85b09..a0fafa13 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1219,7 +1219,19 @@ returned an invalid value. ) (defmethod new-dispatch :value [m] - (pprint/pprint-logical-block :prefix "[" :suffix "]" + (cond + + (:hidden? m) + (.write ^java.io.Writer *out* "...") + + :else +(pprint/write-out (:x m)) + + ) + + + + #_(pprint/pprint-logical-block :prefix "[" :suffix "]" (pprint/print-length-loop [aseq (seq (:x m))] (when aseq (pprint/write-out (first aseq)) @@ -1234,8 +1246,17 @@ returned an invalid value. (clojure.pprint/with-pprint-dispatch new-dispatch - (clojure.pprint/pprint {:role :value :x (range 0 30)}) + (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? false}) ) + + (clojure.pprint/with-pprint-dispatch new-dispatch + (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? true}) + ) + + ;; HERE: doing some experiments on using pprint to underline and hide values + + + From e6d8fbbdd9aeb568ed7ca7bb91a6778abc537686 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 4 Jan 2020 12:53:27 -0700 Subject: [PATCH 11/19] cop --- doc/arch/adr-003.md | 1 - src/expound/alpha.cljc | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md index 148727b8..388099b6 100644 --- a/doc/arch/adr-003.md +++ b/doc/arch/adr-003.md @@ -75,7 +75,6 @@ Given some context, add additional context around the spec. Given some value (or properties of value), allow user to specify custom output (presumably to shrink size of output). - ### Non-problems * Minor formatting tweaks to error messages e.g. changes in indentation diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index a0fafa13..f128b70a 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1220,6 +1220,11 @@ returned an invalid value. (defmethod new-dispatch :value [m] (cond + (zero? (:distance m)) + (pprint/pprint-logical-block + (pprint/write-out (:x m)) + (.write ^java.io.Writer *out* "^^^^^^^^^") + ) (:hidden? m) (.write ^java.io.Writer *out* "...") From f04bc4e2aea6753ad8baa835fa6ba1f5a55ba281 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sun, 5 Jan 2020 14:06:24 -0700 Subject: [PATCH 12/19] cp --- src/expound/alpha.cljc | 84 +++++++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 21 deletions(-) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index f128b70a..943cffc4 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1200,18 +1200,55 @@ returned an invalid value. ;; https://github.com/clojure/clojure/blob/4ef4b1ed7a2e8bb0aaaacfb0942729252c2c3091/src/clj/clojure/pprint/dispatch.clj#L151-L157 ;; in CLJ + ;; example of printing + #_(pprint/pprint-logical-block :prefix "[" :suffix "]" + (pprint/print-length-loop [aseq (seq (:x m))] + (when aseq + (pprint/write-out (first aseq)) + (when (next aseq) + ;; This will have to change for CLJS + #_ (-write *out* " ") + (.write ^java.io.Writer *out* " ") + (pprint/pprint-newline :linear) + (recur (next aseq)))))) + (require '[clojure.pprint :as pprint]) ;; hack to redefine method (def new-dispatch nil) - (defmulti - new-dispatch - "The pretty print dispatch function for simple data structure format." - (fn [m] - (:role m) + (defmulti new-dispatch + "TODO - name" + (fn [m] + (:role m) + ) ) - ) + + ;; this is promising but how do I connect each form + ;; with metadata? obviously each form is not unique overall + + ;; since this is breaking change, probably don't release except for in expound.alpha2 + + + ;; the way this used to work is that we walked the tree using 'summary value' and printed it out as + ;; we went, using the through the structure and re-created it recursively + ;; + + ;; this is tricky because naively printing out the form recursively will not + ;; capture the metadata in the data + ;; it's an open question whether we can really reliably create that metadata, + ;; but that's a problem for another day. + + ;; HERE - how to recursively print out data structure and grab meta-data + ;; data strucutre is implicit in structure which is a good thing + ;; additionally if we want to print out index of bad value, (location) + ;; then we need more than the current value + + ;; let's rethink location. + ;; is showing location as part of data structure useful? + ;; yes, because mapping a path to a data structure omits the container types + ;; etc + (defmethod new-dispatch :default [x] @@ -1220,9 +1257,11 @@ returned an invalid value. (defmethod new-dispatch :value [m] (cond + (zero? (:distance m)) (pprint/pprint-logical-block (pprint/write-out (:x m)) + (pprint/pprint-newline :mandatory) (.write ^java.io.Writer *out* "^^^^^^^^^") ) @@ -1230,36 +1269,39 @@ returned an invalid value. (.write ^java.io.Writer *out* "...") :else -(pprint/write-out (:x m)) + (pprint/write-out (:x m)) ) - - #_(pprint/pprint-logical-block :prefix "[" :suffix "]" - (pprint/print-length-loop [aseq (seq (:x m))] - (when aseq - (pprint/write-out (first aseq)) - (when (next aseq) - ;; This will have to change for CLJS - #_ (-write *out* " ") - (.write ^java.io.Writer *out* " ") - (pprint/pprint-newline :linear) - (recur (next aseq)))))) ) (clojure.pprint/with-pprint-dispatch new-dispatch - (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? false}) + (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? false :distance 0}) ) (clojure.pprint/with-pprint-dispatch new-dispatch - (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? true}) + (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? true :distance 0}) + ) + + + ;; Yes, clojure records can have metadata + (defrecord Foo [x y]) + + (let [f (with-meta (Foo. 1 2) {:bye true}) + + ] + [(.x f) (.y f) (meta f)] + ) - ;; HERE: doing some experiments on using pprint to underline and hide values + ;; HERE - how to embed entire thing in deeper data structure? + + + From ecf3fe75857e2a14aa15895d9fb9f8aeffb6b85f Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sun, 19 Jan 2020 15:36:49 -0700 Subject: [PATCH 13/19] cp --- src/expound/alpha.cljc | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 943cffc4..05bcbd75 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1244,10 +1244,25 @@ returned an invalid value. ;; additionally if we want to print out index of bad value, (location) ;; then we need more than the current value + ;; TODO: transfer these notes to ADR ;; let's rethink location. - ;; is showing location as part of data structure useful? - ;; yes, because mapping a path to a data structure omits the container types - ;; etc + ;; why is showing location as part of data structure useful? + ;; because mapping a path to a data structure omits the container types + ;; [0 0 0 1] doesn't say if those are lists, vecs or (unlikely but possible) + ;; maps so user has to figure that out in their head. + ;; The question we're trying to answer is: where is the invalid value located? + ;; This is fundamentally a function of the context value, the specific value + ;; (need better terminology for this), and also I think, the problem type itself + ;; + ;; Actually the problem of describing the location is arguably NOT related to the + ;; problem type. We always want to consistently show where the problem is, but + ;; depending on the problem type, we might be able to summarize the problem value? + + ;; context value - the entire value that is checked + ;; problem value - the specific value that is failing + + ;; Hm, that's not quite true, buecase + @@ -1298,21 +1313,9 @@ returned an invalid value. ) ;; HERE - how to embed entire thing in deeper data structure? - - - - - - - - - - - - - + From 28ccab7abdaa0e0dde1d3d831741d5b61f814dfa Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Wed, 22 Jan 2020 21:03:34 -0700 Subject: [PATCH 14/19] p --- src/expound/alpha.cljc | 77 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 11 deletions(-) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 05bcbd75..d5f59f01 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1253,43 +1253,76 @@ returned an invalid value. ;; The question we're trying to answer is: where is the invalid value located? ;; This is fundamentally a function of the context value, the specific value ;; (need better terminology for this), and also I think, the problem type itself + + ;; context value - the entire value that is checked + ;; problem value - the specific value that is failing + ;; ;; Actually the problem of describing the location is arguably NOT related to the ;; problem type. We always want to consistently show where the problem is, but ;; depending on the problem type, we might be able to summarize the problem value? - ;; context value - the entire value that is checked - ;; problem value - the specific value that is failing - ;; Hm, that's not quite true, buecase - + ;; Hm, that's not quite true, because there are actually three concerns here. + ;; 1. "context value" - this helps us show location of the "problem value" + ;; 2. "problem value" - help understand problem (by display enough info), also helps identify + ;; problem value (imagine a giant map which should be nil, do we need to print it all? Or + ;; can we summarize it for that predicate?) + ;; so it's printing enough of the problem value in order to understand problem. + ;; Problem value and problem are connected, but + ;; The problem here is that people identify values via location and the problem value itself, + ;; so it's not clear that we can summarize generally, since it may be more useful to say "oh, the thing that has x" + ;; vs "the thing located at spot x" + ;; This is especially relevant for recursive values where things can be nil, do we need to show the entire top-level value + ;; just to say it's nil? + ;; Maybe a summary is sufficient with suffient location info? + ;; If the values in the context are tagged with distance, we could also tag problem values with depth. + + ;; What if we had type, distance, depth. + ;; 3. problem - separate from above, describes what is wrong - + ;; Are there other ways to show context? + + (defmethod new-dispatch :default [x] (pprint/simple-dispatch x) ) + + (defmethod new-dispatch :value [m] (cond - (zero? (:distance m)) (pprint/pprint-logical-block (pprint/write-out (:x m)) - (pprint/pprint-newline :mandatory) + (pprint/pprint-newline :linear) + ;; HERE ... this won't work because we could + ;; have additional data on this line (.write ^java.io.Writer *out* "^^^^^^^^^") ) (:hidden? m) - (.write ^java.io.Writer *out* "...") + (pprint/pprint-logical-block + (.write ^java.io.Writer *out* "...") + (.write ^java.io.Writer *out* " ") + (pprint/pprint-newline :linear) + ) :else - (pprint/write-out (:x m)) + (pprint/pprint-logical-block + (pprint/write-out (:x m)) + (.write ^java.io.Writer *out* " ") + (pprint/pprint-newline :linear) + ) ) - + ) - + (defmethod new-dispatch :collection [xs] + (doseq [x (:x xs)] + (pprint/write-out x) + ) ) @@ -1301,6 +1334,28 @@ returned an invalid value. (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? true :distance 0}) ) + (clojure.pprint/with-pprint-dispatch new-dispatch + (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? true :distance 1}) + ) + + (clojure.pprint/with-pprint-dispatch new-dispatch + (clojure.pprint/pprint + {:role :collection + :x [ + {:role :value :x "a" :hidden? true :distance 1} + {:role :value :x "b" :hidden? false :distance 0} + {:role :value :x "c" :hidden? true :distance 1} + ] + :hidden? false + :distance? 1 + } + ) + ) + + + + + ;; Yes, clojure records can have metadata (defrecord Foo [x y]) From 8012d1e31df7aa1372e342014afaa2bad5bd5893 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sat, 25 Jan 2020 19:56:46 -0700 Subject: [PATCH 15/19] cp --- src/expound/alpha.cljc | 73 +++++++++++++++++++++++++----------------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index d5f59f01..86d4f32e 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1288,18 +1288,20 @@ returned an invalid value. (defmethod new-dispatch :default [x] (pprint/simple-dispatch x) ) - - + + ;; This might work well because it won't affect print width + (def marker :expound.problems/relevant) + (println marker) (defmethod new-dispatch :value [m] (cond (zero? (:distance m)) (pprint/pprint-logical-block + (pprint/write-out marker) (pprint/write-out (:x m)) (pprint/pprint-newline :linear) - ;; HERE ... this won't work because we could - ;; have additional data on this line - (.write ^java.io.Writer *out* "^^^^^^^^^") + (pprint/write-out marker) + (.write ^java.io.Writer *out* " ") ) (:hidden? m) @@ -1338,23 +1340,45 @@ returned an invalid value. (clojure.pprint/pprint {:role :value :x "abcdef" :hidden? true :distance 1}) ) - (clojure.pprint/with-pprint-dispatch new-dispatch - (clojure.pprint/pprint - {:role :collection - :x [ - {:role :value :x "a" :hidden? true :distance 1} - {:role :value :x "b" :hidden? false :distance 0} - {:role :value :x "c" :hidden? true :distance 1} - ] - :hidden? false - :distance? 1 - } - ) + (let [s (with-out-str + (clojure.pprint/with-pprint-dispatch new-dispatch + (clojure.pprint/pprint + {:role :collection + :x [ + {:role :value :x "a" :hidden? true :distance 1} + {:role :value :x "b" :hidden? false :distance 0} + {:role :value :x "c" :hidden? true :distance 1} + ] + :hidden? false + :distance? 1 + } + ) + )) + ;; This isn't a great idea because this string takes width and therefore will + ;; mess with printing width + + re #"(.*)expound.problems/relevant(.*)expound.problems/relevant(.*)" + ;;[before after] (clojure.string/split s re) + [line before val after] (re-find re s) + ] + + {:s s + :val val + :before before + :after after + } + + #_(println (str before + val + after + "\n" + " ^^^" + )) ) - - + + ;; we could do highlighting not only underneath, but next to value if it is multi-line ;; Yes, clojure records can have metadata @@ -1370,15 +1394,4 @@ returned an invalid value. ;; HERE - how to embed entire thing in deeper data structure? - - - - - - - - - - - ) From 88494e852d9a1786ec9b3718115ca99ec8d42aae Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sun, 26 Jan 2020 12:08:39 -0700 Subject: [PATCH 16/19] cp --- src/expound/alpha.cljc | 15 ++++--- src/expound/printer.cljc | 76 ++++++++++++++++++++++++++++++++++ test/expound/printer_test.cljc | 12 ++++++ 3 files changed, 98 insertions(+), 5 deletions(-) diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index 86d4f32e..d26af1a7 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1357,7 +1357,7 @@ returned an invalid value. ;; This isn't a great idea because this string takes width and therefore will ;; mess with printing width - re #"(.*)expound.problems/relevant(.*)expound.problems/relevant(.*)" + re #"(.*):expound.problems/relevant(.*):expound.problems/relevant(.*)" ;;[before after] (clojure.string/split s re) [line before val after] (re-find re s) ] @@ -1368,17 +1368,22 @@ returned an invalid value. :after after } - #_(println (str before + (println (str before val after "\n" - " ^^^" + " ^^^" )) ) + - - ;; we could do highlighting not only underneath, but next to value if it is multi-line + ;; we could do highlighting not only underneath, but next to value if it is multi-line e.g. > + ;; > + ;; > + ;; Of course, this could be super large for very large values + ;; + ;; making distance and depth different values would of course make it tricky to vary them independently in settings ;; Yes, clojure records can have metadata diff --git a/src/expound/printer.cljc b/src/expound/printer.cljc index 137039b3..af90df6c 100644 --- a/src/expound/printer.cljc +++ b/src/expound/printer.cljc @@ -5,6 +5,7 @@ [clojure.set :as set] [expound.util :as util] [expound.ansi :as ansi] + [clojure.pprint :as pprint] #?(:clj [clojure.main :as main])) (:refer-clojure :exclude [format])) @@ -323,3 +324,78 @@ (map #(str (apply str (repeat rest-lines-indent " ")) %)) (into [(str (apply str (repeat first-line-indent " ")) line)]) (string/join "\n"))))) + + +(def marker :expound.problems/relevant) + +(defmulti new-dispatch + "TODO: name" + (fn [m] + + (:role m) + ) + ) + +(defmethod new-dispatch :default [x] + (pprint/simple-dispatch x)) + +;; TODO: rename +(defmethod new-dispatch :scalar [m] + (let [{:keys [value distance _depth]} m] + (cond + (zero? distance) + (pprint/pprint-logical-block + (pprint/write-out marker) + (pprint/write-out value) + (pprint/pprint-newline :linear) + (pprint/write-out marker) + (.write ^java.io.Writer *out* " ") + ) + + (:hidden? m) + (pprint/pprint-logical-block + (.write ^java.io.Writer *out* "...") + (.write ^java.io.Writer *out* " ") + (pprint/pprint-newline :linear) + ) + + :else + (pprint/pprint-logical-block + (pprint/write-out value) + (.write ^java.io.Writer *out* " ") + (pprint/pprint-newline :linear) + ) + )) + ) + +(defmethod new-dispatch :collection [xs] + (doseq [x (:x xs)] + (pprint/write-out x) + ) + ) + +(defn value2 [xs] + (pprint/with-pprint-dispatch new-dispatch + (let [out (java.io.StringWriter.)] + (pprint/pprint xs out) + (.toString out)) + ) + ) + +(comment + (pprint/with-pprint-dispatch new-dispatch + (let [out (java.io.StringWriter.)] + (pprint/pprint {:role :scalar + :value 1 + :distance 0 + :depth 0 + }) + (.toString out)) + ) + + (value2 {:role :scalar + :value 1 + :distance 0 + :depth 0 + }) + ) diff --git a/test/expound/printer_test.cljc b/test/expound/printer_test.cljc index 513159ed..8891230e 100644 --- a/test/expound/printer_test.cljc +++ b/test/expound/printer_test.cljc @@ -232,3 +232,15 @@ srows (rest (string/split table #"\n"))]] (is (< (count (last srows)) 200))))) + +(deftest value2 + ;; prints basic value with no highlighting + (is (= + "" + (printer/value2 {:role :scalar + :value 1 + :distance 0 + :depth 0 + }) + )) + ) From 0450b540502261efb466e422f545b2137757511b Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sun, 26 Jan 2020 12:29:26 -0700 Subject: [PATCH 17/19] cp --- src/expound/printer.cljc | 69 +++++++++++++++++++++++++++++++--- test/expound/printer_test.cljc | 3 +- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/src/expound/printer.cljc b/src/expound/printer.cljc index af90df6c..34858ac5 100644 --- a/src/expound/printer.cljc +++ b/src/expound/printer.cljc @@ -342,6 +342,7 @@ ;; TODO: rename (defmethod new-dispatch :scalar [m] (let [{:keys [value distance _depth]} m] + (def dvalue value) (cond (zero? distance) (pprint/pprint-logical-block @@ -369,18 +370,69 @@ ) (defmethod new-dispatch :collection [xs] - (doseq [x (:x xs)] + (doseq [x (:value xs)] (pprint/write-out x) ) + ) + +(defn swidth [s] + (apply max + (map + count + (string/split-lines s))) + ) + +(comment + (let [s "adfasdf\n234\n34"] + + + ) + (apply str (repeat 5 " ")) + ) + +;; TODO: rename +(defn hiccup [s] + (map + (fn [x] [:span (string/trim x)]) + (map + string/trimr + (string/split-lines s))) + ) + +(defn highlight-value [s] + (let [re #"(.*):expound.problems/relevant(.*):expound.problems/relevant(.*)" + [_line before val after] (re-find re s) + + indent (apply str (repeat (swidth before) " ")) + underline (apply str (repeat (swidth val) "^")) + ] + (str before + val + after + "\n" + indent + underline + )) + ) + (defn value2 [xs] - (pprint/with-pprint-dispatch new-dispatch - (let [out (java.io.StringWriter.)] - (pprint/pprint xs out) - (.toString out)) + (let [s (pprint/with-pprint-dispatch new-dispatch + (let [out (java.io.StringWriter.)] + (pprint/pprint xs out) + (.toString out)))] + + + (hiccup (highlight-value s)) + ) - ) + ) + + +;; TODO: do I need to return a list of indented elements? +;; post processing: +;; [:span "fobbar"] (comment (pprint/with-pprint-dispatch new-dispatch @@ -392,10 +444,15 @@ }) (.toString out)) ) + + (highlight-value ":expound.problems/relevant1:expound.problems/relevant \n") (value2 {:role :scalar :value 1 :distance 0 :depth 0 }) + + dvalue + ) diff --git a/test/expound/printer_test.cljc b/test/expound/printer_test.cljc index 8891230e..0e500d63 100644 --- a/test/expound/printer_test.cljc +++ b/test/expound/printer_test.cljc @@ -236,7 +236,8 @@ (deftest value2 ;; prints basic value with no highlighting (is (= - "" + [[:span "1"] + [:span "^"]] (printer/value2 {:role :scalar :value 1 :distance 0 From 642567f84757f86922f45977f1446cbeb2cc9cba Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sun, 26 Jan 2020 18:20:29 -0700 Subject: [PATCH 18/19] cp --- doc/arch/adr-003.md | 1 + test/expound/printer_test.cljc | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md index 388099b6..2a0f8a33 100644 --- a/doc/arch/adr-003.md +++ b/doc/arch/adr-003.md @@ -17,6 +17,7 @@ Why might a user want to manipulate the error message? * When a map matches some user-defined pattern (e.g. contains some keys) and/or when the problem is of a certain problem type (e.g. the "missing keys" type, which currently shows a table of missing keys), hide most of the map * Note that for some problem type, this could hide info - e.g. a predicate that checked length of map +* Some information about the value may not just be used to understand why it failed predicate, but also to locate the value. For instance, consider an array with many maps, in which every map contains an `:id` key - we may not want to hide the `:id` value if it helps user locate the invalid value. #### [#110](https://github.com/bhb/expound/issues/110) diff --git a/test/expound/printer_test.cljc b/test/expound/printer_test.cljc index 0e500d63..7f6d532e 100644 --- a/test/expound/printer_test.cljc +++ b/test/expound/printer_test.cljc @@ -244,4 +244,57 @@ :depth 0 }) )) + ;; should the input be in hiccup-style values + + ["a" 1 "b"] + ;; where 1 is the bad value + + {:role :collection + :distance 1 + :value [ + {:role :value + :value "a" + :distance 1 + :depth 0 + } + {:role :value + :value 1 + :distance 0 + :depth 0 + } + {:role :value + :value "b" + :distance 1 + :depth 0 + } + ] + } + ;; pros: + ;; - collection type is explicit + ;; - no indirection for information about value + ;; cons: + ;; - not sure what happens for custom types e.g. records + ;; - hard to see actual structure + ;; - verbose + + + + + + + + ;; highlights value within collection + (is (= + [[:span "1"] + [:span "^"]] + (printer/value2 + [:span + {:role :collection + :distance 1 + :depth 0 + } + ] + ) + )) + ) From f907cc9da43b02c19577560e0ff3631f003c0485 Mon Sep 17 00:00:00 2001 From: Ben Brinckerhoff Date: Sun, 26 Jan 2020 19:28:24 -0700 Subject: [PATCH 19/19] cp --- doc/arch/adr-003.md | 10 +-- src/expound/alpha.cljc | 4 +- src/expound/printer.cljc | 120 +++++++++++++++++++++++++-------- test/expound/printer_test.cljc | 106 ++++++++++++++++++++++++++--- 4 files changed, 196 insertions(+), 44 deletions(-) diff --git a/doc/arch/adr-003.md b/doc/arch/adr-003.md index 2a0f8a33..5e3ce5ca 100644 --- a/doc/arch/adr-003.md +++ b/doc/arch/adr-003.md @@ -34,7 +34,7 @@ Why might a user want to manipulate the error message? #### [#148](https://github.com/bhb/expound/issues/148) -* Given the values and the error, allow user to inject content between value nad the error (or really, anywhere) +* Given the values and the error, allow user to inject content between value and the error (or really, anywhere) #### "Embed spec errors" (No issue filed) @@ -87,7 +87,7 @@ Currently Expound can only return a single string for the error message. If a us It's not clear that users would use an solution that allowed for easier editing of error messages: they might instead just want more fixes or features that solve the problem automatically. -If Expound tries to do the right thing in all cases, it will get complext to maintain and to use since users will have to guess how/why it's showing or omitting certain information. +If Expound tries to do the right thing in all cases, it will get complex to maintain and to use since users will have to guess how/why it's showing or omitting certain information. ## Prior Art @@ -114,9 +114,7 @@ Fipp is a better pretty printer for Clojure and ClojureScript Expound currently uses Clojure's built-in pretty-print to display values. -> More advanced formats, -including formats that don't look like Clojure data at all like XML and -JSON, can be rendered by creating custom dispatch functions. +> More advanced formats, including formats that don't look like Clojure data at all like XML and JSON, can be rendered by creating custom dispatch functions. - @@ -182,6 +180,8 @@ It's too tricky to embed metadata within the original data, so we provide a look We also need to pass along the category of failure because we might alter the output. +The deal-breaker here is that I can't meaningfully traverse the original data structure in order and get relevant metadata. Metadata needs to be included because there's no mechanism to look it up later. + ## Decision ## Status diff --git a/src/expound/alpha.cljc b/src/expound/alpha.cljc index d26af1a7..6d1bbd3f 100644 --- a/src/expound/alpha.cljc +++ b/src/expound/alpha.cljc @@ -1285,13 +1285,12 @@ returned an invalid value. ;; Are there other ways to show context? - (defmethod new-dispatch :default [x] + (defmethod new-dispatch :default [x] (pprint/simple-dispatch x) ) ;; This might work well because it won't affect print width (def marker :expound.problems/relevant) - (println marker) (defmethod new-dispatch :value [m] (cond @@ -1375,6 +1374,7 @@ returned an invalid value. " ^^^" )) ) + diff --git a/src/expound/printer.cljc b/src/expound/printer.cljc index 34858ac5..bc65285a 100644 --- a/src/expound/printer.cljc +++ b/src/expound/printer.cljc @@ -328,11 +328,18 @@ (def marker :expound.problems/relevant) +(comment + (vector? :xyz) + (def new-dispatch nil)) + (defmulti new-dispatch "TODO: name" - (fn [m] - - (:role m) + (fn [x] + + (if (vector? x) + (first x) + :default + ) ) ) @@ -340,9 +347,9 @@ (pprint/simple-dispatch x)) ;; TODO: rename -(defmethod new-dispatch :scalar [m] - (let [{:keys [value distance _depth]} m] - (def dvalue value) +(defmethod new-dispatch :scalar [[_ m value]] + + (let [{:keys [distance _depth]} m] (cond (zero? distance) (pprint/pprint-logical-block @@ -369,10 +376,20 @@ )) ) -(defmethod new-dispatch :collection [xs] - (doseq [x (:value xs)] - (pprint/write-out x) - ) +(defmethod new-dispatch :vector [[_ m & children]] + (.write ^java.io.Writer *out* "[") + (doseq [x children] + (pprint/write-out x) + ) + (.write ^java.io.Writer *out* "]") + ) + +(defmethod new-dispatch :index [[_ _m _children]] + ;; do not print indexes for now + ) + +(defmethod new-dispatch :value [[_ m v]] + (pprint/write-out [:scalar m v]) ) (defn swidth [s] @@ -384,9 +401,6 @@ (comment (let [s "adfasdf\n234\n34"] - - - ) (apply str (repeat 5 " ")) @@ -402,26 +416,33 @@ ) (defn highlight-value [s] - (let [re #"(.*):expound.problems/relevant(.*):expound.problems/relevant(.*)" - [_line before val after] (re-find re s) + (let [re1 #"(.*):expound.problems/relevant(.*):expound.problems/relevant(.*)" + [line before val after] (re-find re1 s) + [lines_before lines_after] (string/split s (re-pattern (string/re-quote-replacement line))) indent (apply str (repeat (swidth before) " ")) underline (apply str (repeat (swidth val) "^")) ] - (str before - val - after - "\n" - indent - underline - )) + (str lines_before + before + val + after + "\n" + indent + underline + lines_after + )) ) -(defn value2 [xs] - (let [s (pprint/with-pprint-dispatch new-dispatch +(defn value2* [xs] + (pprint/with-pprint-dispatch new-dispatch (let [out (java.io.StringWriter.)] (pprint/pprint xs out) - (.toString out)))] + (.toString out))) + ) + +(defn value2 [xs] + (let [s (value2* xs)] (hiccup (highlight-value s)) @@ -429,6 +450,10 @@ ) ) +;; what is the problem(s)? +;; some values don't omit enough information +;; other values omit too much information +;; i can't allow users to vary this while I determine sensible defaults ;; TODO: do I need to return a list of indented elements? ;; post processing: @@ -447,12 +472,51 @@ (highlight-value ":expound.problems/relevant1:expound.problems/relevant \n") - (value2 {:role :scalar + #_(value2 {:role :scalar :value 1 :distance 0 :depth 0 - }) + }) + + (value2 [:scalar + {:distance 0 + :depth 0 + } + 1 + ]) + + (value2* [:vector + {:distance 1} + [:index {:distance 1} 0] + [:value {:distance 1} "a"] + [:index {:distance 1} 1] + [:value {:distance 0} 1] + [:index {:distance 0} 2] + [:value {:distance 0} "b"] + ]) + + (highlight-value "[\"a\"\n :expound.problems/relevant1\n :expound.problems/relevant :expound.problems/relevant\"b\"\n :expound.problems/relevant ]\n") + + (value2 [:vector + {:distance 1} + [:index {:distance 1} 0] + [:value {:distance 1} "a"] + [:index {:distance 1} 1] + [:value {:distance 0} 1] + [:index {:distance 1} 2] + [:value {:distance 1} "b"] + ]) + + (value2 [:vector + {:distance 1} + [:index {:distance 1} 0] + [:value {:distance 1} "a"] + [:index {:distance 1} 1] + [:value {:distance 0} 1] + [:index {:distance 1} 2] + [:value {:distance 1} "b"] + ]) - dvalue + ) diff --git a/test/expound/printer_test.cljc b/test/expound/printer_test.cljc index 7f6d532e..a1521413 100644 --- a/test/expound/printer_test.cljc +++ b/test/expound/printer_test.cljc @@ -235,7 +235,7 @@ (deftest value2 ;; prints basic value with no highlighting - (is (= + #_(is (= [[:span "1"] [:span "^"]] (printer/value2 {:role :scalar @@ -244,6 +244,24 @@ :depth 0 }) )) + + (is (= + [[:span "1"] + ] + (printer/value2 + #_{:role :scalar + :value 1 + :distance 0 + :depth 0 + } + [:scalar + {:distance 0 + :depth 0 + } + 1 + ] + ) + )) ;; should the input be in hiccup-style values ["a" 1 "b"] @@ -276,25 +294,95 @@ ;; - not sure what happens for custom types e.g. records ;; - hard to see actual structure ;; - verbose - + + ;; could I flatten the data structure based on well-defined order? + + [{:role :vector + :distance 1 + } + {:role :value + :value "a" + :distance 1 + :depth 0 + } + {:role :value + :value 1 + :distance 0 + :depth 0 + } + {:role :value + :value "b" + :distance 1 + :depth 0 + }] + + ;; no, this doesn't work because we don't know when to "pop" + ;; from the inner vector + [:vector + {:distance 1} + [:index {:distance 1} 0] + [:value {:distance 1} "a"] + [:index {:distance 1} 1] + [:value {:distance 0} 1] + [:index {:distance 0} 2] + [:value {:distance 0} "b"] + ] + + ;; pros + ;; - easier to read?? + ;; - might match total format more uniformly (is that important?) + ;; - structure is more directly accessible, not buried inside 'value' key + ;; cons + ;; - 'role' and 'value' are implicit + ;; + + ;; maybe there are levels of stratified design? + ;; 1. one level is semantics of errors + ;; (2)?. another embedded layer is semantics of error values + ;; 3. lower layer is text (similar to fipp) + ;; (there is a layer to convert former to lower) + + ;; what if I just let people register their own + ;; pprint callback for records? still wouldn't help with + ;; array printing or all the rest. + + ;; if i let someone print out their own records, what if + ;; they have an array of records and only one is bad? they + ;; still need to know about distance and depth ;; highlights value within collection + + ;; hm, the difference between scalars (the type of data) + ;; and value (the role in the tree) isn't super clear (is (= - [[:span "1"] - [:span "^"]] + [ + [:span "[[\"a\" 1 \"b\" ]"] + ;;; hmm, the problem here is that the highlighting + ;; is not a normal indentation, it must track the next + ;; value above in a non-normal indentation way, so + ;; it's not clear we can return as data. + ;; we could of course just return a bunch of vectors with whitespace + ;; ahead of them but it's not clear what that gives us + [:span "^"] + ] (printer/value2 - [:span - {:role :collection - :distance 1 - :depth 0 - } + [:vector + {:distance 1} + [:index {:distance 1} 0] + [:value {:distance 1} "a"] + [:index {:distance 1} 1] + [:value {:distance 0} 1] + [:index {:distance 0} 2] + [:value {:distance 0} "b"] ] ) )) + + )