-
Notifications
You must be signed in to change notification settings - Fork 105
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
Fixed a crash when rendering unsupported characters like "Å". #102
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.
I think this is good; just see my one comment.
src/WpfMath/TexFontInfo.cs
Outdated
@@ -1,4 +1,4 @@ | |||
using System; | |||
using System; |
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.
What's going here? Seems like an identical line, hrmm. Or maybe a BOM was removed?
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 have no idea..
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.
This is what git diff says.
@@ -1,4 +1,4 @@
-<EF><BB><BF>using System;
+using System;^M
I must admit not exactly sure what this means or how i fix it.
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.
Ah, it's a Windows-style line ending (^M
is a carriage return). Maybe this will help: https://superuser.com/questions/194668/grep-to-find-files-that-contain-m-windows-carriage-return. Just amend the commit, of course.
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.
Yeah i figured it out, the file had been saved as UTF-8 instead of UTF-8-BOM. It should be fixed now.
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.
Regarding BOM: we've discussed these a while ago, but nobody was particularly interested, so I single-handedly decided to let the BOM stay as it is. Probably we'll need a small research regarding current behavior of compilers in case we want to remove it. Current "consensus" is that we use BOM (because it helps .NET compilers to work with Unicode in sources), and I'd prefer everyone (incl. myself) to not change that without further discussion.
Maybe just reset it then.
… On 21 Jan 2018, at 18:32, Sverre Skodje ***@***.***> wrote:
@sskodje commented on this pull request.
In src/WpfMath/TexFontInfo.cs <#102 (comment)>:
> @@ -1,4 +1,4 @@
-using System;
+using System;
I have no idea..
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub <#102 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEF3DXvWz05CiLkdD0odFwpFD2ru2P5ks5tM4KzgaJpZM4Rl5ck>.
|
Thank you for the contribution (and thanks to Alexander for initial review!). I'm going to review that (and probably merge) tonight; ETA 12 hours. As a side note: we do care about internationalization, it's just our font doesn't support most of the unicode characters. We want to eventually do something about it. For now, you could use |
Again, sorry for the delay and thank you for the contribution! 👍 |
This fixes a crash when inputting characters or symbols not supported by the font. The issue was discovered using norwegian letters "æ, ø or å".