Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extra brackets added in (fn [] ...) #25

Closed
rafd opened this issue May 2, 2017 · 5 comments
Closed

Extra brackets added in (fn [] ...) #25

rafd opened this issue May 2, 2017 · 5 comments
Labels

Comments

@rafd
Copy link

rafd commented May 2, 2017

I've run into case where zprint is adding extra brackets resulting in incorrect code:

It seems to occur when the body of a fn is moved to a new line:

input:

(zprint-str '(fn [contact] (= (contact :company) "Some Long String Here")) 50)

output:

(fn [contact]
  ((= (contact :company)                  ; <--- note extra set of brackets around the fn body
      "Some Long String Here")))

When the contents of the fn can fit on one line, it's ok:

input:

(zprint-str '(fn [contact] (= (contact :company) "Short")) 50)

output:

(fn [contact] (= (contact :company) "Short"))
@kkinnear
Copy link
Owner

kkinnear commented May 3, 2017

Thank you finding this. The underlying bug manifests in several ways, and I ran into a different symptom of this same bug a week or so ago, and spent a good bit of time fixing it. I haven't yet released the fix because I was trying to react to the clojure.spec -> clojure.spec.alpha change (which for me is still a work very much in progress). I was hoping to release all of this together.

I can reproduce your problem with no difficulty in zprint 0.3.2, and 0.3.3 fixes it:

; In zprint 0.3.3:
(czprint '(fn [contact] (= (contact :company) "Some Long String Here")) 50)
(fn [contact]
  (= (contact :company) "Some Long String Here"))

As it happens, this bug doesn't affect lein-zprint, because it only shows up when using zprint on lists of code that are Clojure structures, not when parsing strings of Clojure code. If instead of quoting the (fn ..), you put it into a string (remembering to escape the quotes in the embedded string with ), and then give it an options map of {:parse-string? true}, it works fine:

; In zprint 0.3.2:
; Fails as Clojure list
(czprint '(fn [contact] (= (contact :company) "Some Long String Here")) 50)
(fn [contact]
  ((= (contact :company)
      "Some Long String Here")))
; Works when zprint parses the string as code
(czprint "(fn [contact] (= (contact :company) \"Some Long String Here\"))" 50 {:parse-string? true})
(fn [contact]
  (= (contact :company) "Some Long String Here"))

Not that you probably care, but the same zprint engine handles code that has been parsed from strings or Clojure structures . There are manipulation functions that operate on a zipper or Clojure data structure with the same outcome -- except in this case where the manipulation functions were slightly off in the Clojure data structure case.

Anyway, I'll try to release a fix soon (i.e., before I solve the clojure.spec.alpha situation).

Thanks again for logging the issue! If I hadn't been modifying something related I might have never run into this myself, as I usually don't use zprint on code that is a Clojure data structure.

@rafd
Copy link
Author

rafd commented May 3, 2017

Thanks for the reply. No rush on my end.

As a partial workaround I had :fn-map {"fn" :binding} (which probably changes the behaviour in subtle ways, but works for me for now).

I overlooked {:parse-string? true}. Good to know.

@kkinnear
Copy link
Owner

kkinnear commented May 4, 2017

That's a nice workaround. I should have a new release out in a day or so.

I clearly need some more tests to see if my code formatting works correctly when formatting Clojure code from an s-expression instead of a string. I have several fidelity tests which check to see if I'm breaking code formatted from text (and I also use it on the zprint source files all the time). But I haven't put the effort into equivalent tests for code in s-expressions. I see that I need to do so.

I'm curious about your use-case, where you are formatting code from s-expressions, if you care to share it.

@kkinnear kkinnear added the bug label May 4, 2017
@rafd
Copy link
Author

rafd commented May 4, 2017

At the moment, I'm using zprint to format examples for a clojure reference book. I have all the copy and code in an edn file, and prefer to use quoted s-expressions (over strings) because I still get syntax highlighting in my text editor.

Perhaps instead of keeping a separate code-path in z-print for s-expressions, you could just pr-str the s-expressions and then pass them onto the string parser?

kkinnear added a commit that referenced this issue May 5, 2017
Also, new tests for Issue #25
@kkinnear
Copy link
Owner

kkinnear commented May 5, 2017

Thanks a lot for telling me how you use this, it helps me to know the various use cases. I'll look forward to seeing your book!

I don't actually have separate code paths for s-expressions and strings. There is just one pretty printing engine using one set of function names to move around the thing it is pretty printing. There are two sets of "move around" functions, one set to move around the zipper that results from the parsing of a string, and one set to move around regular s-expressions.

This Issue #25 is fixed in zprint 0.3.3.

@kkinnear kkinnear closed this as completed May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants