<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - MathMLMencloseElement style should not depend on rendering"
href="https://bugs.webkit.org/show_bug.cgi?id=155406#c5">Comment # 5</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - MathMLMencloseElement style should not depend on rendering"
href="https://bugs.webkit.org/show_bug.cgi?id=155406">bug 155406</a>
from <span class="vcard"><a class="email" href="mailto:darin@apple.com" title="Darin Adler <darin@apple.com>"> <span class="fn">Darin Adler</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=273889&action=diff" name="attach_273889" title="patch">attachment 273889</a> <a href="attachment.cgi?id=273889&action=edit" title="patch">[details]</a></span>
patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=273889&action=review">https://bugs.webkit.org/attachment.cgi?id=273889&action=review</a>
<span class="quote">> Source/WebCore/mathml/MathMLMencloseElement.cpp:134
> String closingBrace(")", String::ConstructFromLiteral);
> TextRun run(closingBrace);</span >
We do not need a String to construct a TextRun, which takes a StringView. We can write something more like this:
LChar rightParenthesis = ')';
TextRun run({ &rightParenthesis, 1 });
I took the liberty of calling this what the Unicode standard calls it, rather than calling it "closing brace", which seems like it could be the name of some other characters.
<span class="quote">> Source/WebCore/mathml/MathMLMencloseElement.cpp:136
> + const FontCascade& font = parentStyle.fontCascade();
> + padding.appendNumber(font.width(run));</span >
I don’t think we need a local variable when we are just using it once. In fact, I would write this:
LChar rightParenthesis = ')';
padding.appendNumber(parentStyle.fontCascade().width(TextRun{ &rightParenthesis, 1 }));
Might need an extra type name or set of curly brackets in there, but something like that would work. The version with more lines of code seems no more clear to me.
<span class="quote">> Source/WebCore/mathml/MathMLMencloseElement.cpp:143
> +String MathMLMencloseElement::longDivLeftPadding() const</span >
This could return a const String& instead of a String to avoid unneeded reference count churn.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>