Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@grokys
Copy link
Contributor

@grokys grokys commented Aug 14, 2017

This was causing #1137; this code was obviously added for a reason in 6b81f3f but I can no longer reproduce VS erroneously signalling a file as changed if we don't add the BOM. Maybe we handle this elsewhere now?

I could do with some 👀 on this @jcansdale as I'm worried I'm missing something.

Fixes #1137.

@grokys grokys requested a review from jcansdale August 14, 2017 11:51
@jcansdale
Copy link
Collaborator

jcansdale commented Aug 16, 2017

I think the potential issue here is when a file does have an explicit BOM. Won't this new code strip it off and the file will now look like it has changed?

IIRC, Git defaults to UTF8 but Visual Studio defaults to Windows-1252.

@jcansdale
Copy link
Collaborator

I can no longer reproduce VS erroneously signalling a file as changed if we don't add the BOM.

The stuff I added to to make sure the temporary file we extract from Git has the same preamble as the working directory file we compare it to. This prevents the diff view showing the ugly warning message. Was this what you means by VS erroneously signalling?

@grokys
Copy link
Contributor Author

grokys commented Aug 16, 2017

I think the potential issue here is when a file does have an explicit BOM. Won't this new code strip it off and the file will now look like it has changed?

This is what i was wondering, and what I was trying to repro but couldn't. I added some test PRs here and here but couldn't get a false changed state. Am I missing anything?

This was causing #1137; it was obviously added for a reason in 6b81f3f but I can no longer reproduce VS erroneously signalling a file as changed if we don't add the BOM. Maybe we handle this elsewhere now.

Fixes #1137.
@grokys grokys force-pushed the fixes/1137-ismodified-incorrect branch from d146494 to ec2ce66 Compare August 16, 2017 13:03
@grokys grokys changed the base branch from release/2.3.0 to master August 16, 2017 13:04
@grokys
Copy link
Contributor Author

grokys commented Aug 16, 2017

Looking further this doesn't seem to be right. The problem seems to be caused by the fact that we add a UTF8 BOM to extracted temp files, which causes IsModified to think the file has been modified.

@grokys grokys closed this Aug 16, 2017
@grokys grokys deleted the fixes/1137-ismodified-incorrect branch August 16, 2017 13:57
@grokys grokys restored the fixes/1137-ismodified-incorrect branch August 16, 2017 13:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants