Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

piwik/plugins/Actions/Actions.php contains mixed \r\n and \n #452

Closed
matthijskooijman opened this issue Dec 2, 2008 · 6 comments
Closed
Assignees
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Milestone

Comments

@matthijskooijman
Copy link

The file piwik/plugins/Actions/Actions.php contains a mix of \r\n and \n, which shows up ugly in vim, could mess up in other editors and cause extra junk in submitted patches when editors remove them automatically. Removing the extra \r’s would make the most sense, since that is consistent with all other files.

This sed command should take care of things:

```
sed -i “s/\r//” plugins/Actions/Actions.php
```

@mattab
Copy link
Member

mattab commented Dec 2, 2008

matthijs, I don’t think it’s the only file affected. Please let me know if you have more files affected and what a good long term solution would be; for example I think there are custom SVN tags to set to ensure that this will always be correct.

@matthijskooijman
Copy link
Author

You’re right, there are more files. I thought I did a find + sed to fix all files, but apparently I mistyped something somewhere. I just repeated it, but that also screws up a lot of binary files (images), so I guess you’d need to filter by extension. To get a list of all extensions, I used:

```
$ find -type f |sed “s/.\.\([^.]\)/\1/”|sort |uniq
```

Selecting the text extensions from this list, I’v compiled the following command. This command removes all embedded carriage returns, and also makes each file newline terminated (which makes diffs that add lines to the files not look like they modify the last line in the file).

```
$ for i in css htm html ini js php sh tpl txt; do find -name “*.$i” -print0
| xargs -0 sed -i ‘s/\r//;${/^$/!s/$/\n/}’; done
```

You are right that there is an svn property that handle this kind of thing. You would still need to do the above sanitizing, after you’ve committed that you can set the “svn:eol-style” property on these text files. If you set it to “native”, then the files will be converted to native line endings on checkout out (and back to unix line endings on commit). This allows users on windows to use an editor with windows line endings, and users on *nix to use unix line endings. The following command will set those properties initially:

```
$ for i in css htm html ini js php sh tpl txt; do find -name “*.$i” -print0
| xargs -0 svn propset svn:eol-style native; done
```

Note that you still need to set this property manually when adding new files! Also, don’t forget to teach your editor to put a newline at the end of files (or remember to put it there manually).

@robocoder
Copy link
Contributor

I’ve attach a perl script which takes a slightly different approach from matthijs, by specifically excluding directories and file extensions. As a result, it touches text files with no file extension, in addition to a few missed above (e.g., .script, .properties, and .xml).

Run it from the piwik source directory. Pass the argument —dry-run to get a list of files that would be modified.

You’ll have to manually set the eol-style for misc/generateDoc.bat (i.e., CRLF).

@mattab
Copy link
Member

mattab commented Jan 14, 2009

Fixed in #870. It seems that it simply set the property but didn’t actually change any file content?

@robocoder
Copy link
Contributor

It looks like Trac doesn’t summarize the diffs for newline changes. I did a fresh checkout and ran the script in —dry-run mode, and it didn’t list any files.

Note: for anyone syncing up, “svn up” chokes on a few files due to mixed newlines. Better to do a fresh checkout.

@robocoder
Copy link
Contributor

Attachment: PERL SCRIPT to fix/convert mixed end-of-line characters (newlines vs CR/LF or plain CR), trim trailing whitespace, set svn:eol-style native
fixnewlines.pl

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

No branches or pull requests

3 participants