search

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

Need to format twice to get to the end formatted state #271

Closed yqrashawn closed 10 months ago

yqrashawn commented 1 year ago

zprint config https://github.com/status-im/status-mobile/pull/14128/commits/5b4c1c5c949d66c928114d4aba0581a72c0dc326 first format https://github.com/status-im/status-mobile/pull/14128/commits/7323b4c111271bb9a292b3dd2d6a9b2f371861a1 second format https://github.com/status-im/status-mobile/pull/14128/commits/bdfff97631d9efc8e5dfa77e529cf6096766a845

PR containing these commits https://github.com/status-im/status-mobile/pull/14128/commits

kkinnear commented 1 year ago

I want to thank you both for noticing this unfortunately widespread problem and for the way that you reported it using your GitHub repo. I didn't start out appreciating the mass of information in diffs in the links you provided. That was because I expected that there was a single problem that, once I found and fixed it, would make it all work fine. However, so far I have found two outright bugs which I've fixed, and one design issue with the formatting heuristics that I'm currently working on.

These kinds of problems are hard to boil down to something small enough to actually debug, and then actually debugging them is also a considerable challenge. So far, after I fix one of them, I figure I've solved the problem and so I pick a few files from the many to choose from that changed, and try them. And, indeed, most of them now don't change. But not all of them. So then I have another problem to track down and fix. And now another. Ultimately, the mass of changing files has been a real help in keeping me from prematurely thinking I'm finished when in fact I'm not. So, looking back, this was a splendid way to report this kind of problem, and I appreciate it!

It turns out that most of these problems (so far) hinge on the use of :respect-nl, since if you always use that, every newline is there ... forever. Some of the formatting heuristics that make the code look nice depend on line counts and comparing line counts of formatting one way compared to another way. With :respect-nl the line counts can change from run to run and this triggers some edge conditions in the heuristics and has exposed an ordering problem in the way that they are used.

In any case, I'll keep at this until the files don't change (or I decide that isn't possible, I guess). I just wanted you to know that I take this issue seriously and am certainly working on it.

yqrashawn commented 1 year ago

Appreciate for letting me know your feeling and progress. This really helps me do more thinking before firing an issue. Should add more description and thoughts behind the scene when creating new issues.

kkinnear commented 1 year ago

Just released zprint 1.2.6 which has added "smart wrap" for comment wrapping. This was the last major change required for multi-run stability. Now that comments wrap more cleverly, they will less often cause additional lines to be created and therefore less often change the formatting of the code the next time the code is formatted. It can still happen, but hopefully less often. I'm going to consider this issue completed unless and until you detect additional problems.

Thanks for bringing this up. It was not an easy problem to solve, and even now it isn't perfect -- but it is a lot better, at least in my testing.