[wesnoth-commits] [wesnoth/wesnoth] c5b394: Replace all instances of naked vgettext() calls wi...

Fri Mar 23 07:27:22 UTC 2018

  Branch: refs/heads/master
  Home:   https://github.com/wesnoth/wesnoth
  Commit: c5b3947e4a837dc98868e3b2c3fa55668fec27a4
      https://github.com/wesnoth/wesnoth/commit/c5b3947e4a837dc98868e3b2c3fa55668fec27a4
  Author: Iris Morelle <shadowm at wesnoth.org>
  Date:   2018-03-23 (Fri, 23 Mar 2018)

  Changed paths:
    M src/actions/move.cpp
    M src/addon/client.cpp
    M src/addon/manager_ui.cpp
    M src/chat_events.cpp
    M src/editor/map/context_manager.cpp
    M src/editor/map/editor_map.cpp
    M src/editor/map/map_context.cpp
    M src/editor/palette/location_palette.cpp
    M src/game_initialization/configure_engine.cpp
    M src/game_initialization/connect_engine.cpp
    M src/game_initialization/flg_manager.cpp
    M src/game_initialization/lobby_data.cpp
    M src/game_initialization/mp_game_utils.cpp
    M src/game_initialization/multiplayer.cpp
    M src/gui/core/linked_group_definition.cpp
    M src/gui/core/window_builder.cpp
    M src/gui/dialogs/addon/manager.cpp
    M src/gui/dialogs/depcheck_confirm_change.cpp
    M src/gui/dialogs/file_dialog.cpp
    M src/gui/dialogs/game_load.cpp
    M src/gui/dialogs/label_settings.cpp
    M src/gui/dialogs/multiplayer/lobby.cpp
    M src/gui/dialogs/multiplayer/mp_change_control.cpp
    M src/gui/dialogs/multiplayer/mp_join_game.cpp
    M src/gui/dialogs/preferences_dialog.cpp
    M src/gui/widgets/chatbox.cpp
    M src/gui/widgets/helper.cpp
    M src/gui/widgets/unit_preview_pane.cpp
    M src/hotkey/hotkey_handler.cpp
    M src/hotkey/hotkey_handler_sp.cpp
    M src/menu_events.cpp
    M src/mp_ui_alerts.cpp
    M src/playturn.cpp
    M src/preferences/display.cpp
    M src/synced_commands.cpp
    M src/synced_user_choice.cpp
    M src/units/unit.cpp
    M src/whiteboard/manager.cpp
    M src/wml_exception.cpp

  Log Message:
  -----------
  Replace all instances of naked vgettext() calls with VGETTEXT()

The vgettext() function, while declared in src/formula/string_utils.hpp,
actually has its implementation out-of-line in
src/formula/string_utils.cpp where GETTEXT_TEXTDOMAIN is defined to
"wesnoth-lib". Because vgettext() is implemented in terms of the _()
function (an inline wrapper around translation::dsgettext()), it passes
the textdomain defined in the file where it was implemented as a
parameter.

This means that every case of vgettext() being used in other code units
where GETTEXT_TEXTDOMAIN is not defined to "wesnoth-lib", is broken if
the string being looked upon doesn't coincidentally exist in the
wesnoth-lib textdomain.

Ages ago, to work around this limitation, an overload of vgettext() that
takes the textdomain name as a parameter was introduced (see commit
0ba3d05204abff72f7d95cf11a91536dab5aa20a). Since this form of vgettext()
is rather unwieldy to use (and in particular, the xgettext message
extraction tool mistakes the first argument for the msgid, see below), a
VGETTEXT() macro was also added that uses the GETTEXT_TEXTDOMAIN symbol
defined in the file where the call is made, and thus we get the correct
string from the correct textdomain.

Switching all cases of naked vgettext() in mainline to VGETTEXT() fixes
a myriad of situations where an interpolated string that has an extant
translation does not actually get translated in practice because of the
mismatched textdomain reference (see issue #2709 for an example with MP
game titles). I couldn't find any cases of the companion vngettext()
function (which handles plurals) being used in the wild naked, but for
future reference it also has a companion VNGETTEXT() macro to pass the
correct textdomain to its textdomain-parameter overload.

One caveat is that this commit DOES break the string freeze in one
particular case -- src/units/unit.cpp has a case where the
textdomain-parameter version of naked vgettext() was in use with
"wesnoth" as the first parameter, and xgettext misidentified this as a
translation entry for a "wesnoth" string in the file's assigned
textdomain (which is the default textdomain, wesnoth). So this will
result in the next pot-update both removing the spurious "wesnoth"
string AND adding the correct string to the relevant catalogue template
("<span color=\"$color\">$number_or_percent</span> HP").
to that textdomain.

Other than that, I believe this does not break the string freeze in any
other fashion and it shouldn't result in any regressions for i18n.

It might be worth considering in the future renaming vgettext() and
vngettext() to names that make people less likely to misidentify them as
functions they can freely call directly without regard to the textdomain
assignment issue.

  Commit: 5c28dcaab66e34af718c7ceff6ccc9e33b58b039
      https://github.com/wesnoth/wesnoth/commit/5c28dcaab66e34af718c7ceff6ccc9e33b58b039
  Author: Iris Morelle <shadowm at wesnoth.org>
  Date:   2018-03-23 (Fri, 23 Mar 2018)

  Changed paths:
    M src/formula/string_utils.hpp

  Log Message:
  -----------
  i18n: Forcefully pull gettext.hpp into formula/string_utils.hpp

There are cases (deprecation.cpp for one) where string_utils.hpp is
included first, which causes the VGETTEXT/VNGETTEXT definition to use
the textdomain-less (a.k.a. forced wesnoth-lib textdomain) overloads of
vgettext() and vngettext(), because GETTEXT_DOMAIN has not yet been
defined by anything. This again results in strings being looked up in
catalogues where xgettext is not adding them.

This is a companion for PR #2711 I should've noticed sooner. Without it,
there were still cases where interpolated strings would not be
translated due to vgettext() using the wrong textdomain for them.

I ran a quick scan on the codebase to make sure there aren't any files
including formula/string_utils.hpp before defining their own
GETTEXT_DOMAIN instead of the gettext.hpp default.

  Commit: a60239116863ea7592d27f2819017b57a34e0ce5
      https://github.com/wesnoth/wesnoth/commit/a60239116863ea7592d27f2819017b57a34e0ce5
  Author: Iris Morelle <shadowm at wesnoth.org>
  Date:   2018-03-23 (Fri, 23 Mar 2018)

  Changed paths:
    M src/formula/string_utils.hpp

  Log Message:
  -----------
  i18n: Make VGETTEXT()/VNGETTEXT() always require a GETTEXT_DOMAIN to be defined

This drops a preprocessor conditional branch that is dead code now that
the file pulls gettext.hpp and the GETTEXT_DOMAIN defaults with it.