search

alohaeditor / Aloha-Editor

Aloha Editor is a JavaScript content editing library
http://www.alohaeditor.org
Other
2.47k stars 535 forks source link

Improving code whitespace consistency #610

Closed wimleers closed 12 years ago

wimleers commented 12 years ago

Many big open source projects, including Drupal, specify very precise expectations about the code: coding standards. They even specify expectations about whitespace:

Files should be formatted with \n as the line ending (Unix line endings), not \r\n (Windows line endings). All text files should end in a single newline (\n). This avoids the verbose "\ No newline at end of file" patch warning and makes patches easier to read since it's clearer what is being changed when lines are added to the end of a file.

Conforming to these coding standards prevents collaboration issues.

With the following ~/.gitattributes file (which is recommended by GitHub):

# Automatically normalize line endings for all text-based files
* text=auto

I get the following automatic changes performed by git:

wim.leers at wimleers-acquia in ~/Work/Aloha-Editor on dev*
$ git d
warning: CRLF will be replaced by LF in src/lib/aloha/markup.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/aloha/rangy-core.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/aloha/repositorymanager.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/aloha/selection.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/util/lang.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/util/position.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/vendor/jquery-1.5.1.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/common/contenthandler/lib/blockelementcontenthandler.js.
The file will have its original line endings in your working directory.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/util/lang.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/util/position.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/vendor/jquery-1.5.1.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/common/contenthandler/lib/blockelementcontenthandler.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/common/contenthandler/lib/genericcontenthandler.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/common/link/css/link.css.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/common/undo/lib/undo-plugin.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/extra/googletranslate/css/googletranslate.css.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/extra/googletranslate/lib/googletranslate-plugin.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/extra/wai-lang/lib/iso639-1-de.json.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/extra/wai-lang/lib/iso639-1-en.json.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/extra/wai-lang/lib/iso639-2-de.json.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/plugins/extra/wai-lang/lib/iso639-2-en.json.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/test/manual/testbox.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/test/unit/contenthandler/example.htm.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/test/unit/pluginapitests.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/test/unit/testutils.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/test/vendor/jquery-1.5.1.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in vendor/jquery-1.5.1.js.
The file will have its original line endings in your working directory.
warning: CRLF will be replaced by LF in src/lib/aloha/markup.js.
The file will have its original line endings in your working directory.
 src/lib/aloha/markup.js                                             | 2440 ++++++++++++------------
 src/lib/aloha/rangy-core.js                                         | 6508 +++++++++++++++++++++++++++++++--------------------------------
 src/lib/aloha/repositorymanager.js                                  | 1106 +++++------
 src/lib/aloha/selection.js                                          | 4280 +++++++++++++++++++++---------------------
 src/lib/util/lang.js                                                |  122 +-
 src/lib/util/position.js                                            |  306 +--
 src/lib/vendor/jquery-1.5.1.js                                      |16632 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------
 src/plugins/common/contenthandler/lib/blockelementcontenthandler.js |  174 +-
 src/plugins/common/contenthandler/lib/genericcontenthandler.js      |  514 ++---
 src/plugins/common/link/css/link.css                                |  110 +-
 src/plugins/common/undo/lib/undo-plugin.js                          |  258 +--
 src/plugins/extra/googletranslate/css/googletranslate.css           |   26 +-
 src/plugins/extra/googletranslate/lib/googletranslate-plugin.js     |  346 ++--
 src/plugins/extra/wai-lang/lib/iso639-1-de.json                     | 1172 ++++++------
 src/plugins/extra/wai-lang/lib/iso639-1-en.json                     | 1172 ++++++------
 src/plugins/extra/wai-lang/lib/iso639-2-de.json                     | 3122 +++++++++++++++----------------
 src/plugins/extra/wai-lang/lib/iso639-2-en.json                     | 3122 +++++++++++++++----------------
 src/test/manual/testbox.js                                          |  650 +++----
 src/test/unit/contenthandler/example.htm                            |    4 +-
 src/test/unit/pluginapitests.js                                     |  410 ++--
 src/test/unit/testutils.js                                          |  950 +++++-----
 src/test/vendor/jquery-1.5.1.js                                     |16632 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------
 vendor/jquery-1.5.1.js                                              |16632 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------------
 23 files changed, 38344 insertions(+), 38344 deletions(-)

Would you be willing to to solve this problem? It's definitely a problem other contributors will run in to at some point.

P.S.: the following ~/.gitconfig additions ensure even cleaner code:

[core]
    # Treat spaces before tabs, and all kinds of trailing whitespace as an error
    whitespace = space-before-tab,trailing-space
deliminator commented 12 years ago

Thank you for this excellent suggestion :-) I added the .gitattributes file and converted existing CRLF -> LF. The .gitconfig would be interesting too, but I can't seem to get it to work....

wimleers commented 12 years ago

You may need to perform the steps listed under "Re-normalizing a repo" on GitHub's recommendations page I linked to earlier.

Does that help?

wimleers commented 12 years ago

Oh, also, you can reference GitHub issues in commit messages by prepending # to the issue number or pull request number.

So e.g. Convert CRLF to LF. Fixes #610. would've automatically closed this issue :)

See https://github.com/blog/831-issues-2-0-the-next-generation, "Commits + Issues" heading.

deliminator commented 12 years ago

I did perform the re-normalizing step for line endings.

The whitespace = space-before-tab... setting doesn't work like that it seems. I think what we need is some tool that fixes existing whitespace problems.

Also, thank you for the info, I will reference commits in the future.

wimleers commented 12 years ago

Maybe it's a git version issue?

$ git --version
git version 1.7.10.2
wimleers commented 12 years ago

I think this issue should be closed; I'd say #739 is the successor.