<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [INTL] Implement Intl.NumberFormat.prototype.resolvedOptions ()"
href="https://bugs.webkit.org/show_bug.cgi?id=147602#c35">Comment # 35</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [INTL] Implement Intl.NumberFormat.prototype.resolvedOptions ()"
href="https://bugs.webkit.org/show_bug.cgi?id=147602">bug 147602</a>
from <span class="vcard"><a class="email" href="mailto:sukolsak@gmail.com" title="Sukolsak Sakshuwong <sukolsak@gmail.com>"> <span class="fn">Sukolsak Sakshuwong</span></a>
</span></b>
<pre>(In reply to <a href="show_bug.cgi?id=147602#c31">comment #31</a>)
<span class="quote">> Comment on <span class="bz_obsolete"><a href="attachment.cgi?id=270575&action=diff" name="attach_270575" title="Patch">attachment 270575</a> <a href="attachment.cgi?id=270575&action=edit" title="Patch">[details]</a></span>
> Patch
>
> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=270575&action=review">https://bugs.webkit.org/attachment.cgi?id=270575&action=review</a>
>
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:3
> > + * Copyright (C) 2015 Sukolsak Sakshuwong (<a href="mailto:sukolsak@gmail.com">sukolsak@gmail.com</a>)
>
> This year is 2016, not 2015.</span >
Fixed.
<span class="quote">> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:96
> > + ASSERT(currency.length() == 3 && currency.isAllSpecialCharacters<isASCIIAlpha>());
>
> This assertion needs to be isASCIIUpper, not just isASCIIAlpha.
>
> Normally we don’t use && in assertions. Instead we write multiple
> assertions. That way if one clause fires, we know which one it was. In this
> case I think 4 separate assertions would be ideal so we know which one
> failed. 2 would be OK, though.</span >
Done.
<span class="quote">> Could also just make computeCurrencySortKey a function template instead of
> writing it twice, except for the length check in the assertion, but there
> are tricks for that too. For example, StringView(currency).length() should
> work with both const String& and const char*.</span >
We have to do ASSERT(isAllSpecialCharacters<isASCIIUpper>(currency, 3)) too. Is there a way to write it so that it works with both const String& and const char*?
<span class="quote">> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:240
> > + currency = currency.upper();
>
> This should be convertToASCIIUppercase, not upper, for better efficiency,
> and because I am working hard to eliminate upper entirely except for
> possibly one or two call sites.</span >
Done.</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>