Invoca/magic_frozen_string_literal

Override newline code

taichi-ishitani opened this issue · 7 comments

Hi,

I use your magic_frozen_string_literal command on Windows and found that the command overrides newline code.

$ ruby -v
ruby 2.5.3p105 (2018-10-18 revision 65156) [x64-mingw32]

Before:
before

After:
after

I think that the command should not override newline code.

CR-LF is the Windows standard for end-of-line, correct? But you're thinking it should always use LF? Or that it needs to match what it finds?

I think the cause is that the file is opened as text (the default):

File.open(filename, "r+") do |file|

Probably opening in binary would fix this:

File.open(filename, "rb+") do |file|

But what if the file has CR-LF line endings in Windows? If we open in binary mode, those will be preserved, but the MAGIC_COMMENT lines that we write will not match, because they are terminated by \n\n. Would this be a problem?

Hi @ColinDKelley

CR-LF is the Windows standard for end-of-line, correct?

Yes, CR-LF is the standard newline code for Windows but I use LF because I use mainly Linux.

But you're thinking it should always use LF? Or that it needs to match what it finds?

I think the original newline code should be maintained regardless of the platform.

But what if the file has CR-LF line endings in Windows? If we open in binary mode, those will be preserved, but the MAGIC_COMMENT lines that we write will not match, because they are terminated by \n\n. Would this be a problem?

If the opening mode is binary mode then \n will be written as LF regardless of the platform. I think this is a problem for Windows users because they expect newline code is CR-LF.

I think there are 3 options for this issue:

  • Detect newline code from the file contents
  • Add an option switch to specify newline code
  • Not support files whose newline code is not standard newline code

Yes, CR-LF is the standard newline code for Windows but I use LF because I use mainly Linux.

Then why not run this tool in Linux?

If the opening mode is binary mode then \n will be written as LF regardless of the platform.

I know. My comment was that if we open the file in binary mode, AND the Windows file is using CR-LF endings, we will create an inconsistency: the existing lines that carry across will end with CR-LF but the MAGIC_COMMENT (which is always replaced) will end in LF. This is very difficult to fix because it involves detection logic as you mention in your first bullet.

Perhaps the simplest answer would be: those who run Ruby tools in Windows need to normalize their newline characters afterwards? There are simple tools for this, right? At tool like that could even be set as a git pre-commit hook.

But to be clear: if you want to code any of your options in a branch, I am fine merging it.

But to be clear: if you want to code any of your options in a branch, I am fine merging it.

I opened PR(#28) for the option below:

Detect newline code from the file contents