Meta formatting

[arichiardi]

Hey Kim, after a looong time I finally got back to formatting and in particular I decided to open a new issue for the second part of #245.

I can finally answer to your comment.

I start with a big thank you - I am about to bump zprint to 1.2.8 and a lot of good stuff got merged.

This is how our deftest definitions currently look like:

(deftest ^{:database true ::test.hooks/system-init-keys system-keys}
         copy-diagnostic-report-test-base-case-destination-has-no-user-input
  (testing "foo))

In short:

• I definitely still like to have copy-diagnostic-report-test-base-case-destination-has-no-user-input on a new line (as in option B of that comment) as it is good for eye-catching against test failures. With "deftest" [:arg1-body {:style :meta-base}] that now happens.

  (deftest ^{:database true ::test.hooks/system-init-keys system-keys}
    copy-diagnostic-report-test-base-case-destination-has-no-user-input
  (...))

• If meta as map does not fit on one line I suppose the best for me is

  (deftest ^{:database true 
             ::test.hooks/system-init-keys system-keys}
    copy-diagnostic-report-test-base-case-destination-has-no-user-input
   ...

  As the map should have less visual impact. Same for non-map. So probably my preferred way from your comments way back would be:

  (def ^:const
       ^:private
       ^:test
       ^:lots
       ^:of
       ^:meta
       ^:stuff
    port-file-name
    (....))

To reiterate the important imho is the var symbol, not the meta.

As usual, my 2c - hopefully it makes sense :)

[kkinnear]

I have completely revamped my approach here. I built on some of the internal changes I made to fix the bug in #245 where I hadn't understood the parsing of multiple meta elements properly, but added a new guide for this processing. This gives me a lot more control over where things end up, at the cost of some architectural uplift to allow the option-fn that creates the guide to have access to more internal routines than previously.

Anyway, here is what you get. I've done the same examples three times with different conditions or parameters, even though some of the examples don't change with some of the conditions or parameters. Which is not necessarily a plus, though there are advantages to this approach. I've used all of the examples I had around from #245 and #312.

Many of these will look familiar. The thing you haven't see before, because I couldn't do it, is this, where individual metadata elements show up in the "hang" position and just keep filling in left-to-right until they run out. If you look at the :width 60 output for this one, you will see more of the metadata elements on the second line.

(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff ^:and ^:even ^:more
     ^:things
  port-file-name
  (....))

I managed to avoid this:

(def ^:const
     ^:private
     ^:test
     ^:lots
     ^:of
     ^:meta
     ^:stuff
     ^:and
     ^:even
     ^:more
     ^:things
  port-file-name
  (....))

which I think is pretty bad. But I could certainly get it back if you wanted it.

Without further ado:

First, with :width 80:

zprint.core=> (czprint i312 {:parse-string? true :style :meta-guide})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff port-file-name (....))

zprint.core=> (czprint i312a {:parse-string? true :style :meta-guide})
(deftest ^{:database true, ::test.hooks/system-init-keys system-keys}
  copy-diagnostic-report-test-base-case-destination-has-no-user-input
  (...))

zprint.core=> (czprint i312b {:parse-string? true :style :meta-guide})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff ^:and ^:even ^:more
     ^:things
  port-file-name
  (....))

zprint.core=> (czprint i312c {:parse-string? true :style :meta-guide})
(def port-file-name (....))

zprint.core=> (czprint i312d {:parse-string? true :style :meta-guide})
(def ^:private port-file-name (....))

zprint.core=> (czprint i245 {:parse-string? true :style :meta-guide})
(def ^:const ^:private port-file-name ".nrepl-port")

zprint.core=> (czprint i245a {:parse-string? true :style :meta-guide})
(def ^:const port-file-name ".nrepl-port")

zprint.core=> (czprint i245b {:parse-string? true :style :meta-guide})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff
  port-file-name
  ".nrepl-port")

zprint.core=> (czprint i245c {:parse-string? true :style :meta-guide})
(deftest ^:database-stuff-and-bother
  websocket-diagnostic-and-a-bit-more-that-does-not-fit
  (let [foo (bar "1")] foo))

Second with :width 60:

zprint.core=> (czprint i312 {:parse-string? true :style :meta-guide :width 60})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff
  port-file-name
  (....))

zprint.core=> (czprint i312a {:parse-string? true :style :meta-guide :width 60})
(deftest ^{:database true,
           ::test.hooks/system-init-keys system-keys}
  copy-diagnostic-report-test-base-case-destination-has-no-user-input
  (...))

zprint.core=> (czprint i312b {:parse-string? true :style :meta-guide :width 60})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff
     ^:and ^:even ^:more ^:things
  port-file-name
  (....))

zprint.core=> (czprint i312c {:parse-string? true :style :meta-guide :width 60})
(def port-file-name (....))

zprint.core=> (czprint i312d {:parse-string? true :style :meta-guide :width 60})
(def ^:private port-file-name (....))

zprint.core=> (czprint i245 {:parse-string? true :style :meta-guide :width 60})
(def ^:const ^:private port-file-name ".nrepl-port")

zprint.core=> (czprint i245a {:parse-string? true :style :meta-guide :width 60})
(def ^:const port-file-name ".nrepl-port")

zprint.core=> (czprint i245b {:parse-string? true :style :meta-guide :width 60})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff
  port-file-name
  ".nrepl-port")

zprint.core=> (czprint i245c {:parse-string? true :style :meta-guide :width 60})
(deftest ^:database-stuff-and-bother
  websocket-diagnostic-and-a-bit-more-that-does-not-fit
  (let [foo (bar "1")] foo))

Here are the same examples, with :width 80, but with :one-line-ok? false. Now, styles can accept parameters. I've created the style meta-guide to have one parameter, :one-line-ok? to say whether or not you want to allow it all on one line if it fits. The above examples have :one-line-ok? true, which is the default. But you can turn it off, which is what you get with the examples below:

zprint.core=> (czprint i312 {:parse-string? true :style {:style-call :meta-guide :one-line-ok? false}})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff
  port-file-name
  (....))

zprint.core=> (czprint i312a {:parse-string? true :style {:style-call :meta-guide :one-line-ok? false}})
(deftest ^{:database true, ::test.hooks/system-init-keys system-keys}
  copy-diagnostic-report-test-base-case-destination-has-no-user-input
  (...))

zprint.core=> (czprint i312b {:parse-string? true :style {:style-call :meta-guide :one-line-ok? false}})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff ^:and ^:even ^:more
     ^:things
  port-file-name
  (....))

; This one fits on one line since there is no metadata.  If there is no metadata, the guide does nothing.
zprint.core=> (czprint i312c {:parse-string? true :style {:style-call :meta-guide :one-line-ok? false}})
(def port-file-name (....))

zprint.core=> (czprint i245 {:parse-string? true :style {:style-call :meta-guide :one-line-ok? false}})
(def ^:const ^:private
  port-file-name
  ".nrepl-port")

zprint.core=> (czprint i245a {:parse-string? true :style {:style-call :meta-guide :one-line-ok? false}})
(def ^:const
  port-file-name
  ".nrepl-port")

zprint.core=> (czprint i245b {:parse-string? true :style {:style-call :meta-guide :one-line-ok? false}})
(def ^:const ^:private ^:test ^:lots ^:of ^:meta ^:stuff
  port-file-name
  ".nrepl-port")

zprint.core=> (czprint i245c {:parse-string? true :style {:style-call :meta-guide :one-line-ok? false}})
(deftest ^:database-stuff-and-bother
  websocket-diagnostic-and-a-bit-more-that-does-not-fit
  (let [foo (bar "1")] foo))

Please look these over. If there are any you think should look different, please show me how you would like them to look. Also, if you don't mind, tell me the thing you want changed (e.g., i312b or i245c or whatever)

Also, if you have specific examples you would like to see formatted with this code before I release it, this would be a great time to give me things to try. I'll format them and you can see what you will get and we can fix it if it isn't right.

Thanks!

[arichiardi]

Well, nothing to add to the changes I think - this pretty nice and will be useful for us, as usual - big thank you for the work you are putting in!

[kkinnear]

Great!

I'm thinking that instead of creating new style: :meta-guide, I will simply replace the existing :meta-alt style with this implementation. It does mostly the same things, but better (I hope). I doubt anyone was using it except you folks, but if they are, they will probably like it better too. If not, they can complain, and I'll put the existing :meta-alt back with a different name. :meta-alt was listed as experimental in any case. I will also remove the "experimental" label as well.

Now I will do the documentation and add some tests for this.

[kkinnear]

Well, I screwed up. I implemented this as :meta-guide (as you can see above), and after I wrote the above comment I got wildly distracted, and never actually did what I said I would do -- replace :meta-alt, write tests and documentation. In no small part because I never actually added this to my official release tracking document. Sigh. Despite not being in the CHANGELOG, there is a new style :meta-guide, which I think does exactly what you want as demonstrated above.

The missing pieces: While I did a bunch of development testing of it, I didn't create any actual committed tests for it. I also didn't get it into the documentation or the CHANGELOG. But it is there, and I worked hard on it, and believe that it works. And it seems you do too, since what is in 1.2.9 is what you approved, above. So the bad news is that you have to change the style you are using from :meta-alt to :meta-guide, but if you do that, it should do just what you want. I'll try to keep better track of things in the future.

Just to say it again -- in 1.2.9 if you use :style :meta-guide instead of :style :meta-alt, you should get the desired meta-data output formatting that you want. I will leave this as :meta-guide for the future.