-
-
Notifications
You must be signed in to change notification settings - Fork 746
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
Don't barf on readonly files if contents haven't changed #95
Don't barf on readonly files if contents haven't changed #95
Conversation
Oh my god what is happening with this formatting :P |
I think that's called the lack of a rebracer file for your weird settings :P Most C# devs don't have it set like you do :) |
if (target.IsReadOnly) | ||
{ | ||
var contents = File.ReadAllText(target.FullName, Encoding.UTF8); | ||
if (contents != template) |
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.
Do we need more than a straight !=
here? What about line endings? 😱
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.
I thought about it but I'm not sure we want to start taking encoding or line endings into account. For systems that are checking-out readonly, they shouldn't be messing with what was checked in.
I know that TFS in particular will not mess with it; I don't know for sure about Perforce or others. That said, if there's a request for it, then we can handle it then...
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.
Fair enough. Was kind of what I was thinking too, but I thought it was worth at least talking about. 👍
This looks good to me now. Still a couple of minor format things (like the way R# indents chained method calls), but I think that can be cleaned up when merging. @paulcbetts: you happy with this? |
@bennor I'll merge this and fix up the rest of the jacked formatting soon |
Thanks @onovotny! |
👍 |
Fix for #94