Features sometimes generates a file with incorrect "\r\n" or CRLF line endings.

An example of when this happens is when "featurizing" the allowed values of a field. (see line 56 of ideation.features.content.inc in Ideation-1.0-rc1 on drupal.org)

Though against drupal coding standards, this hasn't caused me any issues until I tried to create a patch using Gist. Since Gist doesn't allow "\r" line endings it's impossible (I think) to create a patch file involving the "\r\n" line endings.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jmcintyre’s picture

In at least one test situation (the DAMP stack running on MacOS), it looks like MySQL is returning CR/LF for line breaks in text fields. I've seen this happen in strongarm settings as well as allowed values for a CCK field.

Out of curiosity, what does your environment look like? I wonder if there's a chance this could be a MySQL config issue.

langworthy’s picture

MAMP Pro 1.9.4. MySQL 5.1.44

I'll try to test things out on another environment.

mpotter’s picture

Status: Active » Closed (won't fix)

Closing this for lack of activity. Please re-open this issue if you can reproduce it in the latest version.

smithmilner’s picture

Version: 6.x-1.0 » 7.x-1.0
Status: Closed (won't fix) » Active

I'm confirming the presence of bad line endings with features 7.x-1.0

It was a team member that rolled the feature but I noticed the line endings. This is occurring for me in multi-line strongarm variables. Eg.

  • user_mail_cancel_confirm_body
  • user_mail_password_reset_body
  • user_mail_register_admin_created_body
  • user_mail_register_admin_created_subject
  • user_mail_status_activated_body

etc

mpotter’s picture

Status: Active » Closed (won't fix)

See the comment in #1 above. You need to check your MySQL config and make sure it isn't using CR/LF for line breaks. Features is just calling the var_export routine for your Strongarm variables so there isn't really anything to be done for this in Features itself. But if somebody thinks this is something Features can fix, post a patch. Or if it's something that Strongarm can fix, move this over to the Strongarm project queue.

langworthy’s picture

Anyone know the my.cnf option for line breaks? Google isn't helping me at the moment.

Alan D.’s picture

Version: 7.x-1.0 » 7.x-2.x-dev
Status: Closed (won't fix) » Active

This is a bug and should not rely on DB settings.

You create the file on windows, you get \n\r on unix \n, etc. Then on the server this may be different. Perfectly legit to export windows > unix / mac > windows / whatever. Some developers may be on windows, others on unix, so different features may have different line endings

But the only place where this matters is the diff, so this code will solve peoples issues:

str_replace(array("\r\n", "\r"), "\n", $text);

This is what the text field diff handler calls in Diff 7.x-3.x branch to resolve similar issues

function diff_normalise_text($text) {
  $text = str_replace(array("\r\n", "\r"), "\n", $text);
  return $text;
}

[edit]
And this code is used in core too, in check_markup().

Drupal: filter.module line 750ish in 7.x/8.x, line 450ish in 6.x.

  // Convert all Windows and Mac newlines to a single newline, so filters only
  // need to deal with one possibility.
  $text = str_replace(array("\r\n", "\r"), "\n", $text);
Alan D.’s picture

Status: Active » Needs review
FileSize
1.43 KB
DamienMcKenna’s picture

Status: Needs review » Needs work
DamienMcKenna’s picture

Status: Needs work » Needs review
ar-jan’s picture

This is still an issue with 2.0. In my case, a feature containing filter configuration exports the pathologic base path settings with CR line endings.

Patch in #8 no longer applies, here's a reroll. Unfortunately it did not yet solve the issue for me. Does the same need to be done specifically for configurations like filter settings or is this approach not / no longer working?

hefox’s picture

Title: Features sometimes generates incorrect line endings » Replace wrong style line ending in configuration on export
Category: Bug report » Feature request
Status: Needs review » Needs work

I agree with mpotter that this is more a database configuration (or php, which I think can effect that) issue that causes this, but seems reasonable to change it for those that aren't willing/able to change the configuration.

The current patch is just changing the difference checking code, not the code that is actually exported as far as I can tell.

ar-jan’s picture

Ah. I guess that the diff check doesn't need to do the replacement, and it needs to happen only for the export itself.

The following seems to work for me. Is it the correct place to be doing the str_replace?

ar-jan’s picture

Status: Needs work » Needs review

Status.

Alan D.’s picture

If on the Diff, previous exports that had \n\r would also be supported; On the export, then only new exports would get it, but happy with either solution, it is trivial to re-roll a feature :)

ar-jan’s picture

I think enforcing LF line endings on export is most beneficial - so we can commit updated features directly to a git repo without converting line endings first.

But we should actually do both I think: with only #13, if you update one line in a configuration form, all form values will again use the CRLF endings from the database, making the UI/Drush diff confusing again.

So I'd vote for #11 + #13. I've tested that both parts work as expected.

hefox’s picture

Features traditionally has not supported not-being-overriden for changing coding style issues like this. Updating features generally means updating features will need to be updated that are using the old coding style to not appear overridden.

Considering this is more a database configuration also, I prefer 13

ar-jan’s picture

Just #13 also seems good to me. Anyone care to set this RTBC?

DamienMcKenna’s picture

FileSize
621 bytes

A minuscule modification from #13 to add a missing period at the end of the comment =)

DamienMcKenna’s picture

Status: Needs review » Needs work

I suggest making this a configuration, so that users can tune it to match their SCM, e.g. it's recommended on Windows to make git convert line endings automatically which would conflict with this (see #1866620: Features rewrites feature files on Windows with Unix line endings, a duplicate).

ann b’s picture

I have a feature where I'm seeing a line break difference. The feature code is using "\n", but the database contains the "\r\n" escape characters. The problem occurs in the strongarm variable "themekey_attributes". Based on what I've read above, the discrepancy could be caused by a number of things (e.g. mysql config, php config, themekey module?).

So my vote is to include #11 in the final patch, so that features diff handles the line break discrepancies. #13 is also helpful to make sure the export always uses the "\n" line break character. And if this were handled in configuration, all the better. Thank you for all the work already done on this issue.

Alan D.’s picture

Status: Needs work » Reviewed & tested by the community

all good 4 years back with the patch :)

Alan D.’s picture

Status: Reviewed & tested by the community » Needs work

Sorry, reverting, I didn't see the last two comments. (should have refreshed before commenting)

Alan D.’s picture

As per Drupal coding standards

Files should be formatted with \n as the line ending (Unix line endings), not \r\n (Windows line endings).

https://www.drupal.org/docs/develop/standards/coding-standards#indenting

donquixote’s picture

@Alan D. (#7)

But the only place where this matters is the diff, so this code will solve peoples issues:

Are you referring to features diff?
In this case I have to disagree. The main problem is in git, if you have to merge files with different line endings, or a small change causes a huge diff.

I just had this for a views feature.

donquixote’s picture

I just had this for a views feature.

I should be more specific:
I had this for custom php fields in views (don't point at me, I did not create those).
A round-trip of drush fr + drush fu added windows line endings to this custom php.