Closed
Bug 336609
Opened 19 years ago
Closed 19 years ago
DeCOMtaminate intl/uconv
Categories
(Core :: Internationalization, defect)
Core
Internationalization
Tracking
()
RESOLVED
FIXED
People
(Reporter: smontagu, Assigned: smontagu)
References
Details
Attachments
(3 files, 2 obsolete files)
283.86 KB,
patch
|
Details | Diff | Splinter Review | |
41.46 KB,
patch
|
Details | Diff | Splinter Review | |
42.89 KB,
patch
|
roc
:
superreview+
|
Details | Diff | Splinter Review |
There's a bunch of utility classes in intl/uconv which are only used internally and have no reason to be XPCOM that I can see.
Assignee | ||
Comment 1•19 years ago
|
||
This patch deCOMtaminates nsIUnicodeDecodeHelper, nsIUnicodeEncodeHelper and nsIMappingCache.
The last seems to be a placeholder for something that never got implemented, and I'm rather tempted to remove it altogether.
Attachment #220789 -
Flags: superreview?(roc)
Attachment #220789 -
Flags: review?(jshin1987)
Assignee | ||
Comment 2•19 years ago
|
||
This version of the patch might be more readable: it's a cvs diff without -N, so not containing any of the files that I removed:
src/nsIMappingCache.h
src/nsIUnicodeDecoder.h
src/nsIUnicodeEncoder.h
nor the files that I moved and didn't change:
src/ubase.h -> util/ubase.h
src/ugen.c -> util/ugen.c
src/umap.c -> util/umap.c
src/umap.h -> util/umap.h
src/unicpriv.h -> util/unicpriv.h
src/uscan.c -> util/uscan.c
combined with a local diff of the files that I both moved and changed:
src/nsMappingCache.cpp -> util/nsMappingCache.cpp
src/nsMappingCache.h -> util/nsMappingCache.h
src/nsUnicodeDecodeHelper.cpp -> util/nsUnicodeDecodeHelper.cpp
src/nsUnicodeDecodeHelper.h -> util/nsUnicodeDecodeHelper.h
src/nsUnicodeEncodeHelper.cpp -> util/nsUnicodeEncodeHelper.cpp
src/nsUnicodeEncodeHelper.h -> util/nsUnicodeEncodeHelper.h
(In reply to comment #1)
> The last seems to be a placeholder for something that never got implemented,
> and I'm rather tempted to remove it altogether.
Please do.
Comment 4•19 years ago
|
||
Comment on attachment 220789 [details] [diff] [review]
patch
r=jshin
we shuld've done this when "deCOM movement" began :-)
Can you ask the cvs admin. of mozilla.org to preserve the revision history of files you're moving?
Having another look at files being moved, it seems to me that 'src' is better than 'util', but I don't think it's a big deal.
Attachment #220789 -
Flags: review?(jshin1987) → review+
Assignee | ||
Comment 5•19 years ago
|
||
Attachment #220789 -
Attachment is obsolete: true
Attachment #221155 -
Flags: superreview?(roc)
Attachment #220789 -
Flags: superreview?(roc)
Assignee | ||
Comment 6•19 years ago
|
||
Attachment #220836 -
Attachment is obsolete: true
Assignee | ||
Comment 7•19 years ago
|
||
(In reply to comment #4)
> Having another look at files being moved, it seems to me that 'src' is better
> than 'util', but I don't think it's a big deal.
Since they're only actually called from nsUCSupport.cpp, which is in util, I thought they logically belonged there.
Why not just make all methods in nsUnicodeDecode/EncodeHelper static, then you won't need any mHelpers, nor will you need to deallocate/free them.
Assignee | ||
Comment 9•19 years ago
|
||
I just noticed a problem here: moving all the stuff from src to util actually makes codesize increase, because uconv and ucvmath both link to ucvutil.
On the other hand, leaving it in in src means that ucvmath gets link errors like
../util/libucvutil_s.a(nsUCSupport.o)(.text+0x791): In function `nsTableDecoderSupport::ConvertNoBuff(char const*, int*, unsigned short*, int*)':
: undefined reference to `nsUnicodeDecodeHelper::ConvertByTable(char const*, int*, unsigned short*, int*, uShiftTableMutable const*, unsigned short const**)'
Is there a way round this?
Assignee | ||
Comment 10•19 years ago
|
||
bsmedberg pointed out to me that the net codesize increase doesn't apply to static builds, and therefore he doesn't consider it too serious.
Assignee | ||
Comment 11•19 years ago
|
||
Only attaching the "user-friendly" version.
Attachment #221413 -
Flags: superreview?(roc)
Attachment #221413 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 12•19 years ago
|
||
Checked in.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•19 years ago
|
Attachment #221155 -
Flags: superreview?(roc)
You need to log in
before you can comment on or make changes to this bug.
Description
•