search

AdvancedCustomFields / acf

Advanced Custom Fields
http://advancedcustomfields.com/
823 stars 168 forks source link

Forcible use of CRLF breaks use of Git with `core.safecrlf` option enabled #855

Closed FlyingDR closed 11 months ago

FlyingDR commented 11 months ago

ACF 6.1.7 starts to put \r\n sequence at the end of local JSON files:

https://github.com/AdvancedCustomFields/acf/blob/48ba60e8faaf613aa7039cb8ae5d2b56e9dc3d5c/includes/local-json.php#L347

\r\n EOL sequence is specific to Windows and using this hardcoded value caused files to have a mixed style of EOL. It also effectively breaks the ability to commit these files to Git in case when core.safecrlf option is enabled and core.eol is set to lf.

Since most of the production sites are running on OS other than Windows - the decision of forcible use of Windows-specific EOL marker doesn't look obvious.

I would propose replacing the hardcoded \r\n sequence with use of PHP_EOL constant and allow developer to update its value if necessary by wrapping default value to apply_filters(). Something like this:

$result = file_put_contents( $file, acf_json_encode( $post ) . apply_filters( 'acf/local_json_file_eol', PHP_EOL ) );
mattgrshaw commented 11 months ago

Hey @FlyingDR,

Thanks for the heads up on this one. Your proposed change seems sensible enough - we'll look into getting that fixed up for ACF 6.2.

mattgrshaw commented 11 months ago

Thanks again for the report. We've made this change in ACF 6.2.0-RC1 and will be releasing it officially within the coming weeks.

I'll make a note on our end to make sure we give you credit in the official release notes!

FlyingDR commented 11 months ago

@mattgrshaw Thank you for the quick response and fix!