zprint icon indicating copy to clipboard operation
zprint copied to clipboard

Should comments have special treatment with `{:parse {:interpose "\n\n"}}`?

Open mynomoto opened this issue 6 years ago • 8 comments

Starting with:

(ns foo)

; a comment that can be very very very very very very very very very very very very very very very very very very very long about baz 
(defn baz [])

Below we have the comment on multiple lines but no empty lines between the comment lines:

$ zprint-0.4.5 '{:parse {:interpose "\n\n"}}' <<< $'(ns foo)\n\n; a comment that can be very very very very very very very very very very very very very very very very very very very long about baz \n(defn baz [])'
(ns foo)

; a comment that can be very very very very very very very very very very very
; very very very very very very very very long about baz

(defn baz
  [])

Now we have a empty line between the multiple comment lines.

$ zprint-0.4.5 '{:parse {:interpose "\n\n"}}' <<< $'(ns foo)\n\n; a comment that can be very very very very very very very very very very very\n; very very very very very very very very long about baz \n(defn baz [])'
(ns foo)

; a comment that can be very very very very very very very very very very very

; very very very very very very very very long about baz

(defn baz
  [])

I don't know if there is something to be done (or if it should be done) that could make the comment lines "stick" with their "reference" or to avoid putting empty lines between the comments with multiple lines. Also the problem running zprint twice and getting different results remind me of #67 which I stumble upon recently.

mynomoto avatar Mar 20 '19 18:03 mynomoto

First, the theory about :interpose is that it will operate on all top level elements. I didn't really think about trying to keep comments next to the things they reference. I suppose I could figure out how to do that. I'm not sure it is a good idea. Maybe you could help me out with telling me your reasoning behind using :interpose. I originally implemented it because someone wanted to use it while using zprint as a library, and wanted better spacing control with (as I rather dimly recall) structures (where there aren't any comments). You are using it as part of source file formatting, it appears. Most people seem to just put the blank lines where they want them and, without :interpose, zprint will let them be. So, help me better understand your reasoning behind using :interpose, so I can get a sense of your use case, if you would.

Second, I don't see you getting different results here when running zprint twice -- it remains a problem, for sure, but not with the examples you are using here, I think. Unless I'm missing something.

kkinnear avatar Mar 20 '19 23:03 kkinnear

Yes, I use it to format source files. We even have it running on CI :wink: Using zprint was the best way to avoid discussing the way code looks on our codebase and we have a coding style where it should be 1 empty line between top level forms, which is why I wanted to use interpose. But I want to special case comments.

About the different results those are not from running zprint twice on the same code, but running zprint on the code and then running zprint on the result. I expected that when running zprint twice on the same code, that on the second run no changes would happen. On the example above I run zprint on the output of the first run and it added an extra line break between the comment lines.

mynomoto avatar Mar 21 '19 16:03 mynomoto

Thanks for explaining this. I will work to keep the comments with the things they comment on.

Additionally, zprint certainly should not continually change a file each time it processes it. There is already a bug for that, but the situation is a bit different, and they might well be different bugs. I'll look into that too. Presently I'm deep into trying to make :respect-nl? work for lists, so I will get to this as soon as I can.

kkinnear avatar Mar 23 '19 23:03 kkinnear

I am finally getting to fix this. I'm sorry it has taken a year! In the unlikely event that you still care about this, I have a question for you. A step to fixing this is to keep all comments without blank lines between them together, no matter what :interpose is specified.

My question is about how to handle other things after comments. One approach is to simply keep a comment next to the following line of any sort if there were no blanks lines after it, and if there were blank lines after it, put in whatever was in :interpolate after it.

Would that give you what you want(ed)?

kkinnear avatar Apr 22 '20 17:04 kkinnear

My question is about how to handle other things after comments. One approach is to simply keep a comment next to the following line of any sort if there were no blanks lines after it, and if there were blank lines after it, put in whatever was in :interpolate after it.

Would that give you what you want(ed)?

Yes, that should be what I would like to happen. That said I stopped using zprint because of the many customizations that I had to do to have something formatted the way I wanted.

I'm thankful for zprint and all the many times I used it.

mynomoto avatar Apr 22 '20 17:04 mynomoto

Thanks for getting back to me, even if you've stopped using zprint!

I'm sad that you found zprint too much trouble to configure. While I expect you don't want to bother with it anymore, I'd be delighted to figure out what it takes to configure zprint to format things the way you like, and then turn that into a :style, to make it less onerous for you to use. Possibly others would find your approach interesting as well, but even if not, I'd be glad to make a style that did what you want.

kkinnear avatar Apr 23 '20 16:04 kkinnear

My latest config was:

{:binding     {:force-nl? true}
 :fn-force-nl #{:arg1-body :arg1-force-nl :binding :flow :flow-body :force-nl
                :force-nl-body :noarg1 :noarg1-body}
 :fn-map      {":import"          :flow
               ":require"         :flow
               "comment"          :flow
               "comp"             :flow
               "concat"           :flow
               "def"              :arg1-body
               "def-dbfn"         :arg2-fn
               "defprotocol"      :arg1-force-nl
               "defrecord"        :arg1-extend
               "defspec"          :arg2-fn
               "deftask"          :arg1-force-nl
               "for-all"          :binding
               "if"               :arg1-force-nl
               "if-not"           :arg1-force-nl
               "import"           :flow
               "merge-env!"       :pair-fn
               "mlet"             :binding
               "pull"             :arg1-force-nl
               "q"                :arg1-force-nl
               "reg-event-db"     :arg1-force-nl
               "reg-event-fx"     :arg1-force-nl
               "reg-fx"           :arg1-force-nl
               "reg-sub"          :arg1-force-nl
               "require"          :flow
               "response-for"     :arg1-pair
               "set-env!"         :pair-fn
               "task-options!"    :pair-fn
               "testing"          :arg1-force-nl
               "transact!"        :arg1-force-nl
               "when"             :arg1-force-nl
               "when-not"         :arg1-force-nl
               "with-dyn-session" :flow
               "with-pre-wrap"    :arg1-force-nl
               "with-responses"   :arg2-fn
               "with-system"      :arg1-force-nl}
 :map         {:comma?    false
               :force-nl? true
               :lift-ns?  false}
 :max-length  120
 :pair-fn     {:hang? false}
 :style       [:justified :pair-nl :map-nl :extend-nl :spec :binding-nl]}

The main problem was that :fn-map kept growing.

What I really like about zprint is that doesn't matter how the code is formatted, the output should be deterministic. This is very good for working on teams and have no discussions at all about how the code should be formatted.

That said, even with that amount of configuration I could not make it format code in a way that I found pleasant to read/easy to understand, specially with nested code and specially for nested maps. So I changed our official formatter to clj-fmt that keeps people of doing crazy things but sometimes we have discussions about formatting on PRs.

What I dislike on the way zprint formats code is that it breaks lines before I think it should. I know that this is a hard problem so I'm not saying that you should do something about it. Sometimes the result is having a (let that is alone on a line. Same thing about breaking between keys and values on a map.

Another problem that we had as our codebase grew is that the time necessary to run zprint on some of the test files (that have lots of indentation and some pretty big forms) was really long.

mynomoto avatar Apr 23 '20 18:04 mynomoto

This is fixed in 1.0.0 -- comments which have no blank lines following them will not get any blanks lines after them from the use of :interpose.

kkinnear avatar Jun 08 '20 16:06 kkinnear