Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config option to inline short threading macros in function bodies #335

Open
chrischambers opened this issue Oct 26, 2024 · 8 comments
Open

Comments

@chrischambers
Copy link

chrischambers commented Oct 26, 2024

I'd like short threading macros in function bodies to occur on a single line - is there a config option for this?

Preferred:

(defn level-order
  [bst]
  (->> bst rest (remove leaf-node?)))

Actual:

(defn level-order
  [bst]
  (->> bst
       rest
       (remove leaf-node?)))

I'm currently using the community style settings with :justified and :respect-nl.

As an aside, thanks for the great software! :)

@kkinnear
Copy link
Owner

kkinnear commented Nov 2, 2024

Yes, there are a variety of ways to achieve what you want. The simplest is to simply remove the special processing for ->> and then that threading macro will always appear on a single line unless it won't fit, and then it will move to multiple lines (which is the normal behavior for everything). For example:

(czprint i335b {:parse-string? true})
(defn level-order
  [bst]
  (->> bst
       rest
       stuff
       bother
       (remove leaf-node?)))


(czprint i335b {:parse-string? true :fn-map {"->>" :none-body}})
(defn level-order [bst] (->> bst rest stuff bother (remove leaf-node?)))

; Narrow the width to simulate a more complex ->> macro

(czprint i335b {:parse-string? true :fn-map {"->>" :none-body} :width 40})
(defn level-order
  [bst]
  (->> bst
       rest
       stuff
       bother
       (remove leaf-node?)))

That is the simplest approach. There are function types which will recognize a function with greater than 2 or greater than 3 arguments, and place those on multiple lines even if they would fit on one line. If you want to pursue that approach for the ->> macro, I can show you how to do that. It is a little more complex, because those function types are currently only specified for "argument" type functions/macros. We can work around that, but it is a bit more complex and I only want to get into that if you actually want to use the feature that recognizes the number of arguments (or body elements) that are in the function invocation. It isn't super complex, but we will have to mess with the :indent and then restore it. Let me know if you want to have the ->> macro on multiple lines if it has >2 or >3 arguments, and we can do that.

Thanks for asking!

@marksto
Copy link

marksto commented Jan 9, 2025

Hi @kkinnear! Me again. 😅 I had the same issue, but with the thread-first macro.

This one is super irritating because it's not just a matter of style/readability. See, there is also a hidden performance penalty when using get-in where a simpler-shorter -> does the job. For instance:

(def test-map {:outer-key {:middle-key {:inner-key "here's your value"}
                           :another-middle-key "some other cool value"}})
=> #'user/test-map
(require '[criterium.core :refer [quick-bench]])
=> nil
(quick-bench (get-in test-map [:outer-key :middle-key :inner-key]))
Evaluation count : 8769276 in 6 samples of 1461546 calls.
             Execution time mean : 73,348452 ns
    Execution time std-deviation : 12,269401 ns
   Execution time lower quantile : 62,984193 ns ( 2,5%)
   Execution time upper quantile : 87,916668 ns (97,5%)
                   Overhead used : 8,081371 ns
=> nil
(quick-bench (-> test-map :outer-key :middle-key :inner-key))
Evaluation count : 22203666 in 6 samples of 3700611 calls.
             Execution time mean : 19,603959 ns
    Execution time std-deviation : 0,622519 ns
   Execution time lower quantile : 18,789236 ns ( 2,5%)
   Execution time upper quantile : 20,309213 ns (97,5%)
                   Overhead used : 8,081371 ns
=> nil

Not much, admittedly, but this realization hurt me every time I have to rewrite -> to get-in to keep it a one-liner.


For me, adding a :fn-map entry for "->" with the default config with the :style :indent-only option added and then also restored did the trick.

The initial minimalistic zprint config could look like this:

{:width   120
 :style   [:justified
           :multi-lhs-hang
           :respect-nl]
 :list    {:wrap-after-multi? false
           :no-wrap-after     #{"&"}
           :indent-only-style :input-hang}}

The test ns — like that:

(def test-map
  {:outer-key {:middle-key         {:inner-key "here's your value"},
               :another-middle-key "some other cool value"}})

(defn test-fn-1
  []
  (let [value (-> test-map :outer-key :middle-key :inner-key)]
    (println value)
    value))

(defn test-fn-2
  []
  (-> test-map
      (assoc :another-outer-key {:middle-key "yet another value"})
      (doto (println))))

With this modified zprint config:

{:width  120
 :style  [:justified
          :multi-lhs-hang
          :respect-nl]
 :fn-map {"->" [:noarg1-body
                {:style              :indent-only
                 :list               {:constant-pair? false}
                 :next-inner-restore [[:style]
                                      [:list :constant-pair?]]}]}
 :list   {:wrap-after-multi? false
          :no-wrap-after     #{"&"}
          :indent-only-style :input-hang}}

Both test-fn-1 and test-fn-2 fns shall stay intact, and this doesn't seem to interfere with other things.


Quite surprisingly, removing a :noarg1-body value from the :fn-force-nl set, didn't work for me as expected.

Having the following modified zprint config:

{:width       120
 :style       [:justified
               :multi-lhs-hang
               :respect-nl]
 :fn-force-nl #{:arg2-force-nl :force-nl :flow :noarg1 :arg2-force-nl-body :force-nl-body :arg1-force-nl
                :flow-body :arg1-force-nl-body}
 :list        {:wrap-after-multi? false
               :no-wrap-after     #{"&"}
               :indent-only-style :input-hang}}

Results in new lines being added inside the test-fn-1's thread-first form:

(defn test-fn-1
  []
  (let [value (-> test-map
                  :outer-key
                  :middle-key
                  :inner-key)]
    (println value)
    value))

This is probably a matter of misplaced expectations or a real bug, I don't know. Maybe you can shed some light here.

@marksto
Copy link

marksto commented Jan 9, 2025

But then, on a real world project with a more complicated zprint config, I started running into some weird issues with this custom :fn-map entry for the "->".

Here's the complete zprint config I have:

{:width       120
 :style       [;; primary globals
               :justified
               :multi-lhs-hang
               :respect-nl
               ;; ns forms styles
               :require-pair
               :sort-require
               ;; extra fanciness
               :extend-nl
               :rod-no-ma-nl]
 ;; restoring some defaults and tuning up
 ;; to better match "Clojure Style Guide"
 :fn-map      {"->"               [:noarg1-body
                                   {:style              :indent-only
                                    :list               {:constant-pair? false}
                                    :next-inner-restore [[:style]
                                                         [:list :constant-pair?]]}]
               "defexpect"        "deftest"
               "defn*"            "defn"
               "expecting"        "testing"
               "more->"           :pair-fn
               "more-of"          :arg1-pair-body
               "with-cassette"    :arg2
               "with-cwd"         :arg1-body
               "with-datasource"  :arg1-body
               "with-transaction" :arg1-body}
 :fn-type-map {:arg1 [:hang {}]}
 :list        {:wrap-after-multi? false
               :no-wrap-after     #{"&"}
               :indent-only-style :input-hang}
 :vector      {:wrap-after-multi? false
               :no-wrap-after     #{"&"}}
 :set         {:wrap-after-multi? false}
 :map         {:indent  0
               :justify {:max-variance 60}
               :comma?  false}
 :binding     {:indent   0
               :justify? false}
 :pair        {:indent  0
               :justify {:max-variance 60}}
 :comment     {:wrap? false}}

And here are some of the issues I face with it.

A swap-vals! form inside a threading

Before (expected to stay intact):

(defn stop!
  []
  (mu/log ::stopping)
  (-> (swap-vals! *state
                  (fn [{:keys [system]}]
                    (when system
                      (ig/halt! system))
                    nil))
      (first)
      :config))

After the first call:

(defn stop!
  []
  (mu/log ::stopping)
  (-> (swap-vals! *state
                    (fn [{:keys [system]}]
                        (when system
                            (ig/halt! system))
                        nil))
      (first)
      :config))

After the second call:

(defn stop!
  []
  (mu/log ::stopping)
  (-> (swap-vals! *state
          (fn [{:keys [system]}]
                (when system
                    (ig/halt! system))
                nil))
      (first)
      :config))

After the third call:

(defn stop!
  []
  (mu/log ::stopping)
  (-> (swap-vals! *state
          (fn [{:keys [system]}]
              (when system
                  (ig/halt! system))
              nil))
      (first)
      :config))

After the fourth call:

(defn stop!
  []
  (mu/log ::stopping)
  (-> (swap-vals! *state
          (fn [{:keys [system]}]
                (when system
                    (ig/halt! system))
                nil))
      (first)
      :config))

And so on, indefinitely...

An assoc form inside a threading

Before (expected to stay intact):

(defn make-datasource
  [{:keys [jdbc-url user password cp-opts] :as _db-config}]
  (let [conn-pool (-> (merge default-cp-opts cp-opts)
                      (assoc :jdbc-url jdbc-url
                             :username user
                             :password password)
                      (cp/make-datasource))]
   ...))

After the first call:

(defn make-datasource
  [{:keys [jdbc-url user password cp-opts] :as _db-config}]
  (let [conn-pool (-> (merge default-cp-opts cp-opts)
                      (assoc :jdbc-url jdbc-url
                                          :username user
                                          :password password)
                      (cp/make-datasource))]
   ...))

Then it stays intact.

A deeply nested map inside a threading

Before (expected to stay intact):

(defn make-request! [config param-1 param-2]
  (try
    (-> "..."
        (client/post (merge base-request
                            {:form-params  (build-token-params config param-1 param-2)
                             :content-type :x-www-form-urlencoded}))
        :body)
    (catch Exception ex
      ...)))

After the first call:

(defn make-request! [config param-1 param-2]
  (try
    (-> "..."
        (client/post (merge base-request
                                    {:form-params (build-token-params config param-1 param-2)
                                         :content-type :x-www-form-urlencoded}))
        :body)
    (catch Exception ex
      ...)))

After the second call:

(defn make-request! [config param-1 param-2]
  (try
    (-> "..."
        (client/post (merge base-request
                               {:form-params (build-token-params config param-1 param-2)
                                    :content-type :x-www-form-urlencoded}))
        :body)
    (catch Exception ex
      ...)))

Then it stays intact.


Please, note that all these happen only when they occur inside a function body.

If I drop the custom :fn-map entry for the "->", all these places will stay intact.

I feel there is yet another subtle bug or a behavior that I cannot understand from the docs.
If it is the latter, it would be great to hear your thoughts on this matter, Kim.

@marksto
Copy link

marksto commented Jan 9, 2025

And one more thing. If we, say, go with an :indent option for a :list inside a thread-first macro instead of the :style :indent-only, things will do that strange arg-line break that, I believe, you've already discussed in this issue.

The modified zprint config diff:

 :fn-map      {"->" [:noarg1-body
                     {:list               {:indent         4
                                           :constant-pair? false}
                      :next-inner-restore [[:list :indent]
                                           [:list :constant-pair?]]}]}

The results in the aforementioned three cases:

;; A `swap-vals!` form inside a threading
(defn stop!
  []
  (mu/log ::stopping)
  (->
      (swap-vals! *state
                  (fn [{:keys [system]}]
                    (when system
                      (ig/halt! system))
                    nil))
      (first)
      :config))

;; An `assoc` form inside a threading
(defn make-datasource
  [{:keys [jdbc-url user password cp-opts] :as _db-config}]
  (let [conn-pool (->
                      (merge default-cp-opts cp-opts)
                      (assoc :jdbc-url jdbc-url
                             :username user
                             :password password)
                      (cp/make-datasource))]
   ...))

;; A deeply nested map inside a threading
(defn make-request! [config param-1 param-2]
  (try
    (->
        "..."
        (client/post (merge base-request
                            {:form-params  (build-token-params config param-1 param-2)
                             :content-type :x-www-form-urlencoded}))
        :body)
    (catch Exception ex
      ...)))

With no re-formatting upon a subsequent zprint call.

@marksto
Copy link

marksto commented Jan 9, 2025

@kkinnear The only workaround that I'm aware of is to use "guides" (:guide or :guided-body) for this. But, since these stay an internal, undocumented feature, there is no way for me to understand how they work and how to tune them up in each particular case. Maybe these "guides" should/could be documented someday? At least, their formal grammar.

@kkinnear
Copy link
Owner

There is a lot going on here, and I have not yet fully digested it. Let me respond first to the basic problem with ->.

What is going wrong with your approach using :style :indent-only in the :fn-map is that you cannot mix it with other styles. You can only do :style :indent-only at the top level options map. Never inside of any other options map. There are several reasons for this, and it is mentioned in Issue #134. I should probably document it more clearly in the docs as well. Hmm. I should probably make that an error, come to think of it. I would have to special case it, but it might be worth it.

In addition, while you get extra-credit points for using :next-inner-restore, :style isn't one of the things that you can restore that way. I have documented that somewhere, but don't have the time just now to look up where. To explain instead, when you use a :style the options map processes the :style values first, basically making the changes to the options map that the :style specifies. When you use :next-inner-restore it doesn't have anything to put back, because the style gets applied and there isn't a :no-indent-only style you could apply later (using :next-inner) to get rid of it. To use :next-inner-restore you would have to specify the specific details of what :indent-only did. But don't do that, as you shouldn't be using :indent-only in an options map not at the top level.

So, that approach (i.e., dropping into :indent-only when not at the top level options map) isn't going to work for you anytime soon (or, necessarily, ever).

What I think might work well is to simply drop the formatting for ->. Using your 'minimal config" (in my .zprintrc, so it doesn't show up) I tried this and it seems fine to me:

(czprint i335c {:parse-string? true})
(defn test-fn-1
  []
  (let [value (-> test-map
                  :outer-key
                  :middle-key
                  :inner-key)]
    (println value)
    value))

(czprint i335c {:parse-string? true :fn-map {"->" :none-body}})
(defn test-fn-1
  []
  (let [value (-> test-map :outer-key :middle-key :inner-key)]
    (println value)
    value))

(czprint i335d {:parse-string? true})
(defn test-fn-2
  []
  (-> test-map
      (assoc :another-outer-key {:middle-key "yet another value"})
      (doto (println))))

(czprint i335d {:parse-string? true :fn-map {"->" :none-body}})
(defn test-fn-2
  []
  (-> test-map
      (assoc :another-outer-key {:middle-key "yet another value"})
      (doto (println))))

I think that would solve your problem much more easily than anything else would. On the other hand, you are clearly a power user and so maybe you tried that and it didn't work for you. If so, let's pursue that approach and not mess with :indent-only.

Also, you said: "Quite surprisingly, removing a :noarg1-body value from the :fn-force-nl set, didn't work for me as expected."

Indeed it didn't, because while it looked as if you might have removed :noarg1-body from the set, you didn't actually do so. Specifying a set in an options map adds the things in the set in the configuration, it doesn't specify the actual set. Here is how to add and remove things from sets: https://cljdoc.org/d/zprint/zprint/1.2.9/doc/i-want-to-change-/anything-else-?q=sets#altering-the-configuration-of-sets-in-the-options-map.

So, that was a good idea, and it also works:

(czprint i335c {:parse-string? true})
(defn test-fn-1
  []
  (let [value (-> test-map
                  :outer-key
                  :middle-key
                  :inner-key)]
    (println value)
    value))

(czprint i335c {:parse-string? true :remove {:fn-force-nl #{:noarg1-body}}})
(defn test-fn-1
  []
  (let [value (-> test-map :outer-key :middle-key :inner-key)]
    (println value)
    value))

Either approach would be a good way to handle ->.

It is unfortunate that handling sets is kind of complicated, but I didn't know of any better way to allow people to add things and remove things from sets without (as you tried to do) specifying the entire set every time it was changed. That just didn't seem to be a reasonable approach.

That's what I have time for now. I'll look at the rest of this issue tomorrow. Thank you for your patience with all of the complexities in the configuration.

@marksto
Copy link

marksto commented Jan 13, 2025

Oh, right... You have reminded me of that "sets thing" (semantics) that I remember reading about in the docs when touching zprint for the first time. Will try it out then and let you know on the results, thank you!

@marksto
Copy link

marksto commented Jan 14, 2025

The :remove {:fn-force-nl #{:noarg1-body}} config option worked like a charm. I didn't notice any issues across a large sample codebase. Thank you, Kim @kkinnear!


With regards to the other proposed solution — leveraging the :fn-map {"->" :none-body} config option — this did indeed lead to strange behavior before, when i tried it the first few times. Although I can't provide any evidence right now. But, when I've modified my test ns like this:

(defn test-fn-1-a
  []
  (let [value (-> test-map :outer-key :middle-key :inner-key)]
    (println value)
    value))

;; new function!
(defn test-fn-1-b
  []
  (let [value (-> test-map :outer-key :middle-key :inner-key :outer-key :middle-key :inner-key :outer-key :middle-key :inner-key)]
    (println value)
    value))

(defn test-fn-2
  []
  (-> test-map
      (assoc :another-outer-key {:middle-key "yet another value"})
      (doto (println))))

it ended up being formatted like that:

(defn test-fn-1-a
  []
  (let [value (-> test-map :outer-key :middle-key :inner-key)]
    (println value)
    value))

;; new function!
(defn test-fn-1-b
  []
  (let [value (-> test-map
                  :outer-key
                  :middle-key :inner-key
                  :outer-key  :middle-key
                  :inner-key  :outer-key
                  :middle-key :inner-key)]
    (println value)
    value))

(defn test-fn-2
  []
  (-> test-map
      (assoc :another-outer-key {:middle-key "yet another value"})
      (doto (println))))

Which is definitely not what I would expect for test-fn-1-b. (The config is the same as the "complete" one above, apart from the "->" being formatted as :none-body.)

While the :remove {:fn-force-nl #{:noarg1-body}} results in the following, expected formatting:

...
(defn test-fn-1-b
  []
  (let [value (-> test-map
                  :outer-key
                  :middle-key
                  :inner-key
                  :outer-key
                  :middle-key
                  :inner-key
                  :outer-key
                  :middle-key
                  :inner-key)]
    (println value)
    value))
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants