-
Notifications
You must be signed in to change notification settings - Fork 34
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
Classifier #10
base: master
Are you sure you want to change the base?
Classifier #10
Conversation
…he "Default" value in fonts and colors will show the wrong color
…assification registry
re: 07cfcd8, the items that aren't currently configurable are that way for a reason. They are either ones that shouldn't really be configurable (bold = bold, italics = italics) or have properties that the Fonts and Colors dialog can't do anything about (font, font size, font characteristics). I don't think they should be exposed, especially since, as you note in #7, this doesn't actually fix the bug. (the "fixes #7" should probably be removed from the description for that reason) If I don't take 07cfcd8, then the only other interesting one is ebec22c. I only have one small nitpicky comment there (capture tokenIndex as (int)tokenType once in GetClassificationTypeForMarkdownToken), and the other comment I had would go away if the other changes aren't merged (mismatches between e.g. UrlInline and InlineUrl), but it seems like a positive change on the whole. I doubt this will make an appreciable performance difference, but the code you wrote is definitely better. |
I Users may still want to customize the colors related to items like bold/italics, which is why I think it makes sense to leave those customizable. I certainly can't see it hurting anything to leave users the option. |
I merged this into my fork because I use a black background in my editor, and I was unable to change the colour of link urls from their default blue, which was far too low in contrast and was unreadable. |
(fixes Block font could do better than Courier New #7)