search

pesterhazy / boot-fmt

Boot task to auto-format Clojure(Script) code
Eclipse Public License 1.0
34 stars 7 forks source link

adds extra newlines between comment lines on output #5

Closed kkinnear closed 7 years ago

kkinnear commented 7 years ago

Hi! Neat library. I'm sure glad you did this, since I know nothing about boot. I wouldn't have known where to even begin. I had to install boot to even try it!

This particular issue -- the extra newlines between all of my ;; comment lines, is because you are using :parse-string-all? with :interpose. Turns out that :parse-string-all? was added for @viebel (the fellow who wrote klipse), as he wanted to use zprint to format multiple forms in a single string. I don't know if he ended up using it or not, but the :parse-string-all? isn't friendly to anything between forms in the string. Rather, it treats them all alike, and puts the :interpose between all of them.

I am pretty sure you could use zprint.core/zprint-file, since at a quick look boot-fmt is operating on files. It doesn't mess with the formatting of anything that is considered "whitespace" in the file, and is what lein-zprint uses. I would have no problem making that part of the documented API if you use it.

That would also give you support for the ;!zprint {<options-map} capability for free. In case you haven't looked into it, the ;!zprint {<options-map>} capability lets you put a comment in your source that changes the formatting for just the next function definition. For details, see the lein-zprint readme. I put it in because I was doing my own files, and found some functions that really needed something other than the default formatting. If you don't want the ;!zprint comments to work [I'll be sad, but] you can still use zprint-file (sort of) in one of two ways.

  1. Let me know that you want to use zprint-file but don't want ;!zprint support, and I'll add an argument to zprint-file to do what it does now w/out ;!zprint support.
  2. You can just copy zprint-file yourself and change the call to process-multiple-forms so that :process-bang-zprint? is false. I don't know how hard that would be to get it to link up with the other stuff in zprint.core, but it would probably be possible.

If you don't want ;!zprint support, I would prefer that you ask for the change to zprint-map, and I'll get it in the next release (due this weekend, I hope).

pesterhazy commented 7 years ago

Thanks for this! I hadn't noticed zprint-file, it does work better! I've used a slightly modified version in the code, as zprint-file always spits to a file whereas I sometimes just want to get the contents as a string. Creating a tempfile seems overkill. Maybe you could provide a separate version zprint-whole (as in this comment) that doesn't operate on files but on strings?

pesterhazy commented 7 years ago

Shebang option support sounds awesome. Actually I was going to ask you to add support for this, I'm super happy to see it's already there.

I've created an issue to add documentation for this feature to boot-fmt.

kkinnear commented 7 years ago

Sure, I will be glad to provide a version of zprint-file which operates on strings and not files. I don't see a comment, for what that's worth, but I'll just pull out the insides of zprint-file into a separate function, zprint-whole. Should also make it easier to test, come to think of it. Right now the ;!zprint tests (such as they are) live in lein-zprint, but the processing lives in zprint itself.

pesterhazy commented 7 years ago

Sorry I meant "commit", not comment: https://github.com/pesterhazy/boot-fmt/commit/7c93439232b8dbdb2948c0203d70f7f9fdb60f03#diff-55867f69b1a11ee1474a25191d2d82e8R9

:)