Closed
Bug 227633
Opened 21 years ago
Closed 12 years ago
Replace 0x0a by nsCRT::LF, 0x0d by nsCRT::CR
Categories
(MailNews Core :: Backend, defect, P4)
MailNews Core
Backend
Tracking
(Not tracked)
VERIFIED
FIXED
Thunderbird 13.0
People
(Reporter: sgautherie, Assigned: sgautherie)
References
Details
Attachments
(1 file, 3 obsolete files)
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6a) Gecko/20031030 Build Identifier: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.6a) Gecko/20031030 Spun off from bug 119441 comment 25: { instead of 0x0A and 0x0D, consider using nsCRT::CR and nsCRT::LF, to make the code more readable. (if you want to fix the rest of the import code, that would be great, but that can be spun off to a new bug.) } Reproducible: Always Steps to Reproduce:
Assignee | ||
Updated•21 years ago
|
Assignee: general → gautheri
Assignee | ||
Updated•21 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Updated•21 years ago
|
Summary: (Code cleanup) Replace 0x0a/0x0d by nsCRT::CR/nsCRT::LF → (Code cleanup) Replace 0x0a by nsCRT::LF, and 0x0d by nsCRT::CR
Assignee | ||
Comment 1•21 years ago
|
||
This does 1+ minor code optimizations too. 'review only': Since all theses files are using Tabs, the full patch has some more Tab->Space conversions, only to better see the if/while/... blocks (involved in this patch).
Assignee | ||
Updated•21 years ago
|
Attachment #136934 -
Flags: review?(dwitte)
Assignee | ||
Updated•21 years ago
|
Attachment #136934 -
Attachment description: (Av1) Fixes most '/mailnews/' occurences (review only) → (Av1_Bw) Fixes most '/mailnews/' occurences (review only)
Comment 2•21 years ago
|
||
Comment on attachment 136934 [details] [diff] [review] (Av1_Bw) Fixes most '/mailnews/' '0x0?' occurences (review only) >- else { >+ else > m_pBuffer[offset] = *pData; >- } >+ Also i'm no expert at coding and C(++), but this looks wrong :)
Assignee | ||
Updated•21 years ago
|
Target Milestone: --- → mozilla1.7alpha
Assignee | ||
Updated•21 years ago
|
Summary: (Code cleanup) Replace 0x0a by nsCRT::LF, and 0x0d by nsCRT::CR → (Code cleanup) Replace 0x0a/'\n' by nsCRT::LF, and 0x0d/'\r' by nsCRT::CR
Assignee | ||
Updated•21 years ago
|
Attachment #136934 -
Attachment description: (Av1_Bw) Fixes most '/mailnews/' occurences (review only) → (Av1_Bw) Fixes most '/mailnews/' '0x0?' occurences (review only)
Assignee | ||
Updated•21 years ago
|
Target Milestone: mozilla1.7alpha → ---
Comment 3•20 years ago
|
||
Comment on attachment 136934 [details] [diff] [review] (Av1_Bw) Fixes most '/mailnews/' '0x0?' occurences (review only) nice work! is there any more of this to come for the rest of the tree? i'd suggest asking for rs= on this, rather than sr=... saves the superreviewer from reading the patch. r=dwitte
Attachment #136934 -
Flags: review?(dwitte) → review+
Assignee | ||
Comment 4•20 years ago
|
||
Av1_Bw, updated to current trunk.
Attachment #136934 -
Attachment is obsolete: true
Assignee | ||
Comment 5•20 years ago
|
||
Comment on attachment 154285 [details] [diff] [review] (Av1a-Bw) <mailnews/*> (review only) Keeping {{ (Av1_Bw) Fixes most '/mailnews/' '0x0?' occurences (review only) patch 2003-12-06 07:58 PDT dwitte: review+ }} Seth: Dan suggests giving 'rs' would be appropriate: proceed as you see fit... Then, I'll post a full diff for checkin...
Attachment #154285 -
Flags: superreview?(sspitzer)
Attachment #154285 -
Flags: review+
Assignee | ||
Comment 6•20 years ago
|
||
(In reply to comment #2) > (From update of attachment 136934 [details] [diff] [review]) > >- else { > >+ else > > m_pBuffer[offset] = *pData; > >- } > >+ > > Also i'm no expert at coding and C(++), but this looks wrong :) I'm only removing the brackets (and changing indentation)... What would be wrong exactly ?
Assignee | ||
Comment 7•20 years ago
|
||
(In reply to comment #3) > (From update of attachment 136934 [details] [diff] [review]) > nice work! is there any more of this to come for the rest of the tree? Yes, there should be ... depending on how this (first) patch goes.
Assignee | ||
Comment 8•20 years ago
|
||
Matthias: Could you try to compile, and possibly test, this patch ? Thanks.
Comment 9•20 years ago
|
||
(In reply to comment #8) > Matthias: Could you try to compile, and possibly test, this patch ? Thanks. It compiles and the composer part seems to work (tested with a few signatures) but I cannot test the import parts.
Assignee | ||
Comment 10•20 years ago
|
||
(In reply to comment #9) > (In reply to comment #8) > > Matthias: Could you try to compile, and possibly test, this patch ? Thanks. > > It compiles and the composer part seems to work (tested with a few signatures) > but I cannot test the import parts. Let's be optimistic !? Seth: Would you have time to rs (or sr) this patch ?
Updated•20 years ago
|
Product: Browser → Seamonkey
Assignee | ||
Comment 11•20 years ago
|
||
Comment on attachment 154285 [details] [diff] [review] (Av1a-Bw) <mailnews/*> (review only) No super-review from <sspitzer@mozilla.org> since "2004-07-25" :-( DanM: DanW suggests giving 'rs' would be appropriate: proceed as you see fit...
Attachment #154285 -
Flags: superreview?(sspitzer) → superreview?(dmose)
Comment 12•19 years ago
|
||
Comment on attachment 154285 [details] [diff] [review] (Av1a-Bw) <mailnews/*> (review only) Nice work, and sorry for the delay. rs=dmose@mozilla.org.
Attachment #154285 -
Flags: superreview?(dmose) → superreview+
Assignee | ||
Comment 13•19 years ago
|
||
Av1a-Bw, full diff for checkin, and diffed to the current Trunk. Keeping {{ (Av1a-Bw) <mailnews/*> (review only) patch 2004-07-25 06:32 PDT 21.24 KB gautheri: review+ dmose: superreview+ }} 'approval1.8b4=?': Trivial code cleanup, no risk.
Attachment #154285 -
Attachment is obsolete: true
Attachment #191128 -
Flags: approval1.8b4?
Assignee | ||
Updated•19 years ago
|
Attachment #191128 -
Flags: superreview+
Attachment #191128 -
Flags: review+
Comment 14•19 years ago
|
||
Comment on attachment 191128 [details] [diff] [review] (Av1b) <mailnews/*> (full diff) I'm concerned that we're replacing tabs with newlines out of context here... let's hold off and do this in 1.9, but I would really prefer that we use tabs in context unless we're really going to fix up blocks of whitespace together.
Attachment #191128 -
Flags: approval1.8b4? → approval1.8b4-
Updated•14 years ago
|
Component: General → Backend
Product: SeaMonkey → MailNews Core
QA Contact: general → backend
Assignee | ||
Comment 15•13 years ago
|
||
Av1b, unbitrotted. "99%" of comment 14 suggestion(s) has been done in the meantime. Included logic rewrites are: *nsMsgCompose.cpp: merges 2 |*wPtr = ...|. *nsEudoraCompose::FindNextEndLine(): moves 2 |(count < len)| to front. *nsEudoraMailbox::ExamineAttachment(): moves 1 |(cnt < len)| to front. (Mostly) Already r+ and rs+, but as that was 6.5 years ago...
Attachment #191128 -
Attachment is obsolete: true
Attachment #586694 -
Flags: review?(mbanner)
Assignee | ||
Updated•13 years ago
|
Attachment #586694 -
Attachment description: (Av1c) Replace 0x0A by nsCRT::LF, 0x0D by nsCRT::CR, 9 by '\t', and 32 by ' ', in mailnews/. r=dwitte rs=dmose. → (Av1c) Replace 0x0A by nsCRT::LF, 0x0D by nsCRT::CR, 9 by '\t', and 32 by ' ', in mailnews/
Assignee | ||
Comment 16•13 years ago
|
||
Comment on attachment 586694 [details] [diff] [review] (Av1c) Replace 0x0A by nsCRT::LF, 0x0D by nsCRT::CR, 9 by '\t', and 32 by ' ', in mailnews/ [Checked in: Comment 17] Succeeded as http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=cb3971f9652d
Assignee | ||
Updated•12 years ago
|
Priority: -- → P4
Updated•12 years ago
|
Attachment #586694 -
Flags: review?(mbanner) → review+
Assignee | ||
Updated•12 years ago
|
Attachment #154285 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Attachment #191128 -
Flags: superreview+
Attachment #191128 -
Flags: review+
Assignee | ||
Comment 17•12 years ago
|
||
Comment on attachment 586694 [details] [diff] [review] (Av1c) Replace 0x0A by nsCRT::LF, 0x0D by nsCRT::CR, 9 by '\t', and 32 by ' ', in mailnews/ [Checked in: Comment 17] http://hg.mozilla.org/comm-central/rev/dba9d75af64f
Attachment #586694 -
Attachment description: (Av1c) Replace 0x0A by nsCRT::LF, 0x0D by nsCRT::CR, 9 by '\t', and 32 by ' ', in mailnews/ → (Av1c) Replace 0x0A by nsCRT::LF, 0x0D by nsCRT::CR, 9 by '\t', and 32 by ' ', in mailnews/
[Checked in: Comment 17]
Assignee | ||
Updated•12 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Summary: (Code cleanup) Replace 0x0a/'\n' by nsCRT::LF, and 0x0d/'\r' by nsCRT::CR → Replace 0x0a by nsCRT::LF, 0x0d by nsCRT::CR
Target Milestone: --- → Thunderbird 13.0
Assignee | ||
Comment 18•12 years ago
|
||
Ftr, as these seem fine as is: http://mxr.mozilla.org/comm-central/search?string=0x0a&find=%2Fmailnews%2F "Found 24 matching lines in 6 files" http://mxr.mozilla.org/comm-central/search?string=0x0d&find=%2Fmailnews%2F "Found 25 matching lines in 9 files" V.Fixed, per MXR.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•