Metadata on deftest (and def)

[arichiardi]

Hi @kkinnear,

As our use of zprint increases, we are slowly tackling and opening issues when we don't know how manage some of our forms (as you saw :smile:)

Thank you in advance for your work, we are going to try 1.2.2 soon here.

In the meantime, we have many places where we declare metadata on stuff:

(def ^{:doc "Json serde. This enables our default camel-case out kebab case in behaviour."}
     default-json-serde
  serde/json-camel-kebab-serde)

(deftest ^{:database true ::test.hooks/system-init-keys system-keys}
         websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")]
    foo))

The meta seems to break the arg1-body setup and I am not sure how to handle that. I could probably go :inner but I thought I should ask first.

[kkinnear]

Hello @arichiardi. Truly, thanks for asking. I sat down to try to figure out a fix for this problem, but ... I can't quite figure out the problem. When I try the two examples you gave, above, I get what you show (as long as my width is enough for the string in the first example). Presumably you would like these two examples to look different than they came out? Maybe it is obvious that there is a better way to format these and I'm just missing something? Perhaps you could give me your preferred output and I'll see what I can come up with to make that happen. Thanks!

[arichiardi]

Oh dear, sorry I did not specify what the desiderata is:

(def ^{:doc "Json serde. This enables our default camel-case out kebab case in behaviour."}
  default-json-serde
  serde/json-camel-kebab-serde)

and

(deftest ^{:database true ::test.hooks/system-init-keys system-keys}
  websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")]
    foo))

The funny thing is that some of the defs are actually behaving that way already, for example this one:

(def
  ^{:doc
    "Keys necessary for starting the application bus and the reactions.

Note that reactions will consume bus events so you won't be able to consume :chan directly."}
  ws-component-keys
  (into event-bus-component-keys
        [:appserver.lib.ws/endpoints ;; websocket config component (fed to pedestal)
         :appserver.lib.pubsub.components/pubsub ;; reaction component
         :appserver.lib.transport.components/transport-processor ;; websocket message processing
         :appserver.users/default-user :appserver.auth/params :appserver.uri.ws/scheme
         :appserver.uri/authority :appserver.uri.ws/path]))

[kkinnear]

First, let me say that this one is a real challenge. It turns out that metadata is parsed together with the thing to which the metadata is associated and they show up as a single element. So the usual ways I have of dealing with this don't really apply, since the metadata isn't two things when it is output from the parser (though it of course certainly looks like it is). So my usual "here is a hack in the options map to get you what you want" is not on the table.

That said, I might be able to get you close to what you want. Above, you said "some of the defs are actually behaving that way already, for example this one:"

(def
  ^{:doc
    "Keys necessary for starting the application bus and the reactions.

Note that reactions will consume bus events so you won't be able to consume :chan directly."}
  ws-component-keys
  (into event-bus-component-keys
        [:appserver.lib.ws/endpoints ;; websocket config component (fed to pedestal)
         :appserver.lib.pubsub.components/pubsub ;; reaction component
         :appserver.lib.transport.components/transport-processor ;; websocket message processing
         :appserver.users/default-user :appserver.auth/params :appserver.uri.ws/scheme
         :appserver.uri/authority :appserver.uri.ws/path]))

Except that one isn't, to my eyes anyway, actually behaving like you said you wanted it.

My understanding of what you want from your first two examples is to have the metadata on the same line as the def or deftest, and the symbol on the next line. Also, the symbol is not lined up in a "hang" under the metadata, but rather just normally indented. The example above is not like that, but rather everything is "flowed" under the def. I make this point for two reasons:

• which approach is really what you want?
• I can give you what is in the example above right now

It turns out to be rather easy to do either the normal :arg1-body behavior for def (or whatever) without metadata, and to do the "flow" for any def with metadata. Here are some examples:

First, the way you don't like:

zprint.core=> (czprint i224a {:parse-string? true})
(deftest ^{:database true, ::test.hooks/system-init-keys system-keys}
         websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

Next, I think this isn't your preferred approach, but it might be better than the one just above -- at least it is like the example you gave last:

zprint.core=> (czprint i224a {:parse-string? true :fn-map {"deftest" [:fn {:list {:option-fn (fn [opts n exprs] (if (meta (second exprs)) {:fn-style :flow-body} {:fn-style :arg1-body}))}}]} :style :community})
(deftest
  ^{:database true, ::test.hooks/system-init-keys system-keys}
  websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

This is the same options map as the previous example, but there is no metadata, so you can see the :arg1-body behavior:

zprint.core=> (czprint i224b {:parse-string? true :fn-map {"deftest" [:fn {:list {:option-fn (fn [opts n exprs] (if (meta (second exprs)) {:fn-style :flow-body} {:fn-style :arg1-body}))}}]} :style :community})
(deftest websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

That is the best I can do without actually changing the code and putting out a new release. Which I will be doing soon.

Here it is more readably:

(czprint
  i224a
  {:parse-string? true,
   :fn-map {"deftest" [:fn
                       {:list {:option-fn (fn [opts n exprs]
                                            (if (meta (second exprs))
                                              {:fn-style :flow-body}
                                              {:fn-style :arg1-body}))}}]},
   :style :community})

Obviously you will have to change the :fn-map for both def and deftest. And anything else that you want this behavior for. I have tested it minimally, but it does appear to work.

It would be nice to have you confirm which approach is really your favorite, since I'm going to try to actually allow that. I assume it is the first two examples you gave, and not like the third one, the one that I believe you mistakenly thought was already formatted like you wanted (and that the option-fn that I gave, above, produces). Is that true?

[arichiardi]

Hey @kkinnear, thanks yep it appears I mistakenly seen the third the same as the first two examples. For some reason I am always bringing you hot potatoes to handle :smile:

what you want from your first two examples is to have the metadata on the same line as the def or deftest, and the symbol on the next line. Also, the symbol is not lined up in a "hang" under the metadata, but rather just normally indented.

Yes that's what I would like to have, but I am ok with the workaround around deftest - the following for now

zprint.core=> (czprint i224a {:parse-string? true :fn-map {"deftest" [:fn {:list {:option-fn (fn [opts n exprs] (if (meta (second exprs)) {:fn-style :flow-body} {:fn-style :arg1-body}))}}]} :style :community})
(deftest
  ^{:database true, ::test.hooks/system-init-keys system-keys}
  websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

looks alright to me - I am not going to apply it to def though or I'll have to change too many places.

...one additional note would be that the meta should behave like a regular map if it gets big - ideally flowing like the third example, but I am sure you've already thought about that.

[kkinnear]

I'm working toward a real way to get you what you want. I have some questions. Which of these do you prefer if the metadata and the symbol would all fit on the same line?

; Version A -- this is now easy

(czprint i224c {:parse-string? true :meta {:split? true}})
(deftest ^{:database true}
  websocket-diagnostic-and-a-bit-more
  (let [foo (bar "1")] foo))

; Version B -- this is what you get today, which is easy, but you don't get version A, above, when
; it doesn't all fit on one line.  I can do this when it fits, and 
; do A when it doesn't if that is what you want.

(czprint i224c {:parse-string? true :meta {:split? false}})
(deftest ^{:database true} websocket-diagnostic-and-a-bit-more
  (let [foo (bar "1")] foo))

Also, regarding your comment about the meta map behaving like a regular map if it get big. It doesn't flow, it hangs the elements of the map:

(czprint "(deftest ^{:database true \n ::test.hooks/system-init-keys \n system-keys :another-map-key :another-map-value}\n\n         websocket-diagnostic-report-measurements-updated-event\n\n  (let [foo (bar \"1\")]\n    foo))\n" {:parse-string? true  :meta {:split? true} :dbg? false})
(deftest ^{:database true,
           ::test.hooks/system-init-keys system-keys,
           :another-map-key :another-map-value}
  websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

Does that work for you? I can get it to flow the map if it doesn't fit on one line, if you prefer that. Like this:

; Big map, doesn't fit on one line:

(czprint "(deftest ^{:database true \n ::test.hooks/system-init-keys \n system-keys :another-map-key :another-map-value}\n\n         websocket-diagnostic-report-measurements-updated-event\n\n  (let [foo (bar \"1\")]\n    foo))\n" {:parse-string? true  :meta {:split? true} :dbg? false :list {:hang-expand 0}})
(deftest
  ^{:database true,
    ::test.hooks/system-init-keys system-keys,
    :another-map-key :another-map-value}
  websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

; Small map, fits on one line

(czprint "(deftest ^{:database true \n ::test.hooks/system-init-keys \n system-keys}\n\n         websocket-diagnostic-report-measurements-updated-event\n\n  (let [foo (bar \"1\")]\n    foo))\n" {:parse-string? true  :meta {:split? true} :dbg? false :list {:hang-expand 0}})
(deftest ^{:database true, ::test.hooks/system-init-keys system-keys}
  websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

Is that better?

[arichiardi]

I'm working toward a real way to get you what you want.

Awesome!

; Version B -- this is what you get today, which is easy, but you don't get version A, above, when
; it doesn't all fit on one line.  I can do this when it fits, and 
; do A when it doesn't if that is what you want.

(czprint i224c {:parse-string? true :meta {:split? false}})
(deftest ^{:database true} websocket-diagnostic-and-a-bit-more
  (let [foo (bar "1")] foo))

I think version B is the most common and what most clojurians would expect (apologies if more work), cause most of the times you actually have the metadata map specified as ^:something.

For instance on def you'd have (def ^:private foo "bar") or on a simple kaocha deftest you'd have just a keyword like so:

(deftest ^:database websocket-diagnostic-and-a-bit-more
  (let [foo (bar "1")] foo))

; Big map, doesn't fit on one line:

(czprint "(deftest ^{:database true \n ::test.hooks/system-init-keys \n system-keys :another-map-key :another-map-value}\n\n         websocket-diagnostic-report-measurements-updated-event\n\n  (let [foo (bar \"1\")]\n    foo))\n" {:parse-string? true  :meta {:split? true} :dbg? false :list {:hang-expand 0}})
(deftest
  ^{:database true,
    ::test.hooks/system-init-keys system-keys,
    :another-map-key :another-map-value}
  websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

; Small map, fits on one line

(czprint "(deftest ^{:database true \n ::test.hooks/system-init-keys \n system-keys}\n\n         websocket-diagnostic-report-measurements-updated-event\n\n  (let [foo (bar \"1\")]\n    foo))\n" {:parse-string? true  :meta {:split? true} :dbg? false :list {:hang-expand 0}})
(deftest ^{:database true, ::test.hooks/system-init-keys system-keys}
  websocket-diagnostic-report-measurements-updated-event
  (let [foo (bar "1")] foo))

Is that better?

Yes, this second version is better imho - the previous leaves a lot of "empty space" and you have to move your eyes to the right to note the map. If the metadata map is big it is usually for an "important" reason (in my case system keys are customized) and you should have good perception of that.

I hope this makes sense and thanks for asking and working on this!

[kkinnear]

Sorry to keep asking, but I'm not having any luck guessing which you prefer.

What about these, where there isn't a map, but they don't fit on the same line?

; Version A is what I was thinking to always do if there wasn't a map in the metadata
; and it doesn't fit on one line (which is what it does today)

zprint.core=> (czprint i224e {:parse-string? true})

(deftest ^:database-stuff-and-bother
         websocket-diagnostic-and-a-bit-more-that-does-not-fit
  (let [foo (bar "1")] foo))

; Version B is, I suppose, an alternative

zprint.core=> (czprint i224e {:parse-string? true :meta {:split? true}})

(deftest ^:database-stuff-and-bother
  websocket-diagnostic-and-a-bit-more-that-does-not-fit
  (let [foo (bar "1")] foo))

[arichiardi]

Hey, no problem :smile: Version B is best imho

[kkinnear]

So, I've created a new :style, called :meta-alt in the latest release, 1.2.3. I have tried really hard to have it include all of the various things we've talked about in this issue. It is currently marked as "experimental", so I can continue to refine it if necessary. Why don't you give it a try, and see if it does what you want. Let me know how it comes out. Thanks!

[arichiardi]

Ok sounds good I'll try that and get back to you :smile:

[arichiardi]

Started comments on the release https://github.com/kkinnear/zprint/discussions/232#discussioncomment-3095766

[arichiardi]

Back to the actual topic of this issue, I tried to format these:

(deftest
  ^:database
  foo-test
  (testing "foo"
    (is (= :bar :bar))))

(deftest ^{:database true :doc "This is a long comment on the test, so long that is quite
 boring. Not boring in the sense that it is not really telling much but boring in the sense that it
 is very long to read."} comment-test
  (testing "comment"
    (is (= :bar :bar))))

(deftest ^{:database true ::test.hooks/system-init-keys [reporting-service-key ordering-system-key user-service-system-key]} data-test
  (testing "data"
    (is (= :bar :bar))))

with the attached .zprint.edn

and I am very thankful for the result :smile:

(deftest ^:database foo-test
  (testing "foo"
    (is (= :bar :bar))))

(deftest
  ^{:database true
    :doc
    "This is a long comment on the test, so long that is quite
 boring. Not boring in the sense that it is not really telling much but boring in the sense that it
 is very long to read."}
  comment-test
  (testing "comment"
    (is (= :bar :bar))))

(deftest
  ^{:database true
    ::test.hooks/system-init-keys [reporting-service-key ordering-system-key
                                   user-service-system-key]}
  big-data-test
  (testing "data"
    (is (= :bar :bar))))

There is an exception though, I think this one:

(deftest ^{:database true ::test.hooks/system-init-keys system-keys} small-data-test
  (testing "data"
    (is (= :bar :bar))))

Would be better as:

(deftest ^{:database true ::test.hooks/system-init-keys system-keys}
  small-data-test
  (testing "data"
    (is (= :bar :bar))))

There might be something in my config though, that's why I am attaching it this time.

[kkinnear]

Turns out that in (deftest ^:database foo-test ...) the ^:database parses as a map: {:database true}. So I can't just change things if I find a map in the metadata. So I changed it so that if there is more than one key in the map, it will format it on two lines, and if only one key, then one line (as long as it fits). See if this works for you...

Put this in your zprint.edn:

{:style-map
     {:meta-base
        {:doc "Alternative format for metadata. Experimental.",
         :list {:option-fn
                  (fn ([] "meta-base-option-fn")
                      ([opts n exprs]
                       (when (meta (second exprs))
                         {:meta {:split? true},
                          :list {:hang-expand 0},
                          :fn-style
                            (if (and (map? (meta (second exprs)))
                                     (> (count (keys (meta (second exprs)))) 1))
                              :arg1-body
                              :arg2),
                          :next-inner-restore [[:list :option-fn]
                                               [:list :hang-expand]]})))}},
      :meta-alt {:doc "Alternative for metadata. Experimental.",
                 :fn-map {"def" [:arg2 {:style :meta-base}],
                          "deftest" [:arg1-body {:style :meta-base}]}}}})

This should replace the current :meta-base and :meta-alt styles, and correctly format everything. This is what I plan to release next, so please let me know how it works.

Thanks!