-
Notifications
You must be signed in to change notification settings - Fork 229
Preserve windows line endings in pubspec.lock if they are already there #2489
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
Preserve windows line endings in pubspec.lock if they are already there #2489
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just nits :D
lib/src/lock_file.dart
Outdated
@@ -98,6 +107,7 @@ class LockFile { | |||
Uri sourceUrl; | |||
if (filePath != null) sourceUrl = p.toUri(filePath); | |||
var parsed = loadYamlNode(contents, sourceUrl: sourceUrl); | |||
final windowsLineEndings = contents.contains('\r\n'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wanted to be a bit more robust we do count(contents, '\r\n') > count(contents, '\n')
, using:
int count(String s, Pattern p) {
int c = 0;
int i = -1;
while ((i = s.indexOf(p, i + 1)) != -1) {
c++;
}
return c;
}
So if there is more CRLF's than LFs we use CRLF, otherwise we use LF.
It's hard to imagine things getting mixed.. But maybe if someone edits the pubspec.lock
or manually resolves a merge conflict? (I suspect most editors would normalize line endings, but just in case it might be nice).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We at least have to make it count(contents, '\r\n') > count(contents, '[^\r]\n')
.
But yes, it would probably be safer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet!
lib/src/entrypoint.dart
Outdated
var lockFilePath = root.path('pubspec.lock'); | ||
writeTextFile(lockFilePath, _lockFile.serialize(root.dir)); | ||
_lockFile = | ||
result.lockFile(windowsLineEndings: lockFile.windowsLineEndings); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you miss something here, didn't you remove the windowsLineEndings
state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes - I think I made a bad merge...
Fixed
Fixes: #2487