brandonbloom / fipp

Fast Idiomatic Pretty Printer for Clojure
525 stars 44 forks source link

Escaping of newlines in strings #75

Closed isaksky closed 3 years ago

isaksky commented 3 years ago

Currently it looks like fipp always escapes newlines in strings. For example:

user=> (require '[fipp.edn :as edn])
nil
user=> (def foo {:sql "select *
  #_=> from projects
  #_=> where id < 5"})
#'user/foo
user=> (edn/pretty foo)
[:group "{" [:align ([:span [:text ":sql"] " " [:text "\"select *\\nfrom projects\\nwhere id < 5\""]]) nil] "}"]
user=> (edn/pprint foo)
{:sql "select *\nfrom projects\nwhere id < 5"}
nil

My use case involves updating EDN files with handwritten SQL fragments, so for me, escaping of the newlines is harmful.

A solution I found was to copy and paste fipp.edn/EdnPrinter (~70 lines), then change visit-string from this:

  (visit-string [this x]
    [:text (binding [*print-readably* true]
             (pr-str x))])

to this:

 (visit-string [this x]
    ;; This is the part that changed.
    (binding [*print-readably* true]
      [:span "\""
       :pass x
       :span "\""]))

Is there a better way? If not, should there be? (E.g., a setting you can pass to EdnPrinter?)

brandonbloom commented 3 years ago

A few things:

1) Fipp doesn't currently make newline handling configurable because it relies on pr for fast serialization to EDN for strings. 2) The visit machinery is designed precisely so that it's small enough for you to copy/paste/edit if you need customized pretty documents. 3) If you just want to monkey-patch the visit-string method, it should be possible to use extend (or extend-type or extend-protocol) instead of copy/pasting the whole of the EdnParser. 4) I'm not sure that syntax you used is valid. Did you mean [:span "\"" [:pass x] [:span "\\""]]? Which could also be simplified to [:span "\"" [:pass x] "\""]. 5) Have you seen https://github.com/brandonbloom/fipp/blob/master/doc/primitives.md ? I don't think you want :pass at all, as it's designed for zero-width control sequences. 6) It's also worth calling out the fact that Fipp is not a code formatter: https://github.com/brandonbloom/fipp/issues/21#issuecomment-64693415

If you wanted to do this "right", I'd probably look at inlining & editing the definition of print-method for strings:

https://github.com/clojure/clojure/blob/3b6256e654bf250ddfd01cdaa4be9f39a74c2de6/src/clj/clojure/core_print.clj#L212-L221

That would allow you to get proper newlines behavior by simply changing char-escape-string to return a [:break] node to get hard breaks that are measurement aware.

Another potential trick would be to wrap the output in a [:group ...] use a :line node with an inline form of the escape sequence: [:line "\\n"]. However, I'm not sure how to make this work correctly because Clojure does not strip prefix indentation, so using this trick would change the content of the strings. SQL doesn't care, but this would be a disaster for many other use cases. You might want a custom tagged-literal type to opt-in to this behavior.

Let me know if any of that helps.

isaksky commented 3 years ago

Ok, thanks for explaining. That makes sense, especially with the design thoughts in 6. I'll go with one of those methods you suggested.

That means this is not an issue, so I'll close it. Thanks for the awesome library!