Add support for Windows newlines in the log

[zdavidsen]

Really not an issue for most text editors, but Notepad requires CRLF and it's a quick easy fix

[meepen]

Usually the mode you open a file handles these things for you, this is probably the wrong way

[zdavidsen]

Hmm, I'll take a look and see if I can't find a better way

[zdavidsen]

It's worth mentioning that pretty much the only program that this affects is Notepad, as pretty much every other program handles LF gracefully. Though Notepad is the default, I imagine that most people who are actually checking the log probably have another editor on hand. Another detail pointed out in this thread is that different line endings could break parsing, though I don't imagine that should be a huge issue here?

That said, I can't find any node documentation that suggests that line endings can be auto handled based on the file mode. The only solution I've found exposed is os.EOL, which involves another require, but would be a better solution.

Apologies if I've missed something better, I've never worked directly with node.js before, so this is mostly pulled from some quick searches and skimming the docs.

[meepen]

If you could use os since it's in node by default that's fine, I was mostly mentioning wb and w mode since almost every other language supports it and handles new lines cross platform perfectly, node might not have that

Might also want to look to see if there is an option in fs to write to a file in non binary mode

On Mon, Jul 2, 2018, 7:44 PM Zac Davidsen notifications@github.com wrote:

It's worth mentioning that pretty much the only program that this affects is Notepad, as pretty much every other program handles LF gracefully. Though Notepad is the default, I imagine that most people who are actually checking the log probably have another editor on hand. Another detail pointed out in this thread https://github.com/nodejs/node/issues/5038 is that different line endings could break parsing, though I don't imagine that should be a huge issue here?

That said, I can't find any node documentation that suggests that line endings can be auto handled based on the file mode. The only solution I've found exposed is os.EOL, which involves another require, but would be a better solution.

Apologies if I've missed something better, I've never worked directly with node.js before, so this is mostly pulled from some quick searches and skimming the docs.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/meepen/salien-bot/pull/175#issuecomment-401970013, or mute the thread https://github.com/notifications/unsubscribe-auth/AF5lSUrZcQg58CBubYgrZi8y1Ao-VZqrks5uCrBfgaJpZM4U__lJ .

[zdavidsen]

It's a little hard to tell, but it looks like node already writes in non-binary if you pass the write function a string, and only writes in binary if you pass it a buffer? semi relevant doc

I'll switch it over to use os tomorrow when I get some time. Would you prefer to keep os in a const global like fs and then just drop os.EOL in for the newline character, or just pull os.EOL into a let and not keep all of os around?

[meepen]

const os = require("os"); should be fine