Summary: | Simplify StringBuilder API/align with makeString by removing appendFixed* functions and using FormatNumber struct instead | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||||||||||||
Component: | New Bugs | Assignee: | Sam Weinig <sam> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, alecflett, beidson, benjamin, berto, calvaris, cdumez, cgarcia, cmarcelo, commit-queue, darin, dbates, dino, eric.carlson, esprehn+autocc, ews-watchlist, fmalita, fred.wang, glenn, gustavo, gyuyoung.kim, jamesr, japhet, jer.noble, jsbell, kangil.han, keith_miller, kondapallykalyan, luiz, macpherson, mark.lam, menard, mifenton, mmaxfield, msaboff, pdr, philipj, saam, sabouhallawa, schenney, sergio, simon.fraser, tonikitoo, toyoshim, tzagallo, webkit-bug-importer, yutak | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Sam Weinig
2019-12-31 20:01:01 PST
The proposed changes are: - StringBuilder::appendNumber(...) -> replaced with StringBuilder::append(...) - StringBuilder::appendFixedPrecisionNumber(...) -> replaced with StringBuilder::append(FormattedNumber::fixedPrecision(...) - StringBuilder::appendFixedWidthNumber(...) -> replaced with StringBuilder::append(FormattedNumber::fixedWidth(...) with the goal of simplifying StringBuilder's API and unifying it more with how makeString(...) works. Created attachment 386573 [details]
Patch
Created attachment 386579 [details]
Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API Created attachment 386582 [details]
Patch
I think I might scale this back to just the fixed width/precision appends, since I forgot about the UChar vs. uint16_t ambiguity issue, that I am still not sure how best to deal with. In the latest patch, I casted the one uint16_t I knew about, SecurityOriginData's port, to uint32_t, but that doesn't feel like a good solution. Perhaps something like a ForceFormatAsNumber(...) wrapper? I wonder if we could re-define UChar in WebCore to be a "strong typedef", (e.g. struct UChar { explicit UChar(); ... private: uint16_t value; }). Would require a lot of changes when dealing with non-WebCore uses... Not sure it would be worth it. Created attachment 386583 [details]
Patch
Created attachment 386584 [details]
Patch
Created attachment 386591 [details]
Patch
Comment on attachment 386591 [details] Patch Clearing flags on attachment: 386591 Committed r253975: <https://trac.webkit.org/changeset/253975> All reviewed patches have been landed. Closing bug. |