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

[GitHub]

  Branch: refs/heads/1.14
  Home:   https://github.com/wesnoth/wesnoth
  Commit: 69c28e5088b6a9624e08c72be510e0a670d030f0
      https://github.com/wesnoth/wesnoth/commit/69c28e5088b6a9624e08c72be510e0a670d030f0
  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: 96032f978c431525267c75d0e6781e97608145a1
      https://github.com/wesnoth/wesnoth/commit/96032f978c431525267c75d0e6781e97608145a1
  Author: Iris Morelle <shadowm at wesnoth.org>
  Date:   2018-03-23 (Fri, 23 Mar 2018)

  Changed paths:
    M changelog.md

  Log Message:
  -----------
  Update the changelog

[ci skip]


Compare: https://github.com/wesnoth/wesnoth/compare/48514bf318fb...96032f978c43