Update development section of the doc

[loicraux]

1. The Git configuration should be local, not global
2. Cloning with https rather than ssh, it requires no setup ?

[HamletDRC]

loic, are you sure that local will work? You need to have this line endings set when you clone the repo. So there is no local config at that point. I think only global will work in this case.

[loicraux]

First, I actually did not follow the steps for development, since I did not want to override my default global git settings. I think we should really remove these git config --global lines for the README.md. This is the original purpose of this ticket. But these line endings issues are always fun, so I did some further research in my train :)

So what actually happened to me:

• Since my global git setting is set to autocrlf=true, after cloning, all my text files in my working directory consequently had CRLF characters (that is "r\n") as line endings, in particular test-data\NoMultilineStringTestInput.ts
• This particular rule noMultilineStringRule.ts being tested trims the faulty node text this way :
  let firstLine : string = fullText.substring(0, fullText.indexOf('\n'));
• Eventually chai.assert.deepEqual(actual, expected); in TestsHelper.ts fails because the expected string do not have the \r char whereas the actual one has it :)

To fix the Development section of the README.md and get rid of the git config --global commands, one easy way would be to use this command :

git clone [email protected]:... --config core.eol=lf --config core.autocrlf=input

This sets locally these settings before fetching of the remote changes are done. With this command, all files are checked out with LF line endings only, and Git ensures that any potential CRLF line endings will be normalized to LF on checkin to the DB.

Fine. But this is a bit unusual to pass these config settings before cloning or to the clone command itself. This is the old system of Git to handle line endings. We have this new system with the .gitattributes file that overrides any global or local git configuration and forces everybody to use the same git line endings settings, so we should be able to get rid of the --config options commands completely.

* text=auto in our current .gitattributes file set all files with any potential CRLF line endings will be normalized to LF on checkin to the DB, if those files are determined by Git to be text and not binary. This relies on Git's built in binary detection heuristics. But files would be checkout out with CRLF for Windows clients... Not what we want since the unit test would fail.

* text eol=lf instead is better! This attribute forces git to normalize line endings to LF on checkin and prevents conversion to CRLF when the file is checked out for every file that is determined to be text by Git.

Are we eventually ok with a README.md without --config options and .gitattributes file modified to use * eol=lf instead of * text=auto?

No :) The problem with this configuration is the following. If any contributor working with a Windows Editor that uses CRLF line endings modifies test-data\NoMultilineStringTestInput.ts in his working directory so that a CRLF now replaces the previous LF char, and replay the unit tests, he will get a failure of the unit test :(

Many situations to be handled, depending on the text editors, the OSes... What we want is to benefit from LF line endings in Linux workdirs, CRLF line endings in Windows workdirs AND --in any client situation-- LF line endings in our DB repository... And of course the test cases to always pass with uncommitted changes no matter what are your line endings settings...

We can achieve that :) What we should do then imho :

1. The rule noMultilineStringRule.ts can be fixed to use require('os').EOL instead of "\n" to trim the node text (kind of System.separator of Java :)). Or if you don't want to use require('os').EOL, all test input data (all files in folder tests-data) should be considered as binary and not text. This can be told to Git in .gitattributes as well. This way LF will never be turned into CRLF when files in this folder are checked out.

2. The .gitattributes file should contain * text=auto (this is already the case)

3. The Development section of the README.md file should now only contain git clone [email protected]:Microsoft/tslint-microsoft-contrib.git

What is your opinion, especially for point 1) ? Both solutions have pros and cons.. It's weird to set typescript files in tests-data folder as binary files, but also some may not be comfortable with require('os').EOL... Personally I would use require('os').EOL in the implementations of the rules, this can of course be factorized somewhere as a constant.

[HamletDRC]

thank you for the detailed explanation!

I don't care what the solution is as long as there are only LFs in the project. By the way, our own project (sindbad.git) has the same problem. There is a long-standing crlf issue. Send a pull request and I'll merge it.