From c43f9883d639b6c675880865a6f0ea0afa0f6d68 Mon Sep 17 00:00:00 2001 From: "Ignacio R. Morelle" Date: Tue, 18 Nov 2014 03:49:14 -0300 Subject: [PATCH] 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: 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 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. --- changelog | 3 +++ src/serialization/preprocessor.cpp | 3 +++ 2 files changed, 6 insertions(+) diff --git a/changelog b/changelog index c461beab0b96..93e01bbe4044 100644 --- a/changelog +++ b/changelog @@ -197,6 +197,9 @@ Version 1.13.0-dev: * Fixed a bug that prevented [animate_unit] from displaying death or victory animations for those units that filter them based on weapon (eg. Wose) * New WML tags: [full_heal] and [put_to_recall_list]. + * Fixed nested macros emitting incorrect bindings to the default 'wesnoth' + textdomain at the end of a substitution instead of the parent textdomain, + this breaking localization of subsequent strings (bug #22962). * Miscellaneous and bug fixes: * replace 'fight_on_without_leader'=yes/no with defeat_condition=no_leader/ no_units/always/never which allows the wml developer to decide when a side diff --git a/src/serialization/preprocessor.cpp b/src/serialization/preprocessor.cpp index 715a467205c0..4c7d9ad57a19 100644 --- a/src/serialization/preprocessor.cpp +++ b/src/serialization/preprocessor.cpp @@ -1086,6 +1086,9 @@ bool preprocessor_data::get_chunk() std::ostringstream res; preprocessor_streambuf *buf = new preprocessor_streambuf(target_); + // Make the nested preprocessor_data responsible for + // restoring our current textdomain if needed. + buf->textdomain_ = target_.textdomain_; { std::istream in(buf); new preprocessor_data(*buf, buffer, val.location, "", val.linenum, dir, val.textdomain, defines);