kkinnear / zprint

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

Custom Syntax Formatting: Hiccup #294

Closed athomasoriginal closed 1 year ago

athomasoriginal commented 1 year ago

My goal is to format my project's hiccup like this:

;; before
[:div {:a "a"} [:p "hello"]]

;; after
[:button 
  {:on-click (fn [] ...)}
  [:p "hello"]]

I've been able to achieve this with my own style-map config for hiccup (see below for the config). The issue I'm running into is that if I have a multi arity function, the arg signatures is mistaken for hiccup and they will be formatted as well. For example:

;; the original multi arity fn with formatting we like
(defn my-fn 
  ([a] (my-fn a {})
  ([a {:keys [b c]}]
   (do-stuff a b c))

;; the multi-arity fn when confused for hiccup formatting
(defn my-fn 
  ([a] (my-fn a {})
  ([a                 ;; <---mistaken for hiccup
     {:keys [b c]}]   ;; <---mistaken for hiccup
   (do-stuff a b c))

I'm curious if we have a way to get around this? 🤔

hiccup style-map config ```clojure ;; style-map config {:vector-fn {:indent-arg 2} :vector {:wrap? false :option-fn (fn ([] "hiccup-w-option-fn") ([opts n exprs] (let [hiccup? (and (>= n 2) (or (keyword? (first exprs)) (symbol? (first exprs))) (map? (second exprs)))] (cond (and hiccup? (not (:fn-format (:vector opts)))) {:vector {:fn-format :flow}} (and (not hiccup?) (:fn-format (:vector opts))) {:vector {:fn-format nil}} :else nil))))}} ```
NoahTheDuke commented 1 year ago

To make sure this is just a copy-paste error, is it supposed to be defn my-fn?

athomasoriginal commented 1 year ago

To make sure this is just a copy-paste error, is it supposed to be defn my-fn?

Yep! Thanks @NoahTheDuke 🙇 (Updated)

kkinnear commented 1 year ago

Thanks for asking. I am assuming that the def should have been defn, as @NoahTheDuke pointed out.

I think the problem you are encountering is because the second arity of the function has an argument vector that starts with a symbol and is followed by a map. The hiccup style is detecting that as a hiccup vector, since that appears to be a legal form of hiccup vector.

There are two ways that I can try to get the hiccup support to leave argument vectors alone. One way is positional, within a defn, and I'll work on that.

Another way is by finding a way to disambiguate hiccup vectors from argument vectors. I'm not a big hiccup user, so I don't have a lot of experience in that regard. If I look at the hiccup definition, it seems like there isn't any clear way to determine whether a vector is a hiccup vector or an argument vector. Since you have a lot more experience with hiccup than I do -- do you see a way to determine whether a vector is a hiccup vector or an argument vector simply by looking at the contents of the vector? If so, we can easily fix the option-fn to be more discerning about what it formats as hiccup.

If not, then we will have to fall back on something more complex where we look at the position of the vector within the overall defn, and ignore some vectors as hiccup vectors. Which I will work on figuring out how to make happen -- but it would be nice to have a simpler way to accomplish the goal.

Thanks!

kkinnear commented 1 year ago

I believe that I have answered my own question to you. I think one way to disambiguate an argument vector from a hiccup vector is to look at the map which must be the second thing in the vector (to even be considered a possible hiccup vector). If that map has any keys that are :keys, :strs, :syms, :as, :or, then it must be a destructuring map, not a hiccup map. I have a bit more testing to do, but I expect I will be able to offer you a better :option-fn soon, which will not require a different version of zprint to execute.

kkinnear commented 1 year ago

You might want to give this a try. I think it will do what you want. I tried it on some random hiccup I had, and it seems to do what you want for the formatting of the hiccup -- and doesn't mess up the argument vector.

{:vector {:option-fn (fn
                       ([] "hiccup-w-option-fn")
                       ([opts n exprs]
                        (let [hiccup? (and (>= n 2)
                                           (or (keyword? (first exprs))
                                               (symbol? (first exprs)))
                                           (map? (second exprs))
                                           (not (some #{:keys :syms :strs :as
                                                        :or}
                                                      (keys (second exprs)))))]
                          (cond (and hiccup? (not (:fn-format (:vector opts))))
                                  {:vector {:fn-format :flow}}
                                (and (not hiccup?) (:fn-format (:vector opts)))
                                  {:vector {:fn-format nil}}
                                :else nil)))),
          :wrap? false},
 :vector-fn {:indent-arg 2}}

Let me know how this works for you.

Thanks

athomasoriginal commented 1 year ago

Sorry it took a min to get back to this. It took a min to test all the edge cases and see how the above performs.

The config in https://github.com/kkinnear/zprint/issues/294#issuecomment-1494810359 almost works all the time. I landed on the following config.

Updated hiccup style-map config ```clojure ;; style-map config {:vector {:wrap? false :option-fn (fn ([] "hiccup-w-option-fn") ([opts n exprs] (let [arg-1 (first exprs) arg-2 (second exprs) invalid-prop-keys #{:keys :syms :strs :as :or} ;; @note second position can be a vector or a ;; map. When it's a vector, it should begin ;; with a keyword or symbol and it could also ;; just be a string hiccup? (and (>= n 2) (and (or (keyword? arg-1) (symbol? arg-1)) ;; @note specific to this ;; codebase, guards against ;; [_ {}] being mistaken for ;; hiccup ;; hiccup when it cannot be. This is found (not (= '_ arg-1))) (or (string? arg-2) ;; @note arg-2 could be a vector ;; (more hiccup) and so we want ;; to treat it as such (and (and (vector? arg-2) (>= (count arg-2) 2)) (or (keyword? (first arg-2)) (symbol? (first arg-2)))) (and (map? arg-2) (->> (keys arg-2) (some invalid-prop-keys) (not)))))] (cond (and hiccup? (not (:fn-format (:vector opts)))) {:vector {:fn-format :flow}} (and (not hiccup?) (:fn-format (:vector opts))) {:vector {:fn-format nil}} :else nil))))}} ```

My feeling is that this is fine until we get a better :option-fn.

For future reference, here is an example scenario we would ideally want it to work for but it doesn't quite

[hi
  {}
  [:p
    {}
    "hello"]]

;; what it does
[:div [hi]
 [:p
   "hello"]]

;; what we want
[:div 
   [hi]
   [:p
     "hello"]]
kkinnear commented 1 year ago

Nice option-fn! That said, I'm pretty confused here. It looks to me like your option-fn does what you want -- at least it does for me:

; I defined your option fn as a style called :i294d ...
(print i294ud)
{:style-map {:i294d
               {:vector
                  {:option-fn
                     (fn
                       ([] "hiccup-w-option-fn")
                       ([opts n exprs]
                        (let [arg-1 (first exprs)
                              arg-2 (second exprs)
                              invalid-prop-keys #{:keys :syms :strs :as :or}
                              ;; @note second position can be a vector or a
                              ;; map.  When it's a vector, it should begin
                              ;; with a keyword or symbol and it could also
                              ;; just be a string
                              hiccup?
                                (and (>= n 2)
                                     (and (or (keyword? arg-1) (symbol? arg-1))
                                          ;; @note specific to this
                                          ;; codebase, guards against
                                          ;; [_ {}] being mistaken for
                                          ;; hiccup
                                          ;;                  ;; hiccup when it
                                          ;; cannot be.  This is found
                                          (not (= '_ arg-1)))
                                     (or (string? arg-2)
                                         ;; @note arg-2 could be a vector
                                         ;; (more hiccup) and so we want
                                         ;; to treat it as such
                                         (and (and (vector? arg-2)
                                                   (>= (count arg-2) 2))
                                              (or (keyword? (first arg-2))
                                                  (symbol? (first arg-2))))
                                         (and (map? arg-2)
                                              (->> (keys arg-2)
                                                   (some invalid-prop-keys)
                                                   (not)))))]
                          (cond (and hiccup? (not (:fn-format (:vector opts))))
                                  {:vector {:fn-format :flow}}
                                (and (not hiccup?) (:fn-format (:vector opts)))
                                  {:vector {:fn-format nil}}
                                :else nil)))),
                   :wrap? false}}}}

; Now, actually define it locally (new feature, set-options! will accept a string)
(set-options! i294ud)

(print i294d)
[:div [hi] [:p "hello"]]

; I had to up the vector indent to 2 in order to get what you want, but it does seem to do it fine.

(czprint i294d {:parse-string? true :style :i294d :vector {:indent 2}})
[:div
  [hi]
  [:p
    "hello"]]

So, I get the feeling that I'm missing something. This is the current version under development, but I get the same results with 1.2.5.

That is my principal confusion: Why does what you show as the result not reproduce for me? Perhaps you have some additional configuration option map that interacts with it that I don't have?


The other confusion is that I'm not sure what you mean by "getting a better option-fn". I think that yours looks pretty good. Is there some underlying feature that is missing to allow you (or me) to create a better option-fn? How would I know that it was better (presumably because it would process the example you gave into your desired output -- but that is what I get already...).

I'm more than willing to create a better option-fn if I can figure out how to determine if it works or not. I'm also perfectly willing to add fundamental features to make creating such an option-fn possible, if I know what they are.

Thanks much for clearing this up for me!

athomasoriginal commented 1 year ago

Apologies @kkinnear I was not nearly clear enough in my original comment. Going through your points:

Perhaps you have some additional configuration option map that interacts with it that I don't have?

You're right. The issue was I specified in the config the following:

 :map  {:comma?        false
        :force-nl?     true ;; <--- the thing causing the problems
        :sort-in-code? true}

The other confusion is that I'm not sure what you mean by "getting a better option-fn"

Again, this was me reading one of your earlier comments incorrectly. Earlier, we were talking about two options. The first is what we've done here and the second was to find "Another way is by finding a way to disambiguate hiccup vectors from argument vectors". The second approach was the one I was referring to when I said, "getting a better option-fn" (which is not correct).

Either way. This seems to resolve the issue! I really appreciate your time and patience 🙇

kkinnear commented 1 year ago

Thanks for getting back to me! I'm glad things are working for you now.

For what it is worth, I still can't replicate the problem that you showed above, that is:

;; what it does
[:div [hi]
 [:p
   "hello"]]

I tried adding {:map {:force-nl? true}} to things, and I still can't make the style that you wrote and that I demonstrated above produce this output. Which leaves me a bit unsettled.

However, if you are happy with how it works now, then we don't need to continue to spend time getting me to replicate a problem that you don't have anymore. We both have plenty of other things to do.

Please know, however, that if it isn't working the way you want, that I am more than willing to figure out why and do my best to make it work for you.

Thanks!

kkinnear commented 1 year ago

I have released 1.2.6, which has an updated :style :hiccup, with some the corrections that you needed (but without some other things that you have in your custom style). I would suggest that you continue to use your custom style going forward.

Thanks for bringing this up!