Retain line separators when generating files #37
Conversation
src/KnitPlugin.kt
Outdated
| """.trimMargin() | ||
| ) | ||
| } | ||
| return ls ?: System.lineSeparator() |
There was a problem hiding this comment.
What puzzles me here is whether System.lineSeparator() should be the default.
You've already done a great job with preserving separator for existing files, so the migration for existing users is gradual.
But having system-dependent separator as default behaviour may be not so convenient -- it means that every project using Knit and being developed on multiple platforms have to configure its own line separator (=> also somehow figure out such option exists).
WDYT about making UNIX's \n the default here?
There was a problem hiding this comment.
Thanks for looking at my PRs.
I think it is a good idea to set UNIX style as default.
src/KnitPlugin.kt
Outdated
| """Knit defaultLineSeparator must be one of: | ||
| |- Unix (\n) | ||
| |- Windows (\r\n) | ||
| |- Classic Mac OS (\r) |
There was a problem hiding this comment.
This is rather my libraries-related ranting, so feel free to ignore or object :)
'\r' seems to be a long-dead legacy (OS 9 and long dead classic OS X from 2002) and it's unlikely that someone either uses this machines or actually preserves their convention, so I always strongly object against adding \r to any new code
There was a problem hiding this comment.
I was unsure at first what to support, then I just followed what IntelliJ offers (bottom right corner in editor).
I'm fine with just supporting UNIX and WIN. However, one could argue now (even before, but more now) why to pass the line separator explicitly to the extension and not an enum. I think, usually it's awkward in a Gradle script to import enums. Another possibility now would be to switch the two cases with a boolean. Would you like that better?
There was a problem hiding this comment.
I think this is completely okay the way it is. Especially when we've changed the default and this option is just a temporary migration mechanism
Nowadays it is standard to work on projects with Unix line endings even under Windows. However, Knit always uses the default system line separator when writing files. This also implies that it always changes the line endings of files whenever it has to update them and the original line ending differs from the system default. This is quite annoying in the aforementioned situation.
This PR offers the following enhancements:
defaultLineSeparator(withSystem.lineSeparator()as default) in theknitextension allows to specify the line separator which shall be used when new files are written.