Review: Approve
some nits. Not sure how to test this though.
Diff comments:
> === modified file 'src/graphic/text/bidi.cc'
> --- src/graphic/text/bidi.cc 2015-10-12 12:11:58 +0000
> +++ src/graphic/text/bidi.cc 2016-01-06 12:13:29 +0000
> @@ -555,11 +555,12 @@
> namespace i18n {
>
>
> -// True if a string does not contain Latin characters
> -bool has_rtl_character(const char* input) {
> +// True if a string does not contain Latin characters.
Should that not say: 'True if 'input' contains at least one non-latin
character. Only looks at the first 'limit' characters (or bytes?).'
> +// Checks for the first 'limit' characters maximum.
> +bool has_rtl_character(const char* input, int32_t limit) {
> bool result = false;
> const icu::UnicodeString parseme(input);
> - for (int32_t i = 0; i < parseme.length(); ++i) {
> + for (int32_t i = 0; i < parseme.length() && i < limit; ++i) {
> if (is_rtl_character(parseme.char32At(i))) {
> result = true;
> break;
>
> === modified file 'src/ui_basic/table.cc'
> --- src/ui_basic/table.cc 2015-12-31 08:36:41 +0000
> +++ src/ui_basic/table.cc 2016-01-06 12:13:29 +0000
> @@ -365,8 +374,25 @@
> text_width = text_width + m_scrollbar->get_w();
> }
> UI::correct_for_align(alignment, text_width,
> entry_text_im->height(), &point);
> - // Crop to column width
> - dst.blitrect(point, entry_text_im, Rect(0, 0, curw -
> picw, lineheight));
> +
> + // Crop to column width while blitting
> + if (((static_cast<int32_t>(curw) + picw) < text_width))
> {
change curw to be int? We'll never deal with screens so wide that the sign
should matter. And you can get rid of the cast.
> + // Fix positioning for BiDi languages.
> + if (UI::g_fh1->fontset().is_rtl()) {
> + point.x = alignment & Align_Right ?
> curx : curx + picw;
> + }
> + // We want this always on, e.g. for mixed
> language savegame filenames
> + if
> (i18n::has_rtl_character(entry_string.c_str(), 20)) { // Restrict check for
> efficiency
> + dst.blitrect(point,
> +
> entry_text_im,
> +
> Rect(text_width - curw + picw, 0, text_width, lineheight));
> + }
> + else {
> + dst.blitrect(point, entry_text_im,
> Rect(0, 0, curw - picw, lineheight));
> + }
> + } else {
> + dst.blitrect(point, entry_text_im, Rect(0, 0,
> curw - picw, lineheight));
> + }
> curx += curw;
> }
>
--
https://code.launchpad.net/~widelands-dev/widelands/table_align/+merge/279685
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/table_align.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
