[Commits] [wesnoth/wesnoth] 1482e8: preproc: Inherit parent textdomain on slowpath mac...

[GitHub]

  Branch: refs/heads/1.12
  Home:   https://github.com/wesnoth/wesnoth
  Commit: 1482e8d5fbb3ed622436f81ea0f9b6c6aacbb269
      https://github.com/wesnoth/wesnoth/commit/1482e8d5fbb3ed622436f81ea0f9b6c6aacbb269
  Author: Ignacio R. Morelle <shadowm at wesnoth.org>
  Date:   2014-11-18 (Tue, 18 Nov 2014)

  Changed paths:
    M changelog
    M src/serialization/preprocessor.cpp

  Log Message:
  -----------
  preproc: Inherit parent textdomain on slowpath macro substitution (bug #22962)

Fixes bug #22962, reported on the forums as affecting the Swamplings
add-on from 1.10: <http://r.wesnoth.org/r41129>

This is most likely a regression from 1.8.x, as it doesn't affect 1.8.6
and the reporter on the forums claims it affected 1.10.x.

Direct substitutions do not require the instantiation of a new
preprocessor_streambuf, instead inheriting the current context's
streambuf, along with its textdomain. preprocessor_data() is then
constructed with an overruled textdomain which leads to a no-op as it is
identical to the streambuf's current textdomain.

Nested substitutions _do_ require a new streambuf created by means of
its copy constructor, which leaves the new streambuf's textdomain set to
the default of PACKAGE (defined as "wesnoth"). Then a new
preprocessor_data context is instantiated and instructed to overrule the
initial textdomain with the macro's textdomain. However,
preprocessor_data's base class constructor (`preprocessor`) has already
copied the streambuf's initial textdomain id to its previous textdomain
record, causing it to restore that textdomain on destruction instead of
the correct one.

Thus, when performing slowpath substitutions we need to make sure the
preprocessor_data and preprocessor constructors see the parent context's
streambuf textdomain instead of the wesnoth default. Later, the nested
preprocessor_data will restore the correct textdomain and emit a
trailing <FE>line directive for the parent if necessary.

The alternative of having the preprocessor_streambuf copy constructor
copy the original streambuf's textdomain would also fix this bug, but it
would break the intended behavior for slowpath file inclusions, which
always default to wesnoth. Although most WML containing translatable
strings have a #textdomain directive at the start nowadays (and this is
in fact enforced by wmllint), it's probably a bad idea to change the
current behavior in this case for UMC, at least for the 1.12.x stable
series.

I do not expect regressions from this commit and it certainly won't
cause compatibility issues or behavior changes outside i18n -- at least
for add-ons that are doing things right and not relying on translatable
strings having the wrong value.

DM, DW, DiD, EI, HttT, LoW, Liberty, SoF, THoT, TRoW, TSG, data/core,
and data/multiplayer all present preprocessor output differences before
and after this fix, but they amount to no changes to the parser's
output; unlike SotBE and UtBS, which benefit from the fix:

 * data/campaigns/Son_Of_The_Black_Eye/scenarios/07_The_Desert_of_Death.cfg:439
   had the {TURNS_RUN_OUT} core macro substitution bound to the
   campaign's own textdomain instead of wesnoth for some reason.

 * data/campaigns/Under_the_Burning_Suns/scenarios/05_A_Subterranean_Struggle.cfg:1749
   onwards (the ENEMY_ATTACK and ALLY_REINFORCEMENTS macros) bound
   several strings to the wesnoth textdomain instead of the campaign's
   textdomain.

I also tested this commit with my own add-on, After_the_Storm, which
presented similar results as DM et al above. Finally, Swamplings is
properly fixed by this change.