kkinnear / zprint

Executables, uberjar, and library to beautifully format Clojure and Clojurescript source code and s-expressions.
MIT License
555 stars 48 forks source link

`:binding {:justify? true}` is ignored when set style to `:indent-only` #196

Closed cloojure closed 2 years ago

cloojure commented 3 years ago

If I use style :respect-nl I can get nicely formatted let bindings:

(let [host      "athompson@cip001-bigcip.silage.lgscout.com"
      base      "/ReadyNAS4200/LGData/CIP_Data/bigcip/tpx_2_2"
      dirs-list "files-02.txt"
      out-root  "/Users/alanthompson/data"
      src-dirs  (get-lines dirs-list)
     ]
  ; (println :cnt (count src-dirs))
  ; (prn :src-dirs src-dirs)

  (doseq [dir src-dirs]
    (newline)
    (println "-----------------------------------------------------------------------------")
    (let [src      (format "%s:%s/%s" host base dir)
          dest-dir (format "%s/%s" out-root dir)
          >>       (dosh ["mkdir" "-p" dest-dir])
          cmd      ["rsync" "-av"
                    "--compress-level=4"
                    ; "--list-only"
                    src
                    dest-dir
                   ]
         ]
      (prn :dir dir)
      (newline)
      (prn :cmd cmd)
      (dosh cmd)
    ))

  (newline)
)

However, if I use style :indent-only the let bindings become ragged (1-space only):

(let [host "athompson@cip001-bigcip.silage.lgscout.com"
      base "/ReadyNAS4200/LGData/CIP_Data/bigcip/tpx_2_2"
      dirs-list "files-02.txt"
      out-root "/Users/alanthompson/data"
      src-dirs (get-lines dirs-list)
     ]
  ; (println :cnt (count src-dirs))
  ; (prn :src-dirs src-dirs)

  (doseq [dir src-dirs]
    (newline)
    (println "-----------------------------------------------------------------------------")
    (let [src (format "%s:%s/%s" host base dir)
          dest-dir (format "%s/%s" out-root dir)
          >> (dosh ["mkdir" "-p" dest-dir])
          cmd ["rsync" "-av"
               "--compress-level=4"
               ; "--list-only"
               src
               dest-dir
              ]
         ]
      (prn :dir dir)
      (newline)
      (prn :cmd cmd)
      (dosh cmd)

Here is my ~/.zprintrc.edn for reference:

{:width 125
 :style :indent-only ; :respect-nl 
 :map {:justify? true
       :comma? false }
 :binding {:justify? true}
 :pair {:justify? true}
 }
kkinnear commented 3 years ago

Thanks for asking! The way :indent-only works is that you, well, "only" get "indents". Style :indent-only doesn't recognize any functions as being any different than any others, so it doesn't know that let has pairs and therefore doesn't justify the pairs in the let. I tried to explain that in the "See Zprint"/"Indent Only" section, but it didn't come until toward the end.

As you have discovered, the style :respect-nl is very similar to :indent-only, except that zprint will recognize functions and shorten lines as necessary -- and do justification when asked. Both :respect-nl and :respect-bl offer the full zprint formatting capabilities, with varying treatments of newlines. On the other hand, :indent-only is very different in that it only indents, and doesn't do anything else. It has some smarts in the way that it indents the next line, but that is pretty much it.

Is there something that :indent-only would do for you that :respect-nl didn't offer? If you use :respect-nl and set the line width very wide, you will get the justification that you want but won't get any new newlines from your longer lines, so maybe that might be close to what you want?

cloojure commented 3 years ago

Hi - I am a big fan of Plumatic Schema for describing the shape of the args & result of a function. Unfortunately, zprint has a problem formatting this code.

What it should look like:

(s/defn add2 :- s/Num
  "Add 2 numbers"
  [x :- s/Num
   y :- s/Num]
  (+ x y))

What zprint produces:

(s/defn add2
  :-
  s/Num
  "Add 2 numbers"
  [x :- s/Num
   y :- s/Num]
  (+ x y))
kkinnear commented 3 years ago

This is something of an experiment, but it might work for you. This particular "solution" also requires that you use :respect-nl? true at least for vectors (which is built into it), to make the argument vectors work the way you want. This doesn't use or work with :indent-only.

Try this:

(def  i196 "(s/defn add2 :- s/Num\n  \"Add 2 numbers\"\n  [x :- s/Num\n   y :- s/Num]\n  (+ x y))\n")

zprint.core=> (czprint i196 {:parse-string? true :width 60 :fn-map {"s/defn" [:guided {:guide [:element :element :element-best :element-best :newline :element-newline-best-*] :vector {:respect-nl? true}}]}})

(s/defn add2 :- s/Num
  "Add 2 numbers"
  [x :- s/Num
   y :- s/Num]
  (+ x y))

This is what you put in your .zprintrc:

{:fn-map {"s/defn" [:guided
                    {:guide [:element :element :element-best :element-best
                             :newline :element-newline-best-*],
                     :vector {:respect-nl? true}}]}}

Basically, this just puts the first four things on the first line if they fit. If you don't have four things in an s/defn, it is going to do it anyway, so it isn't very ... adaptive, but if all of your s/defns have four things first, you will be fine. It would be possible for me to write a routine that would handle every possible case of the :- being there or not being there, but that's a good bit of work. It is also possible for me to write something that doesn't depend on :respect-nl? to get the argument vector right, but if this does the job for you, I'd just as soon skip that.

Let me know how it goes, if you would.

cloojure commented 3 years ago

No Joy. Orig code:

(s/defn verify-keys :- [s/Keyword]
  "Verifies that each entity has an identical keyset. Returns a sorted vector of keys."
  [entities :- [tsk/KeyMap]]
  (let [keyset (into (sorted-set) (keys (xfirst entities)))]
    (doseq [entity entities] (assert (= keyset (set (keys entity))) "All entities must have identical keys"))
    (vec keyset)))

after zprint:

(s/defn verify-keys
  :-
  [s/Keyword]
  "Verifies that each entity has an identical keyset. Returns a sorted vector of keys."
  [entities :- [tsk/KeyMap]]
  (let [keyset (into (sorted-set) (keys (xfirst entities)))]
    (doseq [entity entities] (assert (= keyset (set (keys entity))) "All entities must have identical keys"))
    (vec keyset)))

Here is the ~/.zprint.edn file

{
 :width 125
 :style :respect-nl  ; :indent-only
 :binding {:justify? true
           :hang? true }
 :map {:justify? true
       :comma? false 
       }
 :list {:hang? false
        }
 :pair {:justify? true}

 :fn-map {"s/defn" [:guided
                    {:guide [:element :element :element-best :element-best
                             :newline :element-newline-best-*],
                     :vector {:respect-nl? true}}]}
 }

It will fix the :- s/Str correctly if the :fn-map is the only item present in the zprint.edn file, but I need the other stuff too.

cloojure commented 3 years ago

A related issue. It inserts newlines far too often. For example, this orig code:

    (is (str/nonblank-lines= sample-csv
        (table->csv sample-edn)))

becomres

    (is
      (str/nonblank-lines=
        sample-csv
        (table->csv sample-edn)))
kkinnear commented 3 years ago

Thanks for including your .zprint.edn file, as that I would have had no idea what was going on without it.

Both of your issues are related to the {:list {:hang? false}} in your zprint.edn file. The first line of your s/defn is wrong because the two :element-best directives which tried to put the :- [s/Keyword] on the same line as the s/defn looked at the {:list {:hang? false}} and decided that putting those things on the next line was "best" because {:list {:hang? false}} was configured. You could fix that by changing the two :element-bests to :element, which fixes that problem. But something I thought of last evening is probably better, and certainly simpler. To solve the s/defn issue, this would probably be a better .zprint.edn:

{:width 125,
 :style :respect-nl,  ; :indent-only
 :binding {:hang? true, :justify? true},
 :map {:comma? false, :justify? true},
 :list {:hang? false},
 :pair {:justify? true},
 :fn-map {"s/defn" :wrap}}

Since you have :style :respect-nl, you can get by with a fn-type of :wrap for s/defn, and it will just put newlines where you have newlines. That should fix the s/defn issue.

That will not fix the "related issue" where you get newlines too often. That is happening because you put {:list {:hang? false}} in your .zprint.edn. That tells zprint to never put two things in a list on the same line unless it is some recognized function that has a function type where things go on the same line.

There are several ways around this. One approach would be to remove {:list {:hang? false}} from your .zprint.edn and see how it goes. If you know specifically why you added {:list {:hang? false}} to your .zprint.edn file, then give me some examples of what you got and what you wanted, and I'll see if I have a way to get that for you without using {:list {:hang? false}}.

cloojure commented 3 years ago

OK, this seems to be working much better. The .zprint.edn file

{
 :width 125
 :style :respect-nl
 :binding {:justify? true
           :hang? true }
 :map {:justify? true
       :comma? false 
       }
 :pair {:justify? true}
;:list {:hang? false } ; don't use this
 :fn-map {"s/defn" :wrap} ; special for Plumatic Schema 
 }

sample result:

(s/defn process-file :- tsk/KeyMap
  [fname] ; something slurpable
  (let [rep-blk         (json/json->edn (slurp fname))
        rep-blk-keys    (set (keys rep-blk))
        rep-blk-uuid    (uuid/v1)
        threat-dict-vec (grab :observable-dictionary-c-array rep-blk)
        threat-obs-vec  (grab :element-observable-c-array rep-blk)
        rep-out         (it-> rep-blk
                              (dissoc it
                                      :observable-dictionary-c-array
                                      :element-observable-c-array)
                              (glue it {:entity-uuid rep-blk-uuid}))
       ]
    (assert (= rep-blk-keys (set expected-top-keys)))
    (with-map-vals rep-blk
                   [source-observable-s list-name-s]
                   (assert (= "LookingGlass Reputation" source-observable-s))
                   (assert (= "LookingGlass Reputation" list-name-s))
                   rep-out
    )))

I used the :list {:hang? false} to avoid the "hang" of the 2 assert statements near the end. I want it to look like so:

    (with-map-vals rep-blk [source-observable-s list-name-s]
      (assert (= "LookingGlass Reputation" source-observable-s))
      (assert (= "LookingGlass Reputation" list-name-s))
      rep-out)

Any suggestions?

Thanks for the help.

kkinnear commented 3 years ago

I don't know if you want every (assert ...) to be special, or if with-map-vals is the place where you want three things on the first line, and the (assert ...) forms to flow below.

If it is the later, this .zprint.edn file:

{:width 125,
 :style :respect-nl,  ; :indent-only
 :binding {:hang? true, :justify? true},
 :map {:comma? false, :justify? true},
 :pair {:justify? true},
 :fn-map {"s/defn" :wrap "with-map-vals" :wrap}}

with this input:

zprint.core=> (print i196c)
(s/defn process-file :- tsk/KeyMap
  [fname] ; something slurpable
  (let [rep-blk         (json/json->edn (slurp fname))
        rep-blk-keys    (set (keys rep-blk))
        rep-blk-uuid    (uuid/v1)
        threat-dict-vec (grab :observable-dictionary-c-array rep-blk)
        threat-obs-vec  (grab :element-observable-c-array rep-blk)
        rep-out         (it-> rep-blk
                              (dissoc it
                                      :observable-dictionary-c-array
                                      :element-observable-c-array)
                              (glue it {:entity-uuid rep-blk-uuid}))
       ]
    (assert (= rep-blk-keys (set expected-top-keys)))
    (with-map-vals rep-blk [source-observable-s list-name-s]
                   (assert (= "LookingGlass Reputation" source-observable-s))
                   (assert (= "LookingGlass Reputation" list-name-s))
                   rep-out
    )))

will give this output:

zprint.core=> (czprint i196c {:parse-string? true})
(s/defn process-file :- tsk/KeyMap
  [fname] ; something slurpable
  (let [rep-blk         (json/json->edn (slurp fname))
        rep-blk-keys    (set (keys rep-blk))
        rep-blk-uuid    (uuid/v1)
        threat-dict-vec (grab :observable-dictionary-c-array rep-blk)
        threat-obs-vec  (grab :element-observable-c-array rep-blk)
        rep-out         (it-> rep-blk
                              (dissoc it
                                      :observable-dictionary-c-array
                                      :element-observable-c-array)
                              (glue it {:entity-uuid rep-blk-uuid}))
       ]
    (assert (= rep-blk-keys (set expected-top-keys)))
    (with-map-vals rep-blk [source-observable-s list-name-s]
      (assert (= "LookingGlass Reputation" source-observable-s))
      (assert (= "LookingGlass Reputation" list-name-s))
      rep-out
    )))

which, at least in this instance, is what I think you want.

If there are (assert ...) forms that are not working the way you would like in other places, I might still be able to come up with something but I'd need several examples of what you put in and want to get out to be able to generalize enough to potentially find a solution. Since you are using {:style :respect-nl}, there is a lot of flexibility to tailor the output since it depends so heavily on the input - once I know what you want the output to look like.

Thanks for asking!

kkinnear commented 2 years ago

I'm guessing that this either met your needs, or you don't care anymore. If I'm wrong, please re-open this issue.