1908395 - Rename js::StringBuffer

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[Jan de Mooij [:jandem]]

We're now using both js::StringBuffer and mozilla::StringBuffer in SpiderMonkey and it's a little confusing.

The name StringBuffer probably applies better to the MFBT class and we could rename js::StringBuffer to js::StringBuilder or something.

Comment 1

[Steve Fink [:sfink] [:s:]]

Heh.

js::StringBuilder would conflict less, but note that we already have js::JSONFullParseHandler::StringBuilder, js::JSONSyntaxParseHandler::StringBuilder, and js::JSStringBuilder already. That last one also happens to be a subclass of js::StringBuffer, and I don't think we want class JSStringBuilder : public StringBuilder!

Probably we have at least one too many of these related things. It looks like nbp would put Sprinter in this set as well.

(And I won't mention namespace { class StringBuilder } — a StringBuilder in an anonymous namespace in nsContentUtils.cpp.)

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Steve Fink [:sfink] [:s:] from comment #1)

That last one also happens to be a subclass of js::StringBuffer, and I don't think we want class JSStringBuilder : public StringBuilder!

I was thinking of that as a feature: "build a string" vs "build a JSString", but true, it could also be a bit confusing.

Comment 3

[Steve Fink [:sfink] [:s:]]

(In reply to Jan de Mooij [:jandem] from comment #2)

(In reply to Steve Fink [:sfink] [:s:] from comment #1)

That last one also happens to be a subclass of js::StringBuffer, and I don't think we want class JSStringBuilder : public StringBuilder!

I was thinking of that as a feature: "build a string" vs "build a JSString", but true, it could also be a bit confusing.

Oh darn, I totally missed that. I was reading JSStringBuilder as JS_StringBuilder, not JSString_Builder. Thinking of it as the latter... you're right, that's kind of nice!

Comment 4

[Nicolas B. Pierron [:nbp]]

I will note that Sprinter also has a JSprinter variant which provide the correct allocation buffer for the string.

While Sprinter is restricted to error messages and debugging, it does serve the same goal of building a string.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1908395 part 1 - Rename js::StringBuffer to js::StringBuilder. r?arai!

We now also use mozilla::StringBuffer in SpiderMonkey so this is less confusing.

There is already a JSStringBuilder class that inherits from StringBuilder
so it's also a little more consistent.

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1908395 part 2 - Rename testStringBuffer.cpp to testStringBuilder.cpp. r?arai!

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1908395 part 3 - Rename StringBuffer.js test to StringBuilder.js. r?arai!

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1908395 part 4 - Rename StringBuffer.h/cpp to StringBuilder.h/cpp. r?arai!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2178f64ae9e
part 1 - Rename js::StringBuffer to js::StringBuilder. r=arai
https://hg.mozilla.org/integration/autoland/rev/f953b35772bd
part 2 - Rename testStringBuffer.cpp to testStringBuilder.cpp. r=arai
https://hg.mozilla.org/integration/autoland/rev/d03272a851c4
part 3 - Rename StringBuffer.js test to StringBuilder.js. r=arai
https://hg.mozilla.org/integration/autoland/rev/9850752a3b55
part 4 - Rename StringBuffer.h/cpp to StringBuilder.h/cpp. r=arai

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/b2178f64ae9e
https://hg.mozilla.org/mozilla-central/rev/f953b35772bd
https://hg.mozilla.org/mozilla-central/rev/d03272a851c4
https://hg.mozilla.org/mozilla-central/rev/9850752a3b55