NS Deceleration - Require Vector Alignment

[athomasoriginal]

Hey kkinnear! Thank you for your amazing work with zprint. Big Fan!

I was looking into the :how-to-ns configuration and wanted to know if it's possible to take ns declerations which look like this:

(ns ^:figwheel-hooks pillar.app
  (:require
    [clojure.spec.alpha :as s]
    [clojure.string :as string]
    [some.other.ns :refer [magic]))

and have zprint justify (hope I used this terminology right) them so that the first keyword in the vectors would align. For example :as or :refer

(ns ^:figwheel-hooks pillar.app
  (:require
    [clojure.spec.alpha :as s]
    [clojure.string     :as string]
    [some.other.ns      :refer [magic]))

I can totally understand if this isn't a thing. The next best would be to tell zprint to respect hand-formatted vectors when nested in a :require or :import etc.

Thanks again!

[kkinnear]

Thanks for the positive feedback, it really makes a difference!

Today zprint only knows how to do justification on pairs, like the pairs in a cond or a let binding. Those things are processed together. The vectors in the :require are, as far as zprint knows, totally independent. It would be a major change to somehow justify the first elements of what appear to be independent vectors, because the contents of a later vector would change an earlier vector's spacing, possibly affecting its ability to fit on one line. I'm not saying it couldn't be done, just that it would require some significant architectural inspiration to do it. In other words, I don't even know quite where I'd start if I were to try to implement that. Not that I won't think about it, because it is an interesting problem.

That brings us to the second possibility, that zprint might be taught to keep its hands off the spaces in some cases. You might think that this would be no big deal, but at present zprint essentially ignores all spaces. It has learned how to respect newlines, but spaces are just left out when doing the formatting. There are a couple of places where things on different lines are checked for where they were when they came in, but that is outside of the normal formatting process. All of which is to say that this is the first time someone has asked for a feature that would "respect-spaces" while doing the formatting.

That one, too, would be an architectural change, but (probably) of a more local nature. At least I have some idea where to start prototyping something like that. I have to finish the 1.1.0 release, which is a few weeks off, and then I'll see what I can come up with for respecting spaces in vectors.

[athomasoriginal]

Thanks for the great response, @kkinnear!

I figured the original ask was kinda pushing the current system and appreciate the consideration of the "hands off" approach!

Looking forward to the next release!!

[kkinnear]

I'm thinking about ways to actually implement what you want, not just respect the spaces you put in. Not sure I'll be able to do that, but I'm giving it some serious consideration. I don't know what you really expect if the :refer is long. Here are two possibilities if the :refer is long (both of these are formatted for 80 columns):

; Possibility A

(ns zprint.core
  (:require
    [zprint.zprint :as zprint :refer
     [fzprint line-count max-width line-widths expand-tabs zcolor-map
      determine-ending-split-lines]]
    [zprint.zutil  :refer
     [zmap-all zcomment? edn* whitespace? string find-root-and-path-nw]]
    [zprint.finish :refer
     [cvec-to-style-vec compress-style no-style-map color-comp-vec
      handle-lines]]))

; Possibility B

(ns zprint.core
  (:require
    [zprint.zprint :as zprint :refer [fzprint line-count max-width line-widths
                                      expand-tabs zcolor-map
                                      determine-ending-split-lines]]
    [zprint.zutil  :refer [zmap-all zcomment? edn* whitespace? string
                           find-root-and-path-nw]]
    [zprint.finish :refer [cvec-to-style-vec compress-style no-style-map
                           color-comp-vec handle-lines]]))

I would guess that you might want possibility B, but I thought I'd check. Unless you have some other way to handle that, which I haven't happened to have stumbled onto yet -- in which case I'd love to see what you have in mind.

Thanks!

[kkinnear]

Two other possibilities for how to handle the :as:

; Possibility C

(ns zprint.core
  (:require
    [zprint.zprint :refer [fzprint line-count max-width line-widths
                           expand-tabs zcolor-map
                           determine-ending-split-lines]
                   :as zprint]
    [zprint.zutil  :refer [zmap-all zcomment? edn* whitespace? string
                           find-root-and-path-nw]]
    [zprint.finish :refer [cvec-to-style-vec compress-style no-style-map
                           color-comp-vec handle-lines]]))

; Possibility D

(ns zprint.core
  (:require
    [zprint.zprint :as :zprint
                   :refer [fzprint line-count max-width line-widths
                           expand-tabs zcolor-map
                           determine-ending-split-lines]]
    [zprint.zutil  :refer [zmap-all zcomment? edn* whitespace? string
                           find-root-and-path-nw]]
    [zprint.finish :refer [cvec-to-style-vec compress-style no-style-map
                           color-comp-vec handle-lines]]))

I think I like possibility D the best, but I don't think zprint would decide between C or D -- that would depend on the order that you put the information in the vector.

[kkinnear]

Interestingly, I can get zprint to do the following today:

(czprint nsc {:parse-string? true :vector {:option-fn (fn [opts n exprs] (if-not (clojure.string/includes? (str (first exprs)) ".") {:vector {:fn-format nil}} {:vector {:fn-format :none} :vector-fn {:constant-pair-min 1}}))}})
(ns zprint.core
  (:require
    [zprint.zprint :as :zprint
                   :refer [fzprint line-count max-width line-widths expand-tabs
                           zcolor-map determine-ending-split-lines]]
    [zprint.zutil :refer [zmap-all zcomment? edn* whitespace? string
                          find-root-and-path-nw]]
    [zprint.finish :refer [cvec-to-style-vec compress-style no-style-map
                           color-comp-vec handle-lines]]))

It doesn't do the justify thing, but it does make it more like what I think you had in mind. I kind of like it.

If you are interested is using this, I could configure something better than the hack I did just to prototype it, so let me know. Of course, if you don't have any long :refer vectors, then it isn't going to do much for you.

[athomasoriginal]

...if you don't have any long :refer vectors, then it isn't going to do much for you.

Interesting. I hadn't considered the case of :refer because I don't use them in my namespace requires. But if I pretend like I did, because you are developing a tool, I would be happy with either B, C, or D.

So, the only thing for me is the justification.

Thanks for running through the options!

[kkinnear]

In the just released zprint 1.1.2, there is a new style: {:style :require-justify} which does what I think you want.

For example:

zprint.core=> (czprint i166 {:parse-string? true :style :require-justify})
(ns ^:figwheel-hooks pillar.app
  (:require
    [clojure.spec.alpha :as s]
    [clojure.string     :as string]
    [some.other.ns      :refer [magic]]))

I have implemented an entirely new internal extension architecture to allow me to implement things like this in a straightforward manner, and the :require-justify style was the second thing I implemented using this new architecture.

I like the output format so much that I have used it for the zprint source in several files. It cleans up the (ns (:require ...)) so much, I really like it.

I hope that this is what you were looking for!

[athomasoriginal]

Wow! This is awesome!! The :require works perfectly. A few questions:

• Would this work for :require-macro, :import etc? or do I need to setup config a little more?

• There is an edge case specific to cljs that might/might not be working?

  ;; input
  (:require
  [clojure.spec.alpha :as s]
  [clojure.string    :as string]
  [some.other.ns    :refer [magic]]
  ["@firebase/app$default" :as firebase])
  
  ;; output
  (:require
  [clojure.spec.alpha    :as s]
  [clojure.string        :as string]
  [some.other.ns         :refer [magic]]
  ["@firebase/app$default" :as firebase]
  ["@firebase/app$default-and-otherstuffs" :as firebase-other])

  would this be expected? 🤔

This is amazing work though and it's great to see you're also making use of it!!!

[UPDATE May 04, 2020] I messed up the code example for input where I accidentally pasted the wrong original input. The input originally looked like this:

(:require
    [clojure.spec.alpha    :as s]
    [clojure.string :as string]
    [some.other.ns  :refer [magic]]
    ["@firebase/app$default" :as firebase]
    ["@firebase/app$default-and-otherstuffs" :as firebase-other])

[kkinnear]

To answer the easy question -- :require-justify works only for the :require inside an ns macro. It checks explicitly for :require. I don't see any reason why it couldn't easily be made to work for :require-macros. :import would be a bit more interesting, as at a quick look, my imports are in lists, not vectors. I don't know off the top of my head if that is necessary or just something I picked up somewhere along the way. I will look into it.

If you want to send me some :require-macros examples and :import examples (or examples with both), I'll see what I can do to extend the :require-justify formatting to these other ns clauses.

The hard question is that it looks like you are telling me that when you used :require-justify in Clojurescript, the output had things in it that weren't in the input? That is ... unprecedented, certainly. I'm aghast that something like this could happen, and have zero idea how something like this: ["@firebase/app$default-and-otherstuffs" :as firebase-other] could possibly have come about by a bug in zprint. zprint doesn't have default-and-otherstuffs laying around that it could, though some bug in zprint, insert into your code by mistake.

I cannot reproduce this with the information I presently have:

zprint.core=> (print y)
(ns (:require
 [clojure.spec.alpha :as s]
 [clojure.string    :as string]
 [some.other.ns    :refer [magic]]
 ["@firebase/app$default" :as firebase]))nil
zprint.core=> (czprint y {:parse-string? true :style :require-justify})
(ns (:require
      [clojure.spec.alpha    :as s]
      [clojure.string        :as string]
      [some.other.ns         :refer [magic]]
      ["@firebase/app$default" :as firebase]))

Please help me out and give me as much information as you can about your environment, so that I can try to reproduce this problem. This is a very egregious error, but I will have a hard time fixing it if I can't reproduce it.

Thanks again, and in advance.

[athomasoriginal]

RE: :import in vectors

This is a good point and it sounds like there has been debate.

RE: "@firebase/app$default-and-otherstuffs"

Sorry! I see how that example is confusing. This isn't a bug. I messed up my original example and I also corrected it. The "@firebase/app$default-and-otherstuffs" was meant to illustrate that when the first item in the vector is a long string the justification doesn't appear to work. What I meant to highlight is that the :refer and the as don't appear to justify when the first element in the vector is a string.

[some.other.ns         :refer [magic]]
["@firebase/app$default" :as firebase]

So there should be no worries! Sorry about the scare!!

RE: examples

Here are a bunch of examples I sourced from open source/my own work. Note that I don't actually use :use but thought i would throw it in for reference.

;; CLJS Example
(ns my.awesome.app
  (:require 
    [example.library :as library]
    ["@vimeo/player$default" :as vimeo]
    ["@f/app$default" :as firebase])
  (:require-macros 
    [cljs.analyzer.macros :refer [allowing-redef disallowing-ns* disallowing-recur]]
    [cljs.env.macros :as env]
    [devcards.core :as dc :refer [defcard defcard-doc deftest dom-node defcard-om-next]])
  (:import
    [java.io File]
    [java.util HashMap ArrayList]
    [org.apache.storm.task OutputCollector IBolt TopologyContext]         
    [goog.net XhrIo])
  (:use
    [backtype.storm cluster util thrift config log]))

;; CLJ Example
(ns my.clj.appj
  (:require 
    [example.library :as library])
  (:import
    [java.io File]
    [java.util HashMap ArrayList]
    [org.apache.storm.task OutputCollector IBolt TopologyContext]         
    [goog.net XhrIo]
    [org.apache.storm.generated JavaObject Grouping StormTopology
            StormTopology$_Fields Bolt Nimbus$Client
            ComponentCommon Grouping$_Fields SpoutSpec NullStruct StreamInfo
            GlobalStreamId ComponentObject ComponentObject$_Fields])

  (:use
    [backtype.storm cluster util thrift config log]))

Please let me know if you need more examples!

[kkinnear]

I apologize for leaving this hanging for so long. I somehow missed your reply, and didn't pick up on it until I was working through a review of the open issues.

You are so right about strings not justifying right. I thought that was because of the new justification strategy where up to two outliers don't justify and everything else will. But no, it is entirely because the count of the characters in a string ignores the double quotes used when formatting the output. Now the justification routines handle this case correctly.

I'm working on your other requests -- for :require-macros and :import. The first is straightforward, while :import requires some work.

Along the way, I noticed something that I would appreciate your feedback on (if you still care).

Here is one of your examples:

(ns my.awesome.app
  (:require
    [example.library         :as library]
    ["@vimeo/player$default" :as vimeo]
    ["@f/app$default"        :as firebase])
  (:require-macros
    [cljs.analyzer.macros :refer [allowing-redef disallowing-ns*
                                  disallowing-recur]]
    [cljs.env.macros      :as env]
    [devcards.core        :as    dc
                          :refer [defcard defcard-doc deftest dom-node
                                  defcard-om-next]])
  (:import [java.io File]
           [java.util HashMap ArrayList]
           [org.apache.storm.task OutputCollector IBolt TopologyContext]
           [goog.net XhrIo])
  (:use [backtype.storm cluster util thrift config log]))

My question is -- would you like the the env in [cljs.env.macros :as env] to be aligned with the dc on the line below it, like this:

(ns my.awesome.app
  (:require
    [example.library         :as library]
    ["@vimeo/player$default" :as vimeo]
    ["@f/app$default"        :as firebase])
  (:require-macros
    [cljs.analyzer.macros :refer [allowing-redef disallowing-ns*
                                  disallowing-recur]]
    [cljs.env.macros      :as    env]
    [devcards.core        :as    dc
                          :refer [defcard defcard-doc deftest dom-node
                                  defcard-om-next]])
  (:import [java.io File]
           [java.util HashMap ArrayList]
           [org.apache.storm.task OutputCollector IBolt TopologyContext]
           [goog.net XhrIo])
  (:use [backtype.storm cluster util thrift config log]))

I think I could do that, but was sort of mixed on which I would prefer. Since all of this was largely for you, I thought I'd see what you would prefer.

I will be working on the :import next.

Thanks for any suggestions here. Again, I'm sorry I let this languish for so long. All of this will be included in the upcoming 1.2.0 release.

[athomasoriginal]

No problem, @kkinnear! I'm happy that this is being entertained and truly appreciate your efforts!!

RE: Align the env

In my code I don't align the env with the dc. So, for me I am good without it. I could see that being desirable for some though. If it makes it easier, I would say it can be skipped.

Again, thank you for all the hard work!!

[kkinnear]

Easier sounds good, since I'm trying hard to get 1.2.0 out. Thanks very much for your quick response!

[kkinnear]

So, in 1.2.0, just released, I have added justification for :require-macros and :import, and make the justification for those and :require individually configurable. They are all individually accessible and can be used together through the new style: :ns-justify.

This is from the CHANGELOG:

New styles: :require-macros-justify and :import-justify, as well as :ns-justify that includes both of them together with the existing :require-justify. Each of the -justify styles has an associated style: :rj-var, :rjm-var, and :ij-var which allows you to change the :max-variance for justification separately for each by redefining them as changes to the :style-map.

I hope that these satisfy the needs that you were expressing. Give them a try, and let me know what you think!

[kkinnear]

I continue to use this myself. I like the way it looks, though the current implementation is not optimal for every ns macro.