https://searchfox.org/comm-central/search?q=formatStringFrom&case=false&regexp=false&path=*.cpp

About 40 files affected.

This fixed mailnews/addrbook/ - mach build comm/mailnews/addrbook/

More to come, of course.

This wasn't so much fun to write and it won't be fun to review. Let me know if I should find a TB reviewer. Some notes:

1. Wherever the array isn't sized by initialising it, I used AutoTArray<nsString, NN>. I didn't see a pattern in D34196, there is AutoTArray<nsString, 1> strings = {path}; but also nsTArray<nsString> formatStrings = {mHostPort};.
2. I used nsTArray<nsString> when initialising the array.
3. In some cases, strings are passed in as raw strings or nsAString&. I used nsString(xx) in this case to stuff them in to the format strings. Look for nsString( in the patch.
4. I tried to remove as many "...toUTF16" conversions into temporary variables as possible.
5. I didn't do the JS part yet.

Removed more temporary variables.

Wherever the array isn't sized by initialising it, I used AutoTArray<nsString, NN>. I didn't see a pattern in D34196, there is AutoTArray<nsString, 1> strings = {path}; but also nsTArray<nsString> formatStrings = {mHostPort};.

Should have been AutoTArray consistently; the nsTArray there is an oversight.

I used nsTArray<nsString> when initialising the array.

That's fine, but does an extra heap-allocation compared to using AutoTArray. Maybe it doesn't matter in practice, but it was easier for me to do the faster thing instead of analyzing all the callsites.

I didn't do the JS part yet.

The nice thing with the JS part is that if the C++ part lands without the JS part that's not a problem: the JS part will just be passing an extra arg for the length which will get ignored...

Oh, I did use nsTArray instead of AutoTArray in the one place where I std::move things, because nsTArray can be moved cheaply while AutoTArray of course cannot.

Thanks for the comment, Boris. I'll make it AutoTArray everywhere. We actually are in a separate bug here, I kept out of your bug 1557793 since I knew that this would create some noise :-)

One last question, is there a better way to do this:

         const char16_t delimiter[2] = {(char16_t)m_hierarchyDelimiter, '\0'};
-        const char16_t *formatStrings[] = {delimiter};
+        AutoTArray<nsString, 1> formatStrings = {nsDependentString(delimiter)};

EDIT: One very last question: How to deal with strings coming in as nsAString, like so:

-    const char16_t *formatStrings[] = {aAccountname.BeginReading()};
+    AutoTArray<nsString, 1> formatStrings = {PromiseFlatString(aAccountname)};

First reviewer wins. Note: This will land as bustage-fix as soon as bug 1557793 lands.

Another tweak.

One last question, is there a better way to do this:

  AutoTArray<nsString, 1> formatStrings;
  formatString.AppendElement(nsDependentSubstring(&m_hierarchyDelimiter, 1));

ought to work.

EDIT: One very last question: How to deal with strings coming in as nsAString, like so:

  AutoTArray<nsString, 1> formatStrings;
  formatString.AppendElement(aAccountname);

though you could also do:

    AutoTArray<nsString, 1> formatStrings = {nsString(aAccountname)};

and it's probably equivalent in terms of the actual code that runs.

Thanks, Boris. The first solution doesn't work since m_hierarchyDelimiter is just a char*.

Final string tweak, first reviewer wins. Bug 1557793 has all the reviews and will land soon.

Sigh, there go another 100KB which will be stored for eternity. Forgot to refresh.

Thanks, Boris. The first solution doesn't work since m_hierarchyDelimiter is just a char*.

In that case, just:

  AutoTArray<nsString, 1> formatStrings;
  formatString.AppendElement()->Append(m_hierarchyDelimiter);

should work. (I assume you meant it's a char, not a char *.)

We still need to clean up about 25 .js files:
https://searchfox.org/comm-central/search?q=formatStringFrom&case=false&regexp=false&path=*.js*

Thanks.

(In reply to Geoff Lankow (:darktrojan) from comment #21)

Missed this bit?

I'm surprised that compiled.

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/285cab894a6e
Follow-up: Address review comments. r=me
https://hg.mozilla.org/comm-central/rev/f7a7d54671d6
Port bug 1557793: Adapt to array changes in nsIStringBundle.formatStringFromName (JS changes). r=jorgk