Simplify applying of bold and italic fonts #180
Conversation
| fmt.setFont(_formats[StUnderline].font()); | ||
| fmt.setFontUnderline(_formats[StUnderline].fontUnderline()); | ||
| } else if (_formats[Bold].font().bold()) | ||
| fmt.setFontWeight(QFont::Bold); |
There was a problem hiding this comment.
No longer necessary, because this is already covered by the setFont above.
| @@ -289,7 +289,6 @@ void MarkdownHighlighter::initTextFormats(int defaultFontSize) { | |||
| _formats[H5] = format; | |||
| format.setFontPointSize(defaultFontSize * 1.1); | |||
| _formats[H6] = format; | |||
| format.setFontPointSize(defaultFontSize); | |||
There was a problem hiding this comment.
This was superfluous because the changed format is overridden by the next line.
There was a problem hiding this comment.
Ah, format = QTextCharFormat();, ok
|
It would be hard to guess what side effects that has... |
Yeah, it has the side effect that it might set more font properties apart from just the family and the bold/italic, provided those have been set on the font of the format that is being applied (which, at least in the highlighter constructor, doesn't happen). I thought it was a nice simplification of the code. It actually brings the code back to the state just before b6bea15, with the main difference that it addresses the issues of all the changes that were made afterwards in a much simpler way. |
|
Will this get merged? Simplifying the code seems like a good idea to me |
I still don't want to deal with the regressions in QOwnNotes. 😅 |
Maybe I can deal with them, but did you notice any? |
Usually they would creep up for certain fonts on certain operating systems, so it's hard to test. |
|
And it would need lots of testing to see if all permutations of schema settings still work with different fonts on different operating system when not parts of the font settings are set, but everything... |
|
But even before those tests and the need to rebase this PR on main, because it's far behind it, I already get lots of |
We can actually use QTextCharFormat::setFont, but we need to pass QTextCharFormat::FontPropertiesSpecifiedOnly to avoid overriding all properties.
bjorn
force-pushed
the
simplification
branch
from
837d35e to
668b2ba
Compare
|
@pbek I've rebased the patch. Regarding the warning you're seeing, do you think this is somehow related to this PR? What are you doing to get this warning? |
thank you
When I pull in the PR I get this warning many dozen times when I just start the application... |
We can actually use
QTextCharFormat::setFont, but we need to passQTextCharFormat::FontPropertiesSpecifiedOnlyto avoid overriding all properties.This works fine in Notes, but you may want to make sure this also works in the context of QOwnNotes before merging this.
Small follow-up to #179.