Repository navigation
Parameterize color schemes #41
Description
Activity
Hi @alysbrooks ,
From what I know about
deep-diff2, the color is mostly controlled by here.
For example, whengreenis chosen for a string "hello", it outputs\u001b[32mhello\u001b[0m. Here, green color is replaced to color digit 32 through thesgr-codehashmap.How you do think about the following implementation?
- I implement a new namespace under color directory, like
eight-bits-color.clj - Similar to
ansi.clj, but support 256 colors. - Similar to
ansi.clj, but it accept the user to inject a new map to replacesgr-code.
- I implement a new namespace under color directory, like
I made some experiments. Actually, I can change the color to even 256 color without changing any single line of code.
For example, if I change the
:lambdaisland.deep-diff2.printer-impl/deletionfrom[:red]to[:bg-256 5 13], the color code it outputs will change from\u001b[31mto\u001b[48;5;13mScreen shot
user=> (use 'lambdaisland.deep-diff2) nil user=> (def color-printer (printer {:color-scheme {:lambdaisland.deep-diff2.printer-impl/deletion [:bg-256 5 13]}})) #'user/color-printer user=> (pretty-print (diff {:a 1} {:b 2}) color-printer) {+:b 2, -:a 1}Oh, after reading related issues more carefully, I find out that there are
ansi.cljcan controll more than I thought.So it seems that Puget already allows customization through a custom printer? In that case I think there's not much more that needs to happen here, and it's more a matter of exposing this configuration in Kaocha's
tests.edn.Note that not all terminals support full RGB colors, although it's relatively widely supported these days. The downside of RGB colors is that, because they are absolute, you basically have to also set the background, because you can't predict what contrast you'll get. Whereas with the basic 16(?) colors it's up to the terminal how they render that exactly, so the terminal's color scheme should ensure that there's a sensible amount of contrast.
Which kind of goes back to the original issue, since we use these basic built-in colors ("red", "green"), rather than fully specified RGB colors, it's really up to the terminal color scheme to make sure they render ok on the default background.
Reading through the code and this discussion, I think we should take the following steps:
- Document how to set up a custom printer
- Optionally, create a helper function to make it easier to set custom named colors or RGB colors.
lambdaisland.deep-diff2.printer-impl/puget-printerisn't that complicated, but there's quite a bit of set up that's orthogonal. It would be nice if you could do(deep-diff/custom-printer {:deleted :red :inserted [100 250 0]}), for example. - Expose configuration in Kaocha.
I'm not sure my idea for color schemes really makes sense for
deep-diffanymore. Perhaps a library or community extension would be more appropriate for that sort of thing. I think it would make sense in Kaocha (perhaps as a plugin). The one feature that might make sense in deep-diff2 is a fallback option, where we set the foreground colors and background colors to ensure good contrast so people can have something readable if their terminal color scheme isn't ideal or has gotten messed up somehow.I think @humorless and I could split these among ourselves, unless you have time/interest, @plexus?
Sounds good, you can go for it.
This is a nice compromise for #14, thanks @alysbrooks and @humorless! I just used the updated docs to change deletion and insertion highlighting which made it much easier for me to see diffs in a large data structure. ❤️
Reacted by Arne Brasseuruser=> (use 'lambdaisland.deep-diff2) nil user=> (def color-printer (printer {:color-scheme {:lambdaisland.deep-diff2.printer-impl/deletion [:bg-256 5 13]}})) #'user/color-printer user=> (pretty-print (diff {:a 1} {:b 2}) color-printer) {+:b 2, -:a 1}
This example from README unexpectedly resets all the other default
:color-schemevalues. So for example, insertions will not be highlighted anymore.The cause is in how
optsare merged with the defaults inlambdaisland.deep-diff2.printer-impl/puget-printer.
It's also surprising to propose using
lambdaisland.deep-diff2.printer-implas it's an internal namespace.Based on your points, I would propose to fix this like this:
- Logic Level: Use
update + merge
Instead of performing a top-level merge on the entire opts map, the function should specifically merge the inner :color-scheme map. This prevents the user's input from wiping out the default color keys.
(defn puget-printer ([] (puget-printer {})) ([opts] (let [extra-handlers (:extra-handlers opts) ;; 1. Define the default color scheme default-colors {::deletion [:red] ::insertion [:green] ::other [:yellow] :boolean [:bold :cyan] :tag [:magenta]} ;; 2. Merge user-provided colors into the defaults user-colors (:color-scheme opts) merged-colors (merge default-colors user-colors)] (puget-printer/pretty-printer (merge {:width (or *print-length* 100) :print-color true :color-scheme merged-colors ; Use the merged result :print-handlers (dispatch/chained-lookup (print-handler-resolver extra-handlers) puget-printer/common-handlers)} ;; Dissoc :color-scheme here so it doesn't overwrite the merged version (dissoc opts :extra-handlers :color-scheme))))))- Architecture Level: Simplify the API Keywords
To prevent users from having to reference
lambdaisland.deep-diff2.printer-impl/deletion, the library should provide a mapping layer. This allows users to pass simple keywords like:deletion, which are then translated to the internal namespaced keys.(defn- normalize-colors "Translates user-friendly keys to internal namespaced keys." [colors] (let [mapping {:deletion ::deletion :insertion ::insertion :other ::other}] (reduce-kv (fn [m k v] (assoc m (get mapping k k) v)) ;; Fallback to original key if not in mapping {} colors))) ;; Inside puget-printer: (let [user-colors (normalize-colors (:color-scheme opts)) merged-colors (merge default-colors user-colors)] ...)- Logic Level: Use
-
This looks like re-implementation of what the existing helper
lambdaisland.deep-diff2.puget.printer/merge-optionsdoes a bit later inpuget-printer/pretty-printer. So maybe everything can be simplified somehow, taking that into account 🤷 -
I'd avoid introducing "user-friendly" keys (as opposed to "internal" ones). Because additional terminology will make it harder to grasp the codebase.
Considering the only problem with
lambdaisland.deep-diff2.printer-impl/deletionis "impl" part, I'd simply rename these keys everywhere to:deletion(or maybe::lambdaisland.deep-diff2/deletionto avoid any potential clashes). An then if you want to keep backwards-compatibility (and avoid bumping library major version) then support for the old names will be needed, as in the proposed mapping function.
Reacted by Laurence Chen-
Currently, colors are hard-coded, but there are various situations where you might want to set different colors:
In addition to general usefulness, parameterizing color schemes may address #14 and would be required for lambdaisland/kaocha#403.