letfn doesn't indent its fns like fns.

[tuh8888]

I couldn't find anything in a style guide about letfn, but I'm pretty sure it should indent each fn like a fn or defn, with 2 spaces instead of aligning to the argument vector like ->.

How it formats

(letfn [(first-fn [arg1 arg2]
                  (-> (doing-stuff)
                      (and-more-stuff)))
        (second-fn [arg1 arg2]
                   (-> (doing-stuff)
                       (and-more-stuff)))]
    (other-stuff))

How it should format

(letfn [(first-fn [arg1 arg2]
          (-> (doing-stuff)
              (and-more-stuff)))
        (second-fn [arg1 arg2]
           (-> (doing-stuff)
               (and-more-stuff)))]
    (other-stuff))

[kkinnear]

You make a good point. I think it should format the way that you suggest. Interestingly, it seems like you can't have a docstring in a letfn function, which makes my job easier.

You must be using :style :community, as without that it is even worse:

(letfn [(first-fn [arg1 arg2]
                  (-> (doing-stuff)
                      (and-more-stuff)))
          (second-fn [arg1 arg2]
                     (-> (doing-stuff)
                         (and-more-stuff)))]
  (other-stuff))

since letfn is considered a :binding function in the :fn-map, which it most certainly is not. Kind of looks like one, with the vector of things, but the vector doesn't contain a series of pairs of things, which is what :binding assumes.

So the current approach is wrong in several ways. I'll fix this. Which may require creating yet another function type, but I don't really know yet.

Thanks for creating an issue to get this fixed!

[kkinnear]

I have been exploring if there is a way to correctly format letfn using existing configuration tools. I believe that I have something that works, though the number of actual letfn examples I have to try it out on is very small.

Here is the options map that you would add to whatever you are using now:

{:fn-map
   {"letfn" [:arg1
             {:list {:hang-expand 1000},
              :next-inner {:fn-style :fn},
              :next-inner-restore [[:list :hang-expand]]}]}}

Using it results in this:

zprint.core=> (czprint i221 {:parse-string? true :fn-map {"letfn" [:arg1 {:list {:hang-expand 1000} :next-inner-restore [[:list :hang-expand]] :next-inner {:fn-style :fn}}]}})
(letfn [(first-fn [arg1 arg2]
          (-> (doing-stuff)
              (and-more-stuff)))
        (second-fn [arg1 arg2]
          (-> (doing-stuff)
              (and-more-stuff)))]
  (other-stuff))

As I said, I have very few examples of letfn to try this out on. Theoretically it should work, but the real world often upends theory. I would be interested in your experience with this, and if it does what you expect (and want) for letfn.

I would also be interested in any realistic examples of the use of letfn that you might be willing to share. Thanks!

[tuh8888]

Example 1

Looks mostly great to me. Here's my real example I noticed this problem on:

before:

(defn get-reactome-id
  [ontology iri meta-mop & {:keys [post-mapping]}]
  (letfn [(biopax-filler [id mop]
                         (->> id
                              (mt/get-ns :biopax)
                              mops/->id
                              (mops/filler-value mop)))
          (xref-db-id [db-name mop]
                      (->> mop
                           (biopax-filler :xref)
                           (map mt/->iri)
                           (map #(mt/-get-mop ontology % meta-roles))
                           (filter (comp (partial some #{db-name})
                                         (partial biopax-filler :db)))
                           (mapcat (partial biopax-filler :id))
                           first
                           keyword))
          (entity-db-id [db-name mop]
                        (->> mop
                             (biopax-filler :entityReference)
                             (map mt/->iri)
                             (map #(mt/-get-mop ontology % meta-roles))
                             (map (partial xref-db-id db-name))
                             first))]
    (let [[id type]
          ;; TODO Dispatch on type or otherwise more cleverly.
          (or (->
                ;; ENSEMBL Gene
                "ENSEMBL"
                (entity-db-id meta-mop)
                (mt/cond-pred-> (vector :gene)))
              (->
                ;; UniProt Protein
                "UniProt"
                (entity-db-id meta-mop)
                (mt/cond-pred-> (vector :protein)))
              (->
                ;; Most other reactome things: Pathways, Reactions,
                ;; PathwaySteps,
                ;; Complexes, etc.
                "Reactome"
                (xref-db-id meta-mop)
                (mt/cond-pred-> (vector :reactome)))
              (-> iri
                  mops/->id
                  (mt/cond-pred-> (vector :default))))]
      (if-let [f (get post-mapping type)]
        (f id iri meta-mop)
        id))))

after

(letfn [(biopax-filler [id mop]
            (->> id
                 (mt/get-ns :biopax)
                 mops/->id
                 (mops/filler-value mop)))
          (xref-db-id [db-name mop]
            (->> mop
                 (biopax-filler :xref)
                 (map mt/->iri)
                 (map #(mt/-get-mop ontology % meta-roles))
                 (filter (comp (partial some #{db-name})
                               (partial biopax-filler :db)))
                 (mapcat (partial biopax-filler :id))
                 first
                 keyword))
          (entity-db-id [db-name mop]
            (->> mop
                 (biopax-filler :entityReference)
                 (map mt/->iri)
                 (map #(mt/-get-mop ontology % meta-roles))
                 (map (partial xref-db-id db-name))
                 first))]
   (let
     [[id type]
      ;; TODO Dispatch on type or otherwise more cleverly.
      (or (->
            ;; ENSEMBL Gene
            "ENSEMBL"
            (entity-db-id meta-mop)
            (mt/cond-pred-> (vector :gene)))
          (->
            ;; UniProt Protein
            "UniProt"
            (entity-db-id meta-mop)
            (mt/cond-pred-> (vector :protein)))
          (->
            ;; Most other reactome things: Pathways, Reactions,
            ;; PathwaySteps,
            ;; Complexes, etc.
            "Reactome"
            (xref-db-id meta-mop)
            (mt/cond-pred-> (vector :reactome)))
          (-> iri
              mops/->id
              (mt/cond-pred-> (vector :default))))]
     (if-let [f (get post-mapping type)]
       (f id iri meta-mop)
       id)))

My inner let got a little messed up, but is fixed if the expression for the binding is refactored into the letfn like this:

(letfn [(biopax-filler [id mop]
            (->> id
                 (mt/get-ns :biopax)
                 mops/->id
                 (mops/filler-value mop)))
          (xref-db-id [db-name mop]
            (->> mop
                 (biopax-filler :xref)
                 (map mt/->iri)
                 (map #(mt/-get-mop ontology % meta-roles))
                 (filter (comp (partial some #{db-name})
                               (partial biopax-filler :db)))
                 (mapcat (partial biopax-filler :id))
                 first
                 keyword))
          (entity-db-id [db-name mop]
            (->> mop
                 (biopax-filler :entityReference)
                 (map mt/->iri)
                 (map #(mt/-get-mop ontology % meta-roles))
                 (map (partial xref-db-id db-name))
                 first))
          (ensembl-id [mop]
            ;; ENSEMBL Gene
            (-> "ENSEMBL"
                (entity-db-id mop)
                (mt/cond-pred-> (vector :gene))))
          (pr-id [mop]
            ;; UniProt Protein
            (-> "UniProt"
                (entity-db-id mop)
                (mt/cond-pred-> (vector :protein))))
          (reactome-id [mop]
            ;; Most other reactome things: Pathways, Reactions,
            ;; PathwaySteps,
            ;; Complexes, etc.
            (-> "Reactome"
                (xref-db-id mop)
                (mt/cond-pred-> (vector :reactome))))
          (default-id [_]
            (-> iri
                mops/->id
                (mt/cond-pred-> (vector :default))))]
   (let [[id type] ((some-fn ensembl-id pr-id reactome-id default-id) meta-mop)]
     (if-let [f (get post-mapping type)]
       (f id iri meta-mop)
       id)))

Example 2

However, I found another letfn example, this one that I don't want to refactor, where again the following let is misprinted:

Before

(deftest inheritance-fns-tests
  (letfn [(check-inheritance [m id expected]
                             (are [f] (= (get expected f) (count (f m id)))
                               sut/-abstr-roles sut/-spec-roles))]
    (let [expected1 {sut/-abstr-roles 5
                     sut/-spec-roles  0}
          expected2 {sut/-abstr-roles 5
                     sut/-spec-roles  1}
          expected3 {sut/-abstr-roles 3
                     sut/-spec-roles  2}
          mop1      (mops/->mop :Harrison
                                {:inst?    true
                                 :nickname "Harry"}
                                {:parents :Man})
          mop2      (mops/->mop :Man
                                nil
                                {:gender  "Male"
                                 :parents :Human})
          mop3      (mops/->mop :Human
                                nil
                                {:genus   "Homo"
                                 :species "Sapiens"
                                 :example "Harrison"})
          m         (-> (sut/make-mop-map)
                        (sut/-add-mop mop1)
                        (sut/-add-mop mop2)
                        (sut/-add-mop mop3)
                        sut/-infer-hierarchy)]
      (is (= 3 (count (mops/mop-ids m))))
      (is (check-inheritance m :Harrison expected1))
      (is (check-inheritance m :Man expected2))
      (is (check-inheritance m :Human expected3))
      (is (= {"Homo" {}} (sut/-inherit-filler m :Harrison :genus)))
      (is (= #{:Harrison} (set (sut/-find-instances m :Human))))
      (is (= #{:example} (set (sut/-filler-in m "Harrison")))))))

after

(deftest inheritance-fns-tests
  (letfn [(check-inheritance [m id expected]
            (are [f] (= (get expected f) (count (f m id)))
              sut/-abstr-roles sut/-spec-roles))]
   (let [expected1
         {sut/-abstr-roles 5
          sut/-spec-roles  0} expected2
         {sut/-abstr-roles 5
          sut/-spec-roles  1} expected3
         {sut/-abstr-roles 3
          sut/-spec-roles  2} mop1
         (mops/->mop :Harrison
                     {:inst?    true
                      :nickname "Harry"}
                     {:parents :Man}) mop2
         (mops/->mop :Man
                     nil
                     {:gender  "Male"
                      :parents :Human}) mop3
         (mops/->mop :Human
                     nil
                     {:genus   "Homo"
                      :species "Sapiens"
                      :example "Harrison"}) m
         (-> (sut/make-mop-map)
             (sut/-add-mop mop1)
             (sut/-add-mop mop2)
             (sut/-add-mop mop3)
             sut/-infer-hierarchy)]
     (is (= 3 (count (mops/mop-ids m))))
     (is (check-inheritance m :Harrison expected1))
     (is (check-inheritance m :Man expected2))
     (is (check-inheritance m :Human expected3))
     (is (= {"Homo" {}} (sut/-inherit-filler m :Harrison :genus)))
     (is (= #{:Harrison} (set (sut/-find-instances m :Human))))
     (is (= #{:example} (set (sut/-filler-in m "Harrison")))))))

my ~/.zprint.edn

```edn {:binding {:force-nl? true :justify? true} :fn-map {"are" :arg2-pair "cond" :pair-fn "count" :arg1 "svg/group" :arg1 ; TODO Move to project "with-dx" :binding ; TODO Move to project "letfn" [:arg1 {:list {:hang-expand 1000} :next-inner {:fn-style :fn} :next-inner-restore [[:list :hang-expand]]}] "s/cat" :pair-fn "s/fdef" :arg1-pair-body "s/fspec" :pair-fn "testing" :arg1-force-nl "update" :arg2 "with-precision" :arg1} :map {:comma? false :force-nl? true :justify? true :sort? true} :pair {:justify? true} :search-config? true :style :community :width 80} ```

[kkinnear]

Thank you so much! This is both unexpected and very helpful. What is going on is far from obvious, but I'll figure it out. Real life examples are so useful.

I also have to say that I'm impressed having looked at your .zprint.edn -- you are clearly a zprint power user! It pleases me to see that you managed to figure out how to tune zprint to your needs, that being the point of all of the many configuration options.

[tuh8888]

Glad they are helpful. And thanks! I've spent more time than I probably should have going over the reference and still discover new things. I only just discovered yesterday that we can define our own styles via :style-map and that they stack between .zprint.edns. I actually only included the part of my config I thought was relevant to you, but if you're interested, this is my (now) full config:

my full ~/.zprint.edn

```edn {:binding {:force-nl? true} :fn-map {"are" :arg2-pair "cond" :pair-fn "count" :arg1 "with-dx" :binding ; TODO Move to project "letfn" [:arg1 {:list {:hang-expand 1000} :next-inner {:fn-style :fn} :next-inner-restore [[:list :hang-expand]]}] "s/cat" :pair-fn "s/fdef" :arg1-pair-body "s/fspec" :pair-fn "testing" :arg1-force-nl "update" :arg2} :map {:comma? false :force-nl? true :sort? true} :pair {:force-nl? true} :search-config? true :style [:community :justified-original :keyword-respect-nl :require-justify] :style-map {:re-com {:vector {:option-fn (fn [opts n exprs] (when (>= n 2) (let [re-com? (let [current-fn (first exprs)] (and (symbol? current-fn) (#{"re-com" "svg-views"} (namespace current-fn)))) hiccup-with-style? (and (keyword? (first exprs)) (map? (second exprs)))] (cond re-com? {:pair {:force-nl? true :justify? true} :pair-fn {:hang? false} :vector {:fn-format :pair-fn}} hiccup-with-style? {:vector {:fn-format :arg1-force-nl}}))))}} :re-frame {:fn-map {"fn-traced" :fn "re-frame/->interceptor" :pair-fn "reg-event-db" :arg1-body "reg-event-fx" :arg1-body "reg-fx" :arg1-body "reg-sub" :arg1-pair-body "reg-sub-raw" :arg1-pair-body}} :web-app {:fn-map {"defstate" :arg1-pair-body "defstyles" :arg1-body "defsyntax" :fn} :style [:hiccup :re-com :re-frame]}} :width 80} ```

I'm really pleased with zprint, so thank you.

[kkinnear]

Wow, that is truly amazing. I don't know anyone who has written an :option-fn other than myself. Not that I would, I suppose, if they work, since I mostly only find out about things that do not work. But still, you have really figured it out! I'm really happy that you made this work, as I've tried hard to document things and give examples, without knowing how well I have really explained it.

I understand what is wrong with my current letfn solution, and am working on a different kind of solution. It will use something I implemented called "guides", so that I could do more complex style things that don't fit into the current code base. I have considered documenting them, but figured that probably nobody would ever use them besides me, so why bother. But seeing your zprint.edn has caused me to think that maybe somebody else beside me might use them after all. Anyway, that is a conversation for another time. I'll have a better letfn potential fix for you today or tomorrow.

[kkinnear]

The previous letfn approach failed because while it fixed the functions in the letfn vector, it forced a :fn-type on the let which was wrong. There was no way to force the :fn-type on the first thing in the letfn and not the second. Unless I used a :guide. Here is my next try at something for letfn:

{:fn-map {"letfn" [:guided
                   {:guide [:element :indent 2 :options
                            {:next-inner {:fn-style :fn}} :element-best
                            :options-reset :newline :element-best-*]}]}}

This uses a heretofore internal "guide" API. Typically I use an :option-fn to look at the code and produce a :guide based on the actual code, but sometimes that isn't necessary. Which I think is the case here, since letfn has a pretty fixed format.

Here is what you get for one of your examples (without your exact .zprint.edn, just what seemed useful to me for testing):

zprint.core=> (czprint i221e {:parse-string? true :fn-map {"letfn" [:guided {:guide [:element :indent 2 :options {:next-inner {:fn-style :fn}} :element-best :options-reset :newline :element-best-*]}]} :style [:justified :community] :map {:force-nl? true}})
(defn get-reactome-id
  [ontology iri meta-mop & {:keys [post-mapping]}]
  (letfn [(biopax-filler [id mop]
            (->> id
                 (mt/get-ns :biopax)
                 mops/->id
                 (mops/filler-value mop)))
          (xref-db-id [db-name mop]
            (->> mop
                 (biopax-filler :xref)
                 (map mt/->iri)
                 (map #(mt/-get-mop ontology % meta-roles))
                 (filter (comp (partial some #{db-name})
                               (partial biopax-filler :db)))
                 (mapcat (partial biopax-filler :id))
                 first
                 keyword))
          (entity-db-id [db-name mop]
            (->> mop
                 (biopax-filler :entityReference)
                 (map mt/->iri)
                 (map #(mt/-get-mop ontology % meta-roles))
                 (map (partial xref-db-id db-name))
                 first))]
    (let [[id type]
          ;; TODO Dispatch on type or otherwise more cleverly.
          (or (->
                ;; ENSEMBL Gene
                "ENSEMBL"
                (entity-db-id meta-mop)
                (mt/cond-pred-> (vector :gene)))
              (->
                ;; UniProt Protein
                "UniProt"
                (entity-db-id meta-mop)
                (mt/cond-pred-> (vector :protein)))
              (->
                ;; Most other reactome things: Pathways, Reactions,
                ;; PathwaySteps,
                ;; Complexes, etc.
                "Reactome"
                (xref-db-id meta-mop)
                (mt/cond-pred-> (vector :reactome)))
              (-> iri
                  mops/->id
                  (mt/cond-pred-> (vector :default))))]
      (if-let [f (get post-mapping type)]
        (f id iri meta-mop)
        id))))

The challenge comes with the :community style, where "body" functions and "argument" functions are different. As a Lisp person from way back, I find the distinction ... let's just say unhelpful. But it seems to be a thing, so I've done my best to try to accommodate that approach. It comes up here twice -- presumably letfn is a body function. But :guided doesn't know about that, so I had to hack an :indent 2 into the guide. Also, the functions in the letfn vector are themselves body functions, presumably, and they are working correctly as is.

I have detected an issue with the :fn function type -- it only handles single arity functions correctly. It doesn't deal correctly with multi-arity functions. For example, if I change one of your examples to have two multi-arity functions in the letfn you get:

(defn get-reactome-id
  [ontology iri meta-mop & {:keys [post-mapping]}]
  (letfn [(biopax-filler ([id mop]
                          (->> id
                               (mt/get-ns :biopax)
                               mops/->id
                               (mops/filler-value mop)))
                         ([id] (biopax-filler id nil)))
          (xref-db-id ([db-name mop]
                       (->> mop
                            (biopax-filler :xref)
                            (map mt/->iri)
                            (map #(mt/-get-mop ontology % meta-roles))
                            (filter (comp (partial some #{db-name})
                                          (partial biopax-filler :db)))
                            (mapcat (partial biopax-filler :id))
                            first
                            keyword))
                      ([db-name] (xref-db-id db-name nil)))
          (entity-db-id [db-name mop]
            (->> mop
                 (biopax-filler :entityReference)
                 (map mt/->iri)
                 (map #(mt/-get-mop ontology % meta-roles))
                 (map (partial xref-db-id db-name))
                 first))]
    (let [[id type]
          ;; TODO Dispatch on type or otherwise more cleverly.
          (or (->
                ;; ENSEMBL Gene
                "ENSEMBL"
                (entity-db-id meta-mop)
                (mt/cond-pred-> (vector :gene)))
              (->
                ;; UniProt Protein
                "UniProt"
                (entity-db-id meta-mop)
                (mt/cond-pred-> (vector :protein)))
              (->
                ;; Most other reactome things: Pathways, Reactions,
                ;; PathwaySteps,
                ;; Complexes, etc.
                "Reactome"
                (xref-db-id meta-mop)
                (mt/cond-pred-> (vector :reactome)))
              (-> iri
                  mops/->id
                  (mt/cond-pred-> (vector :default))))]
      (if-let [f (get post-mapping type)]
        (f id iri meta-mop)
        id))))

which isn't what I'd prefer. But that isn't your issue, so I can deal with that in the next release.

Let me know how the letfn guide works out for you. I think it should be close. At least your lets should still be :binding functions!

[tuh8888]

Finally got around to testing your :guided approach to letfn and it seems to work great! Thanks a bunch for figuring out a good solution.

[kkinnear]

This is fully operational now (i.e., fixed) in 1.2.3. Thanks for reporting the issue!