Skip to content
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

BOM nuke #9374

Merged
merged 2 commits into from Dec 6, 2020
Merged

BOM nuke #9374

merged 2 commits into from Dec 6, 2020

Conversation

RipleyTom
Copy link
Contributor

Adds /utf-8 option to the vcxprojs and removes all the bom markers in /rpcs3/ subdirectory for .cpp and .h files.
There are 2 commits, one with the actual changes to projects and config and the other with all the bom markers removed.

@@ -1,4 +1,4 @@
#pragma once
#pragma once

Copy link
Contributor

@Megamouse Megamouse Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly this broke some special characters like the microsecond abbreviation. Or maybe it was the addition of BOM. Need to test. Maybe it's currently broken on master anyway

Copy link
Contributor Author

@RipleyTom RipleyTom Dec 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes because we didn't have the /utf-8 option for msvc so it didn't interpret stuff as utf-8 unless it had the bom marker.
Now that the option has been added bom or no bom shouldn't matter but it's better to remove it, mainly because of the gcc bug(where it ignores #pragma once on headers with BOM).

@Nekotekina
Copy link
Member

Make sure you remove BOM only from files which are pure printable ASCII. You can use grep to find such ones.
Otherwise you make a random text editor guess which encoding it's in.

@Nekotekina
Copy link
Member

I forgot, /utf-8 key should be added in .props files as well, and probably cmake.

@Nekotekina
Copy link
Member

Also there was an interesting proposal with git-commit hooks, it may work, but I'd prefer it not only check for the absence of BOM but also scan file for any non-ASCII characters as well.

@RipleyTom
Copy link
Contributor Author

RipleyTom commented Dec 6, 2020

Added to CMakeList.txt and I've changed to using rpcs3_default.props before your comment(and it's used by every .vcxproj from rpcs3 as a baseline, I've checked).

As for scanning for non-ASCII chars with githook it's doable but I don't really see a purpose for it, our print functions handle utf-8 just fine so I'm not sure where it would be an issue if there were one non-ascii char in there.

@Nekotekina
Copy link
Member

Ok, I'll add some BOM later.

@Nekotekina Nekotekina merged commit af8c661 into RPCS3:master Dec 6, 2020
@RipleyTom RipleyTom deleted the bom_nuke branch January 5, 2021 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants