kkinnear / zprint

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

Wired justify in bindings with map on the left #273

Closed yqrashawn closed 10 months ago

yqrashawn commented 1 year ago

zprint config https://github.com/status-im/status-mobile/pull/14128/commits/817a249994ef212a046304b0e563d2cc5fbe244e#diff-ad67b6e0a11904cfc29b634bde55f00d9961091fb1d34ccdb98e79cd7c2f4febR1-R49

example 1: https://github.com/status-im/status-mobile/pull/14128/commits/9ba1f28dbf14215a39833a34d346dd746d58ce3b#diff-acc2864028c846a967aae52fa52b23048f98d0b5cee98b2e118047f2dbb52171R58-R137

image

example 2: https://github.com/status-im/status-mobile/pull/14128/commits/f19bf88bb8acb1536482ad4607480498a6881b5c#diff-b95d5dad0bb120bacc3d6495349ebec19ed96bd1c2aba6a3c9ad84bc638bb78cR61-R79

image

kkinnear commented 1 year ago

I'm not entirely sure just what you find odd in these examples, but your red boxes certainly give me some clues. If I've guessed wrong, please ask me again.

In the first example, the map is indeed justified, but the let bindings are not. Regardless of what you set for the :max-variance, if the justification doesn't produce something that is deemed "better" than the unjustified output, the justification isn't accepted as the "better" format and isn't used. If the left element of a pair (the local name in this case) takes more than one line, then the justification is not "better" than regular formatting for the bindings. That is independent of the formatting of the map itself, which justifies fine internally.

The second example seemed complex to me until I broke it down a bit, but at least what I see as odd is a result of an interaction between :style :justified and :style :respect-nl. Here it is with just :style :justified-original (which does the :max-variance=1000 thing):

(czprint i273 {:parse-string? true :style :justified-original :width 105})
(defn button
  [{:keys [on-press disabled type theme before after haptic-feedback haptic-type on-long-press
           on-press-start accessibility-label loading border-radius style],
    :or   {theme :main, type :primary, haptic-feedback true, border-radius 8, haptic-type :selection}}
   children]
  (let [theme'                                                        (cond disabled :disabled
                                                                            :else    theme)
        {:keys [icon-color background-color text-color border-color]} (themes theme')
        optional-haptic                                               (fn []
                                                                        (when haptic-feedback
                                                                          (haptic/trigger haptic-type)))]
    [animation/pressable
     (merge {:bg-color            background-color,
             :border-radius       border-radius,
             :type                type,
             :disabled            disabled,
             :accessibility-label accessibility-label}
            (when border-color {:border-color border-color, :border-width 1})
            (when on-press
              {:on-press (fn [] (optional-haptic) (on-press))})
            (when on-long-press
              {:on-long-press (fn [] (optional-haptic) (on-long-press))})
            (when on-press-start
              {:on-press-start (fn [] (optional-haptic) (on-press-start))}))
     [rn/view {:style (merge (style-container type) style)}
      (when before [rn/view [icons/icon before {:color icon-color}]])
      (when loading [rn/view {:style {:position :absolute}} [rn/activity-indicator]])
      [rn/view {:style (merge (content-style type) (when loading {:opacity 0}))}
       (cond (= type :icon)     [icons/icon children {:color icon-color}]
             (string? children) [text/text
                                 {:weight :medium, :number-of-lines 1, :style {:color text-color}}
                                 children]
             (vector? children) children)]
      (when after [rn/view [icons/icon after {:color icon-color}]])]]))

You can see that the three let locals are justified.

Here it is with everything above, and :style :respect-nl added:

(czprint i273 {:parse-string? true :style [:justified-original :respect-nl] :width 105})
(defn button
  [{:keys [on-press disabled type theme before after
           haptic-feedback haptic-type on-long-press on-press-start
           accessibility-label loading border-radius style],
    :or   {theme           :main,
           type            :primary,
           haptic-feedback true,
           border-radius   8,
           haptic-type     :selection}}
   children]
  (let [theme'                                                        (cond
                                                                        disabled :disabled
                                                                        :else    theme)
        {:keys [icon-color background-color text-color border-color]}
          (themes theme')

        optional-haptic                                               (fn []
                                                                        (when haptic-feedback
                                                                          (haptic/trigger haptic-type)))]
    [animation/pressable
     (merge {:bg-color            background-color,
             :border-radius       border-radius,
             :type                type,
             :disabled            disabled,
             :accessibility-label accessibility-label}
            (when border-color
              {:border-color border-color,
               :border-width 1})
            (when on-press
              {:on-press (fn []
                           (optional-haptic)
                           (on-press))})
            (when on-long-press
              {:on-long-press (fn []
                                (optional-haptic)
                                (on-long-press))})
            (when on-press-start
              {:on-press-start (fn []
                                 (optional-haptic)
                                 (on-press-start))}))

     [rn/view {:style (merge (style-container type) style)}
      (when before
        [rn/view
         [icons/icon before {:color icon-color}]])
      (when loading
        [rn/view {:style {:position :absolute}}
         [rn/activity-indicator]])
      [rn/view
       {:style (merge (content-style type)
                      (when loading
                        {:opacity 0}))}
       (cond
         (= type :icon)
           [icons/icon children {:color icon-color}]

         (string? children)
           [text/text
            {:weight          :medium,
             :number-of-lines 1,
             :style           {:color text-color}}
            children]

         (vector? children)
           children)]
      (when after
        [rn/view
         [icons/icon after {:color icon-color}]])]]))

You can see that the second local has (themes theme') on the next line. That is because there is a newline after the map (which is the left hand element of the local pair). Thus, it comes out on a new line. And this doesn't break the justification, which is ... interesting. It might possibly be better if there were newlines in the pairs that the justification would be considered "not better" and not happen. But that may or may not be true. I'm going to have to think on that one a bit.

In any case, I hope that this clears up and explains the oddities that you noticed in the output. If not please ask again! Thanks!

yqrashawn commented 1 year ago

Thanks for your detailed explanation. I'm used to the clojure-align function provided by clojure-mode Emacs package. I think that's why I find it wired when checking zprint's output.

There's a feature in clojure-align which it respects below things when align things.

  1. blank lines
  2. multi-line left-hand thing

Here's a thread with video showing the behavior https://github.com/status-im/status-mobile/pull/14128#discussion_r1005464543 Basically, it allows users to control the alignments with blank line

What do you think about this behavior?

yqrashawn commented 1 year ago

If the left element of a pair (the local name in this case) takes more than one line

Does this mean there's no justify in bindings that with at least one multiline left-hand thing?

kkinnear commented 1 year ago

I'm looking again at this issue, and with the current code when I run the second example I gave, above, it is different in that it doesn't justify the let locals at all. This is because of the significant changes I had to make to fix the Issue #271 with repeatability/stability. Several of the issues involved justification and :respect-nl.

That said, I looked at the conversation that you pointed me at, above, with the videos and the back and forth between you and someone else. It took me a while to figure out what was going on, but I think I understand at least the blank line bit, where justification is broken up into segments separated by blank lines. Which isn't something I had ever thought of since I don't use blank lines. zprint is pretty interested in getting the maximum amount of readable code into the minimum of vertical space, and blank lines don't help that. But I can see the appeal of grouping by blank lines.

However, you clearly are interested in justification, and from looking at your code base as much as I have (which is, sigh, quite a bit), I can see why. Justification makes a lot of things look a lot clearer. I'm thinking of offering more control over justification. It looks like the "don't justify unless it is "better" than not justifying" approach that zprint currently takes isn't something that is appreciated in your environment. I think that you would like zprint to justify (or not) based on your configuration of zprint, but not decide in real-time whether justification is "better" than not justifying in every group of pairs.

So, the changes I'm contemplating (without yet knowing how easy or even possible they might be to implement):

  1. Optionally break justification up into groups based on blank lines. I think this might be tough to implement, but I can see the appeal.
  2. Optionally allow you to configure justification that isn't based on being "better" than not justifying. Of course, if it really doesn't fit within the :width, there is no way to justify it.
  3. Currently, a multi-line left hand thing causes that "pair" to not justify, which prevents justification of the entire set of pairs (e.g., bindings). Perhaps I can figure out how to optionally allow justification even when the left hand thing is multi-line.

Those are the things that I'm planning to look into.

I'm interested in your input in several areas:

  1. Are these things that would be of use to you?
  2. What would you want me to do for 2 (above) if a pair simply doesn't fit for justification? Back off on justification for the whole group of lines, or just fail this particular "pair", and justify everything else that is possible.
  3. If 3 is potentially useful for you, I'd be interested in an example of how you think a pair with a multi-line left hand thing will look if it were justified. That is, where do you put the right hand thing? I have my own idea, but if I'm doing this for you, it would be nice if it looked the way you expect (if I can do that).

Thanks!

yqrashawn commented 1 year ago

Wow, thanks for your patience reading all the discussions

where justification is broken up into segments separated by blank lines

This is like :respect-nl for justification, giving user flexibility and space to manually adjust things

Are these things that would be of use to you?

Yes, absolutely!

What would you want me to do for 2 (above) if a pair simply doesn't fit for justification? Back off on justification for the whole group of lines, or just fail this particular "pair", and justify everything else that is possible.

pair simply doesn't fit (1)

You mean exceeds the :width?

Back off on justification for the whole group of lines (2)

Don't quite understand what does this mean. Do you mean to refocus on all code included in parent level and try find ways to reduce the occupied columns?

just fail this particular "pair", and justify everything else that is possible (3)

If the answer of (1) and (2) is yes. Both ( (2) and (3) ) are reasonable solutions to me. But I think (1) might be a better choice for all zprint users.

yqrashawn commented 1 year ago

If 3 is potentially useful for you, I'd be interested in an example of how you think a pair with a multi-line left hand thing will look if it were justified. That is, where do you put the right hand thing? I have my own idea, but if I'm doing this for you, it would be nice if it looked the way you expect (if I can do that).

Got 4 videos for this one, based on the pressable-hooks (first pic in this issue)

image

Video1

https://user-images.githubusercontent.com/15090582/200998817-7ff285d0-c3e6-4fda-b99f-aa591895c168.mp4

Video 2

https://user-images.githubusercontent.com/15090582/200998856-5b4034dc-2d31-436b-a35c-8478db193e07.mp4

Video 3

https://user-images.githubusercontent.com/15090582/200998888-73c26bbb-1754-4adf-8f7c-c6af57a5ed1b.mp4

Video 4

https://user-images.githubusercontent.com/15090582/200998913-c697725c-68f3-4d08-8751-8358d9f2eaaa.mp4

kkinnear commented 1 year ago

Thanks for your quick response. I'm not sure I'm getting all of the information that you are sending with the videos, but I'm certainly trying!

First, when I said "a pair simply doesn't fit for justification", yes I meant within the width. Like the left hand side is just too big to leave any room on the right for anything. Or, perhaps the left hand side fits, but barely, leaving the right hand side very little room -- so that if you justify the pair, the right hand side runs on and on down the side, with mostly just one thing on each line, which can look pretty bad.

I see places in the videos (after they are finished) where the right hand side is on the next line (flowed, in zprint terminology) even though other contiguous things are justified (and there are no blank lines). Typically {:keys [background foreground]} has the right hand side under it despite everything else being justified. That is an example of what I think of as a pair where justification was not done for that pair, but was done for the other contiguous pairs.

Here is an interesting one. The first one is ok, but not great, in that {:keys [icon-color background-color text-color border-color]} is pretty long. The second one looks a lot better (at least to me) because that map is compressed in size on the left. That said, I don't yet know for sure how to get zprint to do that in a way that works reliably enough. Although I'm thinking that trying to format left hand sides that are collections to fit within the width of the widest non-collection left hand side might be the start of an approach that might generalize well enough. I don't know how that will work out.

(defn button
  [{:keys [on-press disabled type theme before after haptic-feedback haptic-type on-long-press
           on-press-start accessibility-label loading border-radius style],
    :or   {theme :main, type :primary, haptic-feedback true, border-radius 8, haptic-type :selection}}
   children]
  (let [theme'                                                        (cond disabled :disabled
                                                                            :else    theme)
        {:keys [icon-color background-color text-color border-color]} (themes theme')
        optional-haptic                                               (fn []
                                                                        (when haptic-feedback
                                                                          (haptic/trigger haptic-type)))]
    [animation/pressable
(defn button
  [{:keys [on-press disabled type theme before after haptic-feedback haptic-type
 on-long-press
           on-press-start accessibility-label loading border-radius style],
    :or   {theme :main, type :primary, haptic-feedback true, border-radius 8, haptic-type :selection}}
   children]
  (let [theme'                    (cond disabled :disabled :else theme)
        {:keys [icon-color
                background-color
                text-color
                border-color]}    (themes theme')
        optional-haptic           (fn [] (when haptic-feedback
                                           (haptic/trigger haptic-type)))]
    [animation/pressable

I'll get back to you once I prototype some of these changes and see what I can (and can't) do. Thanks again for your feedback.

yqrashawn commented 1 year ago

Hi, just spend some time to go over this issue again. I think the issue for me and the team is no justify when there's multiline left-hand form in bindings (as in figure 1 https://user-images.githubusercontent.com/15090582/197556889-0d34299d-478a-4390-8b8d-07dedef61056.png). Figure 2 is definitely fine.

What would you want me to do for 2 (above) if a pair simply doesn't fit for justification? Back off on justification for the whole group of lines, or just fail this particular "pair", and justify everything else that is possible.

Back off is definitely better since one of the main goal of using zprint is to avoid style/format review in PR review

If 3 is potentially useful for you, I'd be interested in an example of how you think a pair with a multi-line left hand thing will look if it were justified. That is, where do you put the right hand thing? I have my own idea, but if I'm doing this for you, it would be nice if it looked the way you expect (if I can do that).

Summary in word would be

  1. format the left-hand multiline form as usual
  2. treat the form as if it's a single line form, justify its right-hand form
kkinnear commented 1 year ago

After a bit of time away, I'm still working through the complexities of this situation. I am definitely up for having justification work when there is a multi-line left hand form. Thanks for asking for that. Prior to this issue, I've not see a lot of examples of that situation, but I agree it looks a lot better.

When prototyping that, I found that the justification is pretty sad when there is no pressure to squeeze a multi-line left hand form into something narrower than the entire remaining width. So I've been experimenting with narrowing the apparent :width for just the last line of a multi-line left hand form. The example you gave me was absolutely stellar in that the left hand map also had a map as the right hand side of its last line, and that last final map was one that would improve the justification if it was squeezed just a bit. I probably wouldn't have concocted a test like this, but the real-world examples are always so much more interesting (i.e., challenging).

Anyway, I have hacked together some code that formats one of your examples like this:

(defn pressable-hooks
  [props]
  (let [{background-color    :bgColor,
         border-radius       :borderRadius,
         border-color        :borderColor,
         border-width        :borderWidth,
         type                :type,
         disabled            :disabled,
         on-press            :onPress,
         on-long-press       :onLongPress,
         on-press-start      :onPressStart,
         accessibility-label :accessibilityLabel,
         children            :children,
         :or                 {border-radius 0,
                              type
                                "primary"}} (bean/bean props)
        long-press-ref                      (react/create-ref)
        state                               (animated/use-value (:undetermined gesture-handler/states))
        active                              (animated/eq state (:began gesture-handler/states))
        gesture-handler                     (animated/use-gesture state state)
        animation                           (react/use-memo
                                              (fn []
                                                (animated/with-timing-transition
                                                  active
                                                  {:duration (animated/cond* active time-in time-out),
                                                   :easing   (:ease-in animated/easings)})))
        {:keys [background foreground]}     (react/use-memo (fn []
                                                              (type->animation {:type      (keyword
                                                                                             type),
                                                                                :animation animation}))
                                                            [type])
        handle-press                        (fn [] (when on-press (on-press)))
        long-gesture-handler                (react/callback
                                              (fn [^js evt]
                                                (let [gesture-state (-> evt
                                                                        .-nativeEvent
                                                                        .-state)]
                                                  (when (and on-press-start
                                                             (= gesture-state
                                                                (:began gesture-handler/states)))
                                                    (on-press-start))
                                                  (when (and on-long-press
                                                             (= gesture-state
                                                                (:active gesture-handler/states)))
                                                    (on-long-press)
                                                    (animated/set-value state
                                                                        (:undetermined
                                                                          gesture-handler/states)))))
                                              [on-long-press on-press-start])]
    (animated/code! (fn []
                      (when on-press
                        (animated/cond* (animated/eq state (:end gesture-handler/states))
                                        [(animated/set state (:undetermined gesture-handler/states))
                                         (animated/call* [] handle-press)])))
                    [on-press])
    (reagent/as-element
      [gesture-handler/long-press-gesture-handler
       {:enabled                 (boolean (and on-long-press (not disabled))),
        :on-handler-state-change long-gesture-handler,
        :min-duration-ms         long-press-duration,
        :max-dist                22,
        :ref                     long-press-ref}
       [animated/view {:accessible true, :accessibility-label accessibility-label}
        [gesture-handler/tap-gesture-handler
         (merge gesture-handler
                {:shouldCancelWhenOutside true,
                 :wait-for                long-press-ref,
                 :enabled                 (boolean (and (or on-press on-long-press on-press-start)
                                                        (not disabled)))})
         [animated/view
          [animated/view
           {:style (merge absolute-fill
                          background
                          {:background-color background-color,
                           :border-radius    border-radius,
                           :border-color     border-color,
                           :border-width     border-width})}]
          (into [animated/view {:style foreground}] (react/get-children children))]]]])))

I haven't yet even begun to clean up the code to qualify this as a prototype. I'm showing it to you as an example of what I'm working toward making happen, largely to see if this looks like something that would be useful to you. Please let me know, when you have a chance, if this output is interesting or if there is something that you find terrible about it. Nothing is more painful for me than to spend a lot of time polishing up a feature just to have the folks which prompted the work to not use it.

One thing that isn't all that obvious in the above example is how I calculate the spacing between the justification. At present I've decided to allow a multi-line left hand form to overflow to the right of the justification column for the right hand side things. You can see that in :accessibility-label and another right below that. They don't expand that much.

Below is a hacked up version of your example where there is a lot more in the map that extends to the right.

(defn pressable-hooks
  [props]
  (let [long-press-ref                      (react/create-ref)
        state                               (animated/use-value (:undetermined gesture-handler/states))
        active                              (animated/eq state (:began gesture-handler/states))
        {background-color    :bgColor,
         border-radius       :borderRadius,
         accessibility-label {:duration (animated/cond* active time-in time-out),
                              :easing   (:ease-in animated/easings)},
         children            :children,
         :or                 {border-radius 0,
                              type
                                "primary"}} (bean/bean props)
        gesture-handler                     (animated/use-gesture state state)
        animation                           (react/use-memo
                                              (fn []
                                                (animated/with-timing-transition
                                                  active
                                                  {:duration (animated/cond* active time-in time-out),
                                                   :easing   (:ease-in animated/easings)})))
        {:keys [background foreground]}     (react/use-memo (fn []
                                                              (type->animation {:type      (keyword
                                                                                             type),
                                                                                :animation animation}))
                                                            [type])
        handle-press                        (fn [] (when on-press (on-press)))
        long-gesture-handler                (react/callback
                                              (fn [^js evt]
                                                (let [gesture-state (-> evt
                                                                        .-nativeEvent
                                                                        .-state)]
                                                  (when (and on-press-start
                                                             (= gesture-state
                                                                (:began gesture-handler/states)))
                                                    (on-press-start))
                                                  (when (and on-long-press
                                                             (= gesture-state
                                                                (:active gesture-handler/states)))
                                                    (on-long-press)
                                                    (animated/set-value state
                                                                        (:undetermined
                                                                          gesture-handler/states)))))
                                              [on-long-press on-press-start])]
    (animated/code! (fn []
                      (when on-press
                        (animated/cond* (animated/eq state (:end gesture-handler/states))
                                        [(animated/set state (:undetermined gesture-handler/states))
                                         (animated/call* [] handle-press)])))
                    [on-press])
    (reagent/as-element
      [gesture-handler/long-press-gesture-handler
       {:enabled                 (boolean (and on-long-press (not disabled))),
        :on-handler-state-change long-gesture-handler,
        :min-duration-ms         long-press-duration,
        :max-dist                22,
        :ref                     long-press-ref}
       [animated/view {:accessible true, :accessibility-label accessibility-label}
        [gesture-handler/tap-gesture-handler
         (merge gesture-handler
                {:shouldCancelWhenOutside true,
                 :wait-for                long-press-ref,
                 :enabled                 (boolean (and (or on-press on-long-press on-press-start)
                                                        (not disabled)))})
         [animated/view
          [animated/view
           {:style (merge absolute-fill
                          background
                          {:background-color background-color,
                           :border-radius    border-radius,
                           :border-color     border-color,
                           :border-width     border-width})}]
          (into [animated/view {:style foreground}] (react/get-children children))]]]])))

I shortened the map and put some larger right hand side thing in it. It still seems reasonable to me, since if I forced the whole map into the narrower width for the justification, it would be quite a bit longer.

I quickly hacked in some code to see what that would look like. Below is an example of restricting the map from overflowing to the right on the lines that are not the last line. It caused the map to not justify at all, which is too bad but not surprising.

 (defn pressable-hooks
  [props]
  (let
    [long-press-ref                  (react/create-ref)
     state                           (animated/use-value (:undetermined gesture-handler/states))
     active                          (animated/eq state (:began gesture-handler/states))
     {background-color :bgColor,
      border-radius :borderRadius,
      accessibility-label
        {:duration
           (animated/cond*
             active
             time-in
             time-out),
         :easing
           (:ease-in
             animated/easings)},
      children :children,
      :or {border-radius
             0,
           type
             "primary"}}             (bean/bean props)
     gesture-handler                 (animated/use-gesture state state)
     animation                       (react/use-memo
                                       (fn []
                                         (animated/with-timing-transition
                                           active
                                           {:duration (animated/cond* active time-in time-out),
                                            :easing   (:ease-in animated/easings)})))
     {:keys [background foreground]} (react/use-memo (fn []
                                                       (type->animation {:type      (keyword type),
                                                                         :animation animation}))
                                                     [type])
     handle-press                    (fn [] (when on-press (on-press)))
     long-gesture-handler            (react/callback (fn [^js evt]
                                                       (let [gesture-state (-> evt
                                                                               .-nativeEvent
                                                                               .-state)]
                                                         (when (and on-press-start
                                                                    (= gesture-state
                                                                       (:began gesture-handler/states)))
                                                           (on-press-start))
                                                         (when (and on-long-press
                                                                    (= gesture-state
                                                                       (:active gesture-handler/states)))
                                                           (on-long-press)
                                                           (animated/set-value
                                                             state
                                                             (:undetermined gesture-handler/states)))))
                                                     [on-long-press on-press-start])]
    (animated/code! (fn []
                      (when on-press
                        (animated/cond* (animated/eq state (:end gesture-handler/states))
                                        [(animated/set state (:undetermined gesture-handler/states))
                                         (animated/call* [] handle-press)])))
                    [on-press])
    (reagent/as-element
      [gesture-handler/long-press-gesture-handler
       {:enabled                 (boolean (and on-long-press (not disabled))),
        :on-handler-state-change long-gesture-handler,
        :min-duration-ms         long-press-duration,
        :max-dist                22,
        :ref                     long-press-ref}
       [animated/view {:accessible true, :accessibility-label accessibility-label}
        [gesture-handler/tap-gesture-handler
         (merge gesture-handler
                {:shouldCancelWhenOutside true,
                 :wait-for                long-press-ref,
                 :enabled                 (boolean (and (or on-press on-long-press on-press-start)
                                                        (not disabled)))})
         [animated/view
          [animated/view
           {:style (merge absolute-fill
                          background
                          {:background-color background-color,
                           :border-radius    border-radius,
                           :border-color     border-color,
                           :border-width     border-width})}]
          (into [animated/view {:style foreground}] (react/get-children children))]]]])))

At least in this example, the output looks much less desirable to me, but I'm open to an opinion here too.


There are a number of configuration and tuning "knobs" I need to add, to say nothing of many other examples I need to try. There is a lot of work left to turn this into something that is generally useful, as opposed to something that makes this particular example look very nice (which I happen to think the first example above does). I will continue to pursue finishing this, and am interested in your feedback along the way.

Thanks for taking the time to look at this.

yqrashawn commented 1 year ago

The first example is great! One question about it, it will be possible to turn the type "primary" key value pair into one line by increasing the :width right?

On my side, several people in my org asked me about the progress of integrating zprint and showed that they can't wait no more.

The blockers on my side is

  1. need to format twice to get the end formatted state
  2. this issue
  3. haven't found ways to use zprint with Cursive

My plan is

  1. config things to run zprint multiple times and let people in my org know about this, maybe set up a pre-commit hook for this, and change things back once it's fixed
  2. start using zprint first, get alone with this issue till it's fixed
  3. ask about people in my org see if anyone has any solution and if anyone using Cursive care about this.
kkinnear commented 1 year ago

Thanks for the feedback. I'm glad the first example looked good to you, it did to me too.

Yes, the type "primary" would format on one line with a little more :width, or if you change the "pressure" on the left hand side. I'm planning to make the "pressure" on the left hand side configurable, so if overall you want more or less "pressure", you can have that.

I'm sorry that zprint isn't all ready for your group yet, but the format twice problem was quite a challenge, and this one too is causing some major rethinking on how the various tuning parameters are configured. I think that your plan is reasonable, since I don't have a quick fix for the format twice problem. I'm still pushing to get the next release out before year end.

Regarding Cursive, if there is a way to export a file and re-import it then zprint can fit into that workflow. In a quick perusal of the Cursive documentation, I don't actually see any way to do that, so I looked in the IntelliJ docs, and didn't see a way to do that there either. Which was kind of surprising, because most editors historically have allowed piping part of or all of a buffer through an external "filter" (in the Unix parlance) and replacing the text piped through the filter with the resulting output of the filter.

For what it is worth, I know of a VSCode extension that integrates zprint with VSCode, but it tends to lag the version of zprint by a bit. I collaborated with the author on that extension, and I think it actually works very nicely. If that is interesting to you, let me know and I'll encourage the author to update as soon as I finish the next zprint release.

Anyway, thanks for your patience. I'll let you know when I have finished the release.

kkinnear commented 1 year ago

Release 1.2.5 is now out. It has the changes for collections on the left-hand-sides of pairs (bindings in particular). It doesn't have anything about segregating justification domains between blank lines. It was a load of work just to get the left-hand-side multi-line thing working. It is not the default -- you get it with :style :multi-lhs-hang.

Here is an example (notably w/out :respect-nl) of your code:

 (czprint i273m {:parse-string? true :style [:justified-original :multi-lhs-hang] :width 105 :binding {:justify-tuning {:binding {:tuning {:hang-flow 6}}}}})
(defn pressable-hooks
  [props]
  (let [{background-color    :bgColor,
         border-radius       :borderRadius,
         border-color        :borderColor,
         border-width        :borderWidth,
         type                :type,
         disabled            :disabled,
         on-press            :onPress,
         on-long-press       :onLongPress,
         on-press-start      :onPressStart,
         accessibility-label :accessibilityLabel,
         children            :children,
         :or                 {border-radius 0,
                              type          "primary"}} (bean/bean props)
        long-press-ref                                  (react/create-ref)
        state                                           (animated/use-value (:undetermined
                                                                              gesture-handler/states))
        active                                          (animated/eq state
                                                                     (:began gesture-handler/states))
        gesture-handler                                 (animated/use-gesture state state)
        animation                                       (react/use-memo
                                                          (fn []
                                                            (animated/with-timing-transition
                                                              active
                                                              {:duration (animated/cond* active
                                                                                         time-in
                                                                                         time-out),
                                                               :easing   (:ease-in animated/easings)})))
        {:keys [background foreground]}                 (react/use-memo (fn []
                                                                          (type->animation
                                                                            {:type      (keyword type),
                                                                             :animation animation}))
                                                                        [type])
        handle-press                                    (fn [] (when on-press (on-press)))
        long-gesture-handler                            (react/callback
                                                          (fn [^js evt]
                                                            (let [gesture-state (-> evt
                                                                                    .-nativeEvent
                                                                                    .-state)]
                                                              (when (and on-press-start
                                                                         (= gesture-state
                                                                            (:began
                                                                              gesture-handler/states)))
                                                                (on-press-start))
                                                              (when (and on-long-press
                                                                         (= gesture-state
                                                                            (:active
                                                                              gesture-handler/states)))
                                                                (on-long-press)
                                                                (animated/set-value
                                                                  state
                                                                  (:undetermined
                                                                    gesture-handler/states)))))
                                                          [on-long-press on-press-start])]
    (animated/code! (fn []
                      (when on-press
                        (animated/cond* (animated/eq state (:end gesture-handler/states))
                                        [(animated/set state (:undetermined gesture-handler/states))
                                         (animated/call* [] handle-press)])))
                    [on-press])
    (reagent/as-element
      [gesture-handler/long-press-gesture-handler
       {:enabled                 (boolean (and on-long-press (not disabled))),
        :on-handler-state-change long-gesture-handler,
        :min-duration-ms         long-press-duration,
        :max-dist                22,
        :ref                     long-press-ref}
       [animated/view {:accessible true, :accessibility-label accessibility-label}
        [gesture-handler/tap-gesture-handler
         (merge gesture-handler
                {:shouldCancelWhenOutside true,
                 :wait-for                long-press-ref,
                 :enabled                 (boolean (and (or on-press on-long-press on-press-start)
                                                        (not disabled)))})
         [animated/view
          [animated/view
           {:style (merge absolute-fill
                          background
                          {:background-color background-color,
                           :border-radius    border-radius,
                           :border-color     border-color,
                           :border-width     border-width})}]
          (into [animated/view {:style foreground}] (react/get-children children))]]]])))
nil

Here is an even better version, with a little different tuning:

(czprint i273m {:parse-string? true :style [:justified-original :multi-lhs-hang] :width 105 :binding  {:justify {:lhs-narrow 2.1} :justify-tuning {:binding {:tuning {:hang-flow 6}}}}})
(defn pressable-hooks
  [props]
  (let [{background-color    :bgColor,
         border-radius       :borderRadius,
         border-color        :borderColor,
         border-width        :borderWidth,
         type                :type,
         disabled            :disabled,
         on-press            :onPress,
         on-long-press       :onLongPress,
         on-press-start      :onPressStart,
         accessibility-label :accessibilityLabel,
         children            :children,
         :or                 {border-radius 0,
                              type "primary"}} (bean/bean props)
        long-press-ref                         (react/create-ref)
        state                                  (animated/use-value (:undetermined
                                                                     gesture-handler/states))
        active                                 (animated/eq state (:began gesture-handler/states))
        gesture-handler                        (animated/use-gesture state state)
        animation                              (react/use-memo
                                                 (fn []
                                                   (animated/with-timing-transition
                                                     active
                                                     {:duration (animated/cond* active time-in time-out),
                                                      :easing   (:ease-in animated/easings)})))
        {:keys [background foreground]}        (react/use-memo (fn []
                                                                 (type->animation {:type (keyword type),
                                                                                   :animation
                                                                                     animation}))
                                                               [type])
        handle-press                           (fn [] (when on-press (on-press)))
        long-gesture-handler                   (react/callback
                                                 (fn [^js evt]
                                                   (let [gesture-state (-> evt
                                                                           .-nativeEvent
                                                                           .-state)]
                                                     (when (and on-press-start
                                                                (= gesture-state
                                                                   (:began gesture-handler/states)))
                                                       (on-press-start))
                                                     (when (and on-long-press
                                                                (= gesture-state
                                                                   (:active gesture-handler/states)))
                                                       (on-long-press)
                                                       (animated/set-value state
                                                                           (:undetermined
                                                                             gesture-handler/states)))))
                                                 [on-long-press on-press-start])]
    (animated/code! (fn []
                      (when on-press
                        (animated/cond* (animated/eq state (:end gesture-handler/states))
                                        [(animated/set state (:undetermined gesture-handler/states))
                                         (animated/call* [] handle-press)])))
                    [on-press])
    (reagent/as-element
      [gesture-handler/long-press-gesture-handler
       {:enabled                 (boolean (and on-long-press (not disabled))),
        :on-handler-state-change long-gesture-handler,
        :min-duration-ms         long-press-duration,
        :max-dist                22,
        :ref                     long-press-ref}
       [animated/view {:accessible true, :accessibility-label accessibility-label}
        [gesture-handler/tap-gesture-handler
         (merge gesture-handler
                {:shouldCancelWhenOutside true,
                 :wait-for                long-press-ref,
                 :enabled                 (boolean (and (or on-press on-long-press on-press-start)
                                                        (not disabled)))})
         [animated/view
          [animated/view
           {:style (merge absolute-fill
                          background
                          {:background-color background-color,
                           :border-radius    border-radius,
                           :border-color     border-color,
                           :border-width     border-width})}]
          (into [animated/view {:style foreground}] (react/get-children children))]]]])))

From the CHANGELOG.md:

I would suggest that you try it without :respect-nl, and if you really need to keep some of your newlines, you might want to go for :respect-bl, which will keep blank lines but otherwise will ignore the existing newlines. It was truly a challenge to get this stuff to justify and look nice without :respect-nl, I don't know that it is possible to do it with :respect-nl. If you absolutely have to use :respect-nl, you might run one overall formatting pass without it (and with tuning parameters that give you a nice look), and then turn it back on. Otherwise you are just forever getting the old poor formatting, enshrined in the existing newlines in the file.

Please respond with questions about these capabilities, as I'm sure I haven't explained them very well. I've been so deep in it for so long that I have probably lost sight of what you really need to know to use them. I've been pushing to get this release out for a long time!

yqrashawn commented 1 year ago

Thanks! Will try.

kkinnear commented 10 months ago

I'm going to consider this basically fixed. I created a new issue for "Consider segregating justification domains between blank lines", to capture that part of the request here.