Bug 205671

Summary: Simplify StringBuilder API/align with makeString by removing appendFixed* functions and using FormatNumber struct instead
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: 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 Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Sam Weinig 2019-12-31 20:01:01 PST
Remove numeric append functions from StringBuilder now that append can is capable of handling them
Comment 1 Sam Weinig 2019-12-31 20:06:13 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.
Comment 2 Sam Weinig 2019-12-31 20:18:41 PST Comment hidden (obsolete)
Comment 3 Sam Weinig 2020-01-01 09:01:41 PST Comment hidden (obsolete)
Comment 4 EWS Watchlist 2020-01-01 09:02:25 PST Comment hidden (obsolete)
Comment 5 Sam Weinig 2020-01-01 11:25:46 PST Comment hidden (obsolete)
Comment 6 Sam Weinig 2020-01-01 11:41:53 PST
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.
Comment 7 Sam Weinig 2020-01-01 11:43:36 PST Comment hidden (obsolete)
Comment 8 Sam Weinig 2020-01-01 11:44:50 PST Comment hidden (obsolete)
Comment 9 Sam Weinig 2020-01-01 16:52:34 PST
Created attachment 386591 [details]
Patch
Comment 10 WebKit Commit Bot 2020-01-02 11:32:05 PST
Comment on attachment 386591 [details]
Patch

Clearing flags on attachment: 386591

Committed r253975: <https://trac.webkit.org/changeset/253975>
Comment 11 WebKit Commit Bot 2020-01-02 11:32:07 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Radar WebKit Bug Importer 2020-01-02 11:33:17 PST
<rdar://problem/58280185>