Opened 15 years ago
Closed 15 years ago
#5408 closed bug (fixed)
Rename TR* macros
Reported by: | bonefish | Owned by: | pulkomandy |
---|---|---|---|
Priority: | normal | Milestone: | R1 |
Component: | Kits/Locale Kit | Version: | R1/Development |
Keywords: | Cc: | ||
Blocked By: | #5927, #5928 | Blocking: | |
Platform: | All |
Description
The TR* macros need to be renamed to not pollute the global namespace. A simple solution would be to add a B_ prefix, although more descriptive names (e.g. B_TRANSLATE instead of TR) wouldn't harm either.
Attachments (3)
Change History (15)
follow-up: 2 comment:1 by , 15 years ago
comment:2 by , 15 years ago
Replying to pulkomandy:
If they annoy you, you can disable them in any file with a simple #define.
It's really not relevant whether or not they annoy me. They are Haiku specific API and thus should live in an appropriate namespace (B_* in this case).
They have a short name on purpose, because they are used all over the place in a localized app.
Well, B_TRANSLATE would still be relatively short and would be a descriptive name. It's one thing to use a short macro name in some source file (for kernel tracing T is used, sometimes with a second letter, if there are different kinds of tracing in one file), but a completely different thing to define it in a public API.
As an example, wxWidgets use _(), wxT(), and _T() for the same purpose, which is even less descriptive...
No reason to copy bad API decisions, right?
comment:4 by , 15 years ago
I wouldn't even mind a shorter name, but the namespace problem is indeed something that we cannot ignore (IMO _T() wouldn't be too bad either, as the _* namespace is already a system namespace).
While I wouldn't mind calling it B_TRANSLATE(), I could assume that this might reduce source readability when used a lot. I think we could have both, though, a short version in the system namespace, and a long version in the B_* namespace.
comment:5 by , 15 years ago
I'm not so sure about _* being a (generally recognized) system namespace (unlike __*). At least there are public functions like _exit() in it, third party libraries often make use of it (for private stuff), and actually our coding style requires to use it for private methods (different casing would prevent clashes with macros in this case, though).
Regarding readability, I grepped a bit through the preferences and apps sources and the usage density of TR() seems to be moderate to low. With the exception of alerts one has rarely more than one translation per statement, so I wouldn't be too concerned about readability. At any rate I don't see a problem with defining and using a TR() macro in a source file, if that improves readability.
comment:7 by , 15 years ago
How's the patch look? I'm thinking, given the size of the task, it'd be better to define both macros during the transitional phase.
If looks good, i'll start crawling the code. Should I send the first few patches here or just commit them directly?
follow-up: 9 comment:8 by , 15 years ago
I would also rename B_TRANSLATE_CMT to B_TRANSLATE_COMMENT, but the rest looks good IMO. Thanks!
comment:9 by , 15 years ago
Replying to axeld:
rename B_TRANSLATE_CMT to B_TRANSLATE_COMMENT
Will do. I'll start in src/add-ons and will go in alphabetical order... so if anyone else cares to lend a hand, please do.
comment:10 by , 15 years ago
TR_MARK_CMT, TR_MARK_ALL and TR_MARK_ID have the wrong B_TRANSLATE_* equivalent. Otherwise looks fine.
comment:11 by , 15 years ago
Blocked By: | 5927, 5928 added |
---|
The two above attachments show what I believe to be the remaining bits.
Two directories ( src/tests/kits/locale/
& src/apps/installedpackages/
) do not build, have tickets reported ( #5927 & #5928 respectively), & have TR-->B_TRANSLATE patches attached.
As for cleaning up headers/os/locale/Catalog.h
, i'd prefer someone else to do it or tell me the which parts should be deleted -- eg, just the #define TR[.*] lines or the #undef TR[.*] lines as well or ...
comment:12 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Apparently this was already fixed.
If they annoy you, you can disable them in any file with a simple #define. They have a short name on purpose, because they are used all over the place in a localized app. As an example, wxWidgets use _(), wxT(), and _T() for the same purpose, which is even less descriptive...