Bug 205671 - Simplify StringBuilder API/align with makeString by removing appendFixed* functions and using FormatNumber struct instead
Summary: Simplify StringBuilder API/align with makeString by removing appendFixed* fun...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sam Weinig
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-31 20:01 PST by Sam Weinig
Modified: 2020-01-02 11:33 PST (History)
47 users (show)

See Also:


Attachments
Patch (145.83 KB, patch)
2019-12-31 20:18 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (156.58 KB, patch)
2020-01-01 09:01 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (158.14 KB, patch)
2020-01-01 11:25 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (157.40 KB, patch)
2020-01-01 11:43 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (156.65 KB, patch)
2020-01-01 11:44 PST, Sam Weinig
no flags Details | Formatted Diff | Diff
Patch (40.06 KB, patch)
2020-01-01 16:52 PST, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>