Menu
▾
▴
#687 wxSmith color names don't get translated
The color names of the wxsColourProperty won't get translated although they use the _() Macro because this gets called before main(). Actually its quite surprising that the code doesn't crash because converting a temporary wxString to a char pointer should point to that deleted object.
Details see here http://forums.codeblocks.org/index.php/topic,22618.0.html
The constructor of the base class (wxEnumProperty) do call wxGetTranslation(), did you test the failure of translation?. In fact the parameter is called untraslatedLabels:
wxEnumProperty::wxEnumProperty( const wxString& label, const wxString& name, const char* const* untranslatedLabels, const long* values, wxPGChoices* choicesCache, int value ) : wxPGProperty(label,name) { SetIndex(0); wxASSERT( choicesCache ); if ( choicesCache->IsOk() ) { m_choices.Assign( *choicesCache ); m_value = wxPGVariant_Zero; } else { for ( int i = 0; *untranslatedLabels; untranslatedLabels++ ) { const long val = values ? *values++ : i++; m_choices.Add(wxGetTranslation(*untranslatedLabels), val); } if ( GetItemCount() ) SetValue( (long)value ); } }Although the use of _() in the namespace has no consequences, it is needed for string extraction when using poedit or a similar tool.
There is only one case where the colour name is not translated, and only for wxWidgets < 3.0. The attached patch should fix this.
Last edit: Miguel Gimenez 2018-05-17
No i didn't test this because i don't use wxSmith. Also the documentation doesn't mention this pretty surprising feature, other "container-widgets" don't work like this. So good catch finding this, so actually my quickfix with
wxTRANSLATE()does work properly then, together with your patch.The use of
_()could do harm if it would work, because it does expand towxGetTranslation(), so you would translate twice.Better patch: use already translated string from class' internal data instead of data in namespace
Sadly, the situation seems to be more complicated. I did a closer look at the wxPropgrid sources and there seems to be a major difference between wx 2.8 and wx 3.x, this would also explain the different parameter types the wxEnumProperty accepts. This integrated translation only happens on wx 3.x which requires const char labels, it does not happpen on wx 2.8 which requires wxChar labels.
I am not familiar with wxPropgrid and the changes between both generations, maybe this is even noted in some change notes or other elements perform additional translation operations. My main motivation was to fix the compilation error, the second one to fix the possible dangling pointers, using wxTRANSLATE fixes both these issues. And on the wx 3.x branch this doesn't break translation which is currently already working, on the wx 2.8 branch it doesn't fix translation which is currently likely broken, i can live with that :).
Since i was building CodeBlocks again i reviewed this issue and propose the following patch which is a combination of the above from Miguel and my original patch which does the following:
- Fixes compilation with
wxUSE_UNSAFE_WXSTRING_CONV = 0- Fixes the single case that did not get translated under wxWidgets > 2.8
- Doesn't change anything under wxWidgets 2.8 (== nothing gets translated)
Last edit: sodev 2018-06-21
Does anything speaks against this patch?
I do not work with poedit, but i think you should be able to add a custom translation marker, so you can add wxTRANSLATE to the list...
Thats the whole purpose of wxTRANSLATE, to act as a marker, it doesn't do anything else. The code needs to perform the actual translation by calling the proper gettext functions (like wxGetTranslation on wxWidgets) and thats the hard part here.
applied in 11711
Thank you for your work!
Well... i have to reopen this ticket because of:
https://travis-ci.org/obfuscated/codeblocks_sf/builds/541598379?utm_source=github_status&utm_medium=notification
the build fails on linux.
my proposed fix:
diff against svn trunk...
Last edit: bluehazzard 2019-06-05
At first i would check what wxWidgets version is used on that machine. As you can see in the file there is already a wxWidgets version check which controls the type of the array. Maybe there is a intermediate version running that is not uprevved yet. And then i would check if wxWidgets defines this macro differently per OS in certain releases, at least on master i can't see this to happen.
This is the official wxWidgets macro to set a mark for translation, either repair it if its broken or adapt the code to its usage instead of inventing your own one.
Wouldn't using wxT("...") fix the problem?
No, this would make it worse. This macro always expands to a wide string plus it won't mark the string for translation.
I digged around in the history, it looks like wx2.8 was the last version where wxTRANSLATE actually expands to wxT. The master version on the property accepts both versions of the array, wxChar and char.
The problem is the CI machine seems to run on wx30 and that version only accepts a wxChar array. So no way to use wxTRANSLATE in that case. There is the wxS macro that returns the proper type but this is also not intended to work as translation marker.
I currently don't see a good solution to make wx30 happy. You could always use the wxChar version but this requires pre-translated strings which doesn't work because the array is global. You could use the wxS macro but this doesn't mark the strings for translation. You could provide a self-made wxS version that acts like a marker but this requires to document this so people who extract strings from CodeBlocks need to know to use that marker as well. Or you could say wx30 is not supported, use something newer :).
I don't know why you're even making this comment. wx30 is the last non-development version of wxWidgets. This is what linuxes are getting for a long while...
Because i was already using 3.1.0, 3.1.1, 3.1.2 and expected these are "regular" releases, but looks like i was wrong. Didn't know that 3.1.x are only development ones.
I took another analysis run, these are my results. The situation is really bad if you need to support wx28, wx30 and wx31.
wx28 requires wxChar, you can use wxTRANSLATE to produce wxChar literals, but the code doesn't perform the translation calls, so you don't get translated values.
wx30 requires wxChar, you can NOT use wxTRANSLATE to produce wxChars literals, the code does perform the translation calls. Problem is to get the wxChar values plus the question if gettext can actually use these to do the translation lookups, i remember it had quite some issues with wide chars.
wx31 requires char, you can use wxTRANSLATE, the code does perform translation calls. Win, everything works here easily.
The approach before my slightly broken patch was to use translated wxString objects which convert into char and wxChar. Problem here is that its quite surprising that this doesn't crash because these wxString object exist only temporary so the returned wxChar or char can point to deleted memory. Second problem is that this requires unsafe string conversions which might produce suprising results on certain locale combinations. And finally the objects aren't translated at all because the translation calls happen before main, long before the message catalogs are setup.
There is only one thing that works for all 3: wxArray. This solution has only one drawback, you can't use the choices cache anymore. So i propose the following patch to fix the issues for all 3 variants. Using lazy initialization tricks i make sure the translation calls happen after the system has been initialized, this can only be defeated if you actually create a global property object which should not happen. With c++11 the initialization is also thread safe which again is not really necessary because these properties should only be created by the main thread anyway.
Only tested on windows against wx-master.
const-corrected version :o)
wxT() :
why not
?
I agree that it is bul**** to introduce an other translation marker but do we have a choice ?
Choose the simplest solution. To me it seems that if it works in 3.1.x just guard this for these versions and don't bother with the rest.
You can't use wxS because i was wrong about it, it translates to the internal wxString datatype which might not be wxChar, this is controlled by other defines.
If you don't care about translation functionality in < wx31 the simplest solution would be to extend the #ifdef to include the definition of the array elements, wrap them in wxT for wxChar and wrap them in wxTRANSLATE for char.
And here comes the static array solution.
Now you can choose, use the dynamic array version to get working translation in < wx31 at the cost of using dynamic arrays and no property cache, use the static array version with broken translation in < wx31 but no dynamic memory and working property cache. In wx31 both versions offer working translation.
Both versions work with
wxUSE_UNSAFE_WXSTRING_CONV = 0and fix the possible use of deleted memory.What is the benefit of this property cache? For the static solution I would rather use XMacro solution.