773151 - Rename nsCAutoString to nsAutoCString

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Randell Jesup [:jesup] (needinfo me)]

Convert nsCAutoString to nsAutoCString, to make it in line with how we name other classes/etc.

[23:20]	njn	wishes nsCAutoString was called nsAutoCString
[23:25]	gavin	too
[23:26]	roc	fix it
[23:28]	njn	roc: MXR says "Too many hits, displaying the first 1000"
[23:28]	njn	sighs
[23:31]	roc	search-and-replace
[23:34]	darktrojan	sed
[23:35]	cpearce	FTW!

[23:46]	jesup	roc: Wonder how well the tree will compile after this: 'chfiles | xargs sed -e "s/nsCAutoString/nsAutoCString/" --in-place' (and the same with ifiles). (chfiles - finds all .c/cpp/c/h files; ifiles finds idl files)
[23:47]	roc	something like that
[23:47]	jcranmer	jesup: you'd have to apply it to comm-central
[23:47]	khuey	why are we renaming this?
[23:47]	jcranmer	or, more likely, put a typedef in an exported header so other projects won't immediately break

[23:50]	roc	khuey: because nsAutoFooBar is common, and nsFooAutoBar is not

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: convert nsCAutoString to nsAutoCString

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

Attachment: provide back-compatibility for (source) code using nsCAutoString

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 641359 [details] [diff] [review]
convert nsCAutoString to nsAutoCString

bitrot on this is likely, but it's easy to recreate

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 641360 [details] [diff] [review]
provide back-compatibility for (source) code using nsCAutoString

Untested - to keep comm-central and other codebases happy (if something more is needed, let me know.  Seamonkey would be another likely impactee.)

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

https://tbpl.mozilla.org/?tree=Try&rev=b1edf73ead5e

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Yeah, we should have done this as a followup to bug 69873. :-)

Comment 7

[Nicholas Nethercote [inactive]]

Better late than never!

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Documentation changes will be needed if we take this!

Comment 9

[Justin Lebar (not reading bugmail)]

FWIW, I am totally in favor of this change.  And "nsAutoCString" is the same length as "nsCAutoString", so we don't have to worry about breaking indentation.

But I'm worried about leaving the |#define nsCAutoString nsAutoCString| in there, because people will continue to (accidentally) use nsCAutoString.  Can we restrict the #define to non-Mozilla-core code?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

I'd love to do so; if someone can tell me how.  Or throw (compile-time) warnings when someone uses it.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

You could put the compatibility define at the end of xpcom/glue/nsStringAPI.h, which is a header beginning with:

#ifdef MOZILLA_INTERNAL_API
#error nsStringAPI.h is only usable from non-MOZILLA_INTERNAL_API code!
#endif

(Most of the internal string headers begin with the opposite.)

Comment 12

[Benjamin Smedberg]

Comment on attachment 641359 [details] [diff] [review]
convert nsCAutoString to nsAutoCString

This sounds good, but I think you should announce this in the newsgroups first and inform people how to update their in-progress patches with sed.

Comment 13

[Benjamin Smedberg]

Comment on attachment 641360 [details] [diff] [review]
provide back-compatibility for (source) code using nsCAutoString

This provides back-compatibility for internal-api consumers, which I'm not sure we want. We should definitely have either a typedef or a #define for the external-api (nsStringAPI.h). Are you planning on making this temporary while comm-central and any in-progress patches/feature branches get updated?

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't think internal-API consumers using nsCAutoString is too likely to be a problem, since most people are just going to copy their definition from somewhere else. But comment #11 makes sense.

Comment 15

[Randell Jesup [:jesup] (needinfo me)]

Attachment: provide back-compatibility for (source) code using nsCAutoString (updated)

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 641743 [details] [diff] [review]
provide back-compatibility for (source) code using nsCAutoString (updated)

Updated per comment 11; using typedef as well

Comment 17

[Randell Jesup [:jesup] (needinfo me)]

Attachment: provide back-compatibility for (source) code using nsCAutoString (updated)

Comment 18

[Randell Jesup [:jesup] (needinfo me)]

Ok, since we are doing an uplift, I'm preparing an updated masschange patch to land this...

Comment 19

[Randell Jesup [:jesup] (needinfo me)]

Attachment: convert nsCAutoString to nsAutoCString

Comment 20

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 655580 [details] [diff] [review]
convert nsCAutoString to nsAutoCString

may have been corrupted by an interrupted upload

Comment 21

[Randell Jesup [:jesup] (needinfo me)]

Attachment: convert nsCAutoString to nsAutoCString

patch is the result of:  chfiles | xargs sed -e 's/nsCAutoString/nsAutoCString/' --in-place

Comment 22

[Randell Jesup [:jesup] (needinfo me)]

https://tbpl.mozilla.org/?tree=Try&rev=c175647627b8

To convert all the patch queues in a repo, do this:
WARNING: back up or push your patches before doing this!  YOU HAVE BEEN WARNED!
(ok, overly melodramatic, but please don't risk losing data; you probably can just do "cp -a .hg/patches* /tmp/backup_patches" for a simple backup)

find .hg -path ".hg/patches*" -type f -print | xargs sed -e 's/nsCAutoString/nsAutoCString/' --in-place

Comment 23

[Benjamin Smedberg]

Comment on attachment 655581 [details] [diff] [review]
convert nsCAutoString to nsAutoCString

The script does its thing, I can't really review this anyway.

Comment 24

[Randell Jesup [:jesup] (needinfo me)]

Here's the more detailed and pedantic way to update patches and repos I posted on m.dev.platform:

If you want to convert directories of patches:

# NOTE: for *BSD or Mac, use -i instead of --in-place
# BACK UP YOUR PATCHES FIRST!  (or have them under repository control)

hg qpop -a
# Pick up the nsAutoCString changes
hg pull -u  
cd .hg/patches
# repeat for patches-whatever for each qqueue you have
# You could wrap this in something that finds all your patches*
# directories and runs this.
find . -not -name 'series' -and -not -name 'status' -and -not -name ".*" -print | xargs sed -e "s/nsCAutoString/nsAutoCString/g" --in-place 


For codebases: to generate a patch to convert them:

# you may need to tweak this find command....  in particular, excluding
# object directories!!!  We assume they're named obj*
find -name '.hg' -prune , -name 'obj*' -prune , -type f -and -not -name '*~' -and -not -name ".*" -print | xargs sed -e "s/nsCAutoString/nsAutoCString/g" --in-place 

Or, if you're paranoid about hitting binaries/html/etc:

# you may need to tweak this find command....  in particular, excluding
# object directories!!!  We assume they're named obj*
find -name '.hg' -prune , -name 'obj*' -prune , \( -name "*.c" -o -name "*.cpp" -o -name "*.h" -o -name "*.cc" -o -name "*.mm" \) -type f -print | xargs sed -e "s/nsCAutoString/nsAutoCString/g" --in-place 

# I used something like this to convert inbound to create the patch

If there are errors above, my apologies - I did try these out (but only
on a Fedora system).

Comment 25

[Andrew McCreight [:mccr8]]

It might be easiest for everybody if you also took one of Ehsan's bash scripts and modified it for usage, and uploaded it somewhere.  It has various nice features like checking that you have popped all of your patches, and we've used variations of that script multiple times before so we know what to expect.  Thanks.

Comment 26

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 655581 [details] [diff] [review]
convert nsCAutoString to nsAutoCString

Run with the wrong script; the right one is the one I just posted for converting a repo (which was in .platform, but not here before).  Also, the patch itself isn't interesting to have on the bug, as the script really needs to be run on a closed tree and then checked in.

Comment 27

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Script to convert a repo based on ehsan's

I ran this script and diffed the resultant patch against the one I had before - no differences.

Comment 28

[Randell Jesup [:jesup] (needinfo me)]

Attachment: unbitrot patches in patch queues

Modified from Ehsan's bitrot patch - main change is to have it function on *all* patch queues, not just the default .hg/patches queue

Tested on a local repo with 10 queues

Comment 29

[Randell Jesup [:jesup] (needinfo me)]

Per discussion with joduinn, targeted to be done during the Oct 8th migration/closure (see blocks above).

Comment 30

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

(In reply to Randell Jesup [:jesup] from comment #29)
> Per discussion with joduinn, targeted to be done during the Oct 8th
> migration/closure (see blocks above).

After further discussions, jesup proposed in today's platform mtg that he would do this in the coming week or so, during "a lull in checkins", instead of waiting for next migration. Most likely a Friday evening or sometime over a weekend. More info as we have it.

Leaving link to bug#785991, until new date is confirmed.

Comment 31

[Mark Banner (:standard8)]

Randell, if we want to avoid landing the typedef due to comm-central, I'm quite happy to see if we can get someone to sync up with you on the landing and just do both repos at the same time.

Comment 32

[Randell Jesup [:jesup] (needinfo me)]

Plan is to land late this evening or Saturday (or earlier Sunday so as not to affect New Zealand employees).  Removing link.

Agreed, we should do both repos together.  Mark, what's required and when would be a good time for you this weekend?

Comment 33

[Randell Jesup [:jesup] (needinfo me)]

m-c: http://hg.mozilla.org/mozilla-central/rev/3d9424eb6eb4
comm-central: http://hg.mozilla.org/integration/mozilla-inbound/rev/3d9424eb6eb4
compat patch (inbound) https://hg.mozilla.org/integration/mozilla-inbound/rev/cb033f52d08f

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cb033f52d08f