Force LF line endings for .ql, .qll, .qlref, and .dbscheme#106
Conversation
|
Added WIP to prevent merge until all three languages sign off. |
|
I don't think you want to do this to the *.dbscheme files -- you'd trigger the need for upgrade scripts, as their content hash matters. |
|
@pavgust All of the dbscheme files are already LF in the database. I've always had to be careful when I made dbscheme changes to avoid introducing rogue CRLFs. With this change, they'll remain LF in the database and in the working tree. Also, I just added a .editorconfig file to this PR, so that anyone using a supported editor will be much less likely to introduce a CRLF. Finally, we hash the dbscheme with |
|
Ah, good -- apologies, I should have checked whether any dbscheme files actually changed. If not, then forcing their line endings to LF is indeed the right thing to do.
There are at least some code paths that has the dbscheme in an equivalent way, but without access to the repo (and hence without reference to |
jbj
left a comment
jbj
left a comment
There was a problem hiding this comment.
Shouldn't we do this for *.expected and *.qhelp files also? I'm less sure about source files for tests and docs (*.c, *.js, etc.), so maybe we should postpone that until later. I'd also be okay with doing that in a follow-up PR and getting this merged before it conflicts with everything.
| @@ -0,0 +1,2 @@ | |||
| [*.{ql,qll,qlref,dbscheme,}] | |||
There was a problem hiding this comment.
I can't find docs that say a trailing comma is acceptable here. In Bash glob syntax, this would expand to *.ql *.qll *.qlref *.dbscheme *.
There was a problem hiding this comment.
Type. Fixed when I added .qhelp to the list.
|
I've added .qhelp files to the LF-only list. There were only two that had CRLFs. I don't think we should do this for .expected files. We compare these using binary comparison. If someone adds CRLFs in their working tree, and the binary comparison succeeds, then the comparison would fail for everyone else. I'd rather the failure happen on the machine where the CRLF was introduced. For other source file extensions, I'd like to hold off until we can discuss further. I'd be fine with LF for all the .js/.cs/.cpp/.html files, but I don't know if there are any of them that chose their current line endings for specific testing purposes. Also, most of these files aren't part of the distribution that end customers see, so it's less important (although still good) to have consistent line endings. |
Some of the alert suppression tests have CRLF line endings to test that suppression works on Windows, so indeed we'd have to be careful about enforcing LF line endings for tests. Apparently the |
Extract anonymous object creation
Add rule to detect cases where CodeQL default setup could be used instead of advanced setup
…h-state C#: Make `StartsWith` and `EndsWith` sanitizers on normalized paths
5 participants
Added a .gitattributes file to specify that *.ql, *.qll, *.qlref, and *.dbscheme files should have their line endings normalized to LF in the database, and to keep the LF line endings in the working tree even on Windows. Instructions for how to renormalize files after future changes are in a comment .gitattributes.
Renormalized the existing such files in the repo. This consisted of a few .qlref files in C#, three test .ql files that @rdmarsh2 recently modified in JavaScript, and essentially every file that @dave-bartolomeo has ever edited:) I don't expect any of these files to cause merge conflicts with anything currently in flight.
I didn't know which other file extensions we want to force this for. I'm fine with forcing it for all text files, assuming that doesn't break anything.