These references are in comments. Simply remove 'nsCRT::' part.

(In reply to Serge Gautherie (:sgautherie) from comment #0) > These references are in comments. > > Simply remove 'nsCRT::' part. As per lines from the URL, the nsCRT:: part's removed

(In reply to Shriram (irc: Mavericks) from comment #1) > As per lines from the URL, the nsCRT:: part's removed I am not sure what you meant...

(In reply to Serge Gautherie (:sgautherie) from comment #2) > (In reply to Shriram (irc: Mavericks) from comment #1) > > As per lines from the URL, the nsCRT:: part's removed > > I am not sure what you meant... AS per URL @ http://mxr.mozilla.org/comm-central/search?string=nsCRT%3A%3Astrlen&case=1&find=%2FnsImapServerResponseParser\.cpp%24 the two matching lines don't have nsCRT:: part anymore

(In reply to Shriram (irc: Mavericks) from comment #3) > the two matching lines don't have nsCRT:: part anymore Well, I still see them: { /mailnews/imap/src/nsImapServerResponseParser.cpp * line 2589 -- char *start = partNumber+5, *end = partNumber+5; // 5 == nsCRT::strlen("BODY[") * line 3136 -- //fCurrentTokenPlaceHolder = fLineOfTokens + nsCRT::strlen(fCurrentLine) - (charsReadSoFar - numberOfCharsInThisChunk); } If not, there would be no more matching lines...

Oh, yeah I see them but they're commented, I thought it's left in comments for some reason. So, it needs to be removed as well. Would that be sufficient?

Yes, my statement 'wo matching lines don't have nsCRT:: part anymore' is incorrect. It should've been 'two matching lines have the nsCRT:: part commented' instead.

Well, did you miss comment 0?

(In reply to Serge Gautherie (:sgautherie) from comment #7) > Well, did you miss comment 0? I can't believe I missed it. Sheesh!

Comment on attachment 598632 [details] [diff] [review] Patch that removes nsCRT:: in two matching lines as per URL field in bug Review of attachment 598632 [details] [diff] [review]: ----------------------------------------------------------------- ::: mailnews/imap/src/nsImapServerResponseParser.cpp @@ +2585,5 @@ > { > char *partNumber = PL_strdup(fNextToken); > if (partNumber) > { > + char *start = partNumber+5, *end = partNumber+5; // 5 == strlen("BODY[") While here, add a space around both '+'. @@ +3132,5 @@ > if (charsReadSoFar > numberOfCharsInThisChunk) > { > // move the lexical analyzer state to the end of this message because this message > // fetch ends in the middle of this line. > + //fCurrentTokenPlaceHolder = fLineOfTokens + strlen(fCurrentLine) - (charsReadSoFar - numberOfCharsInThisChunk); While here, add a space after '//'.

Comment on attachment 598652 [details] [diff] [review] Patch that adds spaces at two matching lines as per URL field in bug [= Diff over previous patch only] thx for the patch - since you're only changing formatting, we might as well fix it all the way: - char *start = partNumber+5, *end = partNumber+5; // 5 == strlen("BODY[") + char *start = partNumber + 5, *end = partNumber + 5; // 5 == strlen("BODY[") please remove all but one of the spaces before the // and turn this into a two line comment, or better yet, just remove the comment entirely: - //fCurrentTokenPlaceHolder = fLineOfTokens + strlen(fCurrentLine) - (charsReadSoFar - numberOfCharsInThisChunk); + // fCurrentTokenPlaceHolder = fLineOfTokens + strlen(fCurrentLine) - (charsReadSoFar - numberOfCharsInThisChunk);

It replaces nsCRT as per title of the bug including formatting. Latest patch takes care of formatting issue in your comment too.

Comment on attachment 599719 [details] [diff] [review] Patch that fixes formatting mentioned in last comment [= Diff over previous patch only] Good, but merge this diff with the previous patch.

Comment on attachment 600633 [details] [diff] [review] Patch that includes all relevant changes made as per comments looks fine, thx.

To ease c-n, can you attach a patch with an explicit comment, not "imported patch bug-715938-fix". http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed

(In reply to Serge Gautherie (:sgautherie) from comment #18) > To ease c-n, can you attach a patch with an explicit comment, not "imported > patch bug-715938-fix". > http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed Would an explicit comment like 'Patch for bug 715938' suffice?

(In reply to Shriram (irc: Mavericks) from comment #19) > Would an explicit comment like 'Patch for bug 715938' suffice? No, something more like "Bug 715938. Remove last 2 references to nsCRT::strlen(), in Imap code.". To which you can add "f=sgautherie r=dbienvenu."

As per 6.2 - add/change a message in http://mercurial.selenic.com/wiki/MqTutorial, I did qrefresh -m "Bug 715938: contains all relevant changes made in comments" before producing the patch. I saw message after -m in the summary when I did 'hg out' I'm surprised "imported bug-715938-fix" wasn't replaced it. I know that this is not what you wrote in last comment. So, should I manually edit the patch and replace line with what you wrote?

(In reply to Shriram (irc: Mavericks) from comment #22) > Created attachment 600738 [details] [diff] [review] > Patch that adds an explicit statement as per last comment The included comment is perfect ;-) Thanks for fixing this bug! NB: In future bugs, what you write in attachment description should usually be in bug comment. And attachment description should instead tell what the patch is actually doing (more like the included comment).

> NB: In future bugs, what you write in attachment description should usually > be in bug comment. And attachment description should instead tell what the > patch is actually doing (more like the included comment). Thanks for the advice Serge. I shall keep that in mind.

V.Fixed, per MXR.