Support positional specifiers on Windows for translations

[mattdowle]

As per #4402 (comment)
Attempt at #4402

[MichaelChirico]

Is it just luck that this came up for snprintf since the root issue seems to be the positional specifiers?

[MichaelChirico]

I guess it's because most message strings are sent through Rprintf or error/warning so R is handling the platform robustness.

I also see fprintf used in fwrite.c but those messages are not translated.

[codecov]

Codecov Report

Merging #4523 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4523   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files          72       72           
  Lines       13971    13971           
=======================================
  Hits        13916    13916           
  Misses         55       55           

Impacted Files	Coverage Δ
src/fmelt.c	99.00% <100.00%> (ø)
src/freadR.c	100.00% <100.00%> (ø)
src/rbindlist.c	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbd4a33...ab14c0a. Read the comment docs.

[MichaelChirico]

A few more related observations

R defines Rsnprintf, maybe we should prefer that? Not clear if it's part of the API, but I do see it handling Windows differently.

https://github.com/wch/r-source/blob/337dc093b104f5a74c96761916cde535b3e4debf/src/main/errors.c#L291

R does use snprintf with translation in a few places (all in ./src/):

./library/tools/src/gramRd.c:	    snprintf(buffer, sizeof(buffer), _("Macro '%s' previously defined."), 
./library/tools/src/gramRd.c:    	snprintf(buffer, sizeof(buffer), _("At most 4 arguments are allowed for user defined macros."));
./library/tools/src/gramRd.c:    		snprintf(ParseErrorMsg, PARSE_ERROR_SIZE, _(yyshortunexpected),
./library/tools/src/gramRd.c:    	    	snprintf(ParseErrorMsg, PARSE_ERROR_SIZE, _(yylongunexpected),
./library/utils/src/io.c:    snprintf(ConsolePrompt, CONSOLE_PROMPT_SIZE, _("Selection: "));
./library/utils/src/windows/dialogs.c:	snprintf(msgbuf, 256, _("unable to retrieve items for %s (%s)"),
./library/utils/src/windows/dialogs.c:	    snprintf(msgbuf, 256, _("unable to add menu (%s)"), errmsg);
./library/utils/src/windows/dialogs.c:	    snprintf(msgbuf, 256, _("unable to add menu item (%s)"), errmsg);
./library/utils/src/windows/dialogs.c:	    snprintf(msgbuf, 256, _("unable to delete menu item (%s)"), errmsg);
./library/grDevices/src/devWindows.c:    snprintf(buf, 100, G_("%s quality..."), "50%");
./library/grDevices/src/devWindows.c:    snprintf(buf, 100, G_("%s quality..."), "75%");
./library/grDevices/src/devWindows.c:    snprintf(buf, 100, G_("%s quality..."), "100%");
./gnuwin32/rui.c:	    snprintf(cmd, 1024, G_("'%s' cannot be found"), s);
./gnuwin32/system.c:    snprintf(pp, 1024, _("Fatal error: %s\n"), s);
./gnuwin32/system.c:	    snprintf(s, 1024, _("Workspace name '%s' is too long\n"), name);
./gnuwin32/system.c:	snprintf(s, 1024, _("ARGUMENT '%s' __ignored__\n"), name);
./gnuwin32/system.c:		    snprintf(s, 1024, _("WARNING: '-e %s' omitted as input is too long\n"), *av);
./gnuwin32/system.c:		snprintf(s, 1024, _("WARNING: unknown option '%s'\n"), *av);
./gnuwin32/run.c:	snprintf(RunError, 500, _("'CreateProcess' failed to run '%s'"), ecmd);
./gnuwin32/run.c:	    snprintf(RunError, 501, _("Exit code was %d"), ret);
./gnuwin32/editor.c:	    snprintf(buf, MAX_PATH+30, G_("Could not save file '%s'"), sname);
./unix/system.c:		    snprintf(msg, 1024, _("WARNING: '-e %s' omitted as input is too long\n"), *av);
./unix/system.c:		snprintf(msg, 1024, _("WARNING: unknown option '%s'\n"), *av);
./unix/system.c:	    snprintf(msg, 1024, _("ARGUMENT '%s' __ignored__\n"), *av);
./extra/graphapp/dialogs.c:	    snprintf(buf, 100, G_("\"%s\" not found"), pfr->lpstrFindWhat);
./extra/graphapp/dialogs.c:	    snprintf(buf, 100, G_("\"%s\" not found"), pfr->lpstrFindWhat);
./main/coerce.c:	    snprintf(msg, 128, _("'length(x) = %lld > 1' in coercion to '%s'"),
./main/gram.c:                        snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected input"));
./main/gram.c:                        snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected end of input"));
./main/gram.c:                        snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected input"));
./main/gram.c:                        snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected string constant"));
./main/gram.c:                        snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected numeric constant"));
./main/gram.c:                        snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected symbol"));
./main/gram.c:                        snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected assignment"));
./main/gram.c:                        snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected end of line"));
./main/gram.c:                  snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE, _("unexpected %s"),
./main/gram.c:	snprintf(R_ParseErrorMsg, PARSE_ERROR_SIZE - 1, _("unexpected %s"),
./main/connections.c:    snprintf(buf, 100, _("unsupported conversion from '%s' to '%s'"), from, to);
./main/errors.c:    Rvsnprintf(buf, BUFSIZE, _(ErrorDB[i].format), ap);
./main/errors.c:    Rvsnprintf(buf, BUFSIZE, _(WarningDB[i].format), ap);

However this issue doesn't matter at all unless there's more than one formatter, so only these are relevant:

grep -Enr "snprintf.*_\(" . --include=*.{c,h} | grep -Fv "Rsnprintf" | grep '%[^"]*%'
./library/utils/src/windows/dialogs.c:245:	snprintf(msgbuf, 256, _("unable to retrieve items for %s (%s)"),
./main/coerce.c:1755:	    snprintf(msg, 128, _("'length(x) = %lld > 1' in coercion to '%s'"),
./main/connections.c:233:    snprintf(buf, 100, _("unsupported conversion from '%s' to '%s'"), from, to);

So 3 possible places for this to show up. We can see existing translations with e.g.

TARGET_MESSAGE="unable to retrieve items for %s (%s)"
grep -Fr "\"$TARGET_MESSAGE\"" . --include=*.po -A 1

1. "unable to retrieve items for %s (%s)" always appears in that order in current translations
2. 'length(x) = %lld > 1' in coercion to '%s' is not translated to any language
3. unsupported conversion from '%s' to '%s' uses the positional identifiers in Korean:

./library/base/po/ko.po:msgid "unsupported conversion from '%s' to '%s'"
./library/base/po/ko.po-msgstr "'%1$s'에서 '%2$s'로의 변환은 지원되지 않습니다"

In connections.c, I see this at the top:

#ifdef Win32
#include <trioremap.h>
#endif

Which is at src/gnuwin32/fixed/h/trioremap.h and mentions:

/* Recent MinGW-w64 releases define snprintf and vsnprintf as macros
   that can't be redefined; these are necessary to make our substitutions
   of the trio replacements.  We need the replacements because msvcrt.dll
   doesn't support hex formatting of floating point numbers, and all (?)
   versions of the MS run-time use three digits in floating point
   E formats, e.g. 1.E000. The replacements also allow printing long long
   integers via %lld which the MS run-time does not (and "%I64d" is not
   C99). */

and redefines snprintf:

#ifndef R_TRIOREMAP_H
#define R_TRIOREMAP_H

int trio_snprintf(char *buffer, size_t max, const char *format, ...);
int trio_vsnprintf(char *buffer, size_t bufferSize, const char *format,
		   va_list args);
#define snprintf trio_snprintf
#define vsnprintf trio_vsnprintf

#endif  /* not R_TRIOREMAP_H */

This trio_snprintf is also being used in Rsnprintf referenced above

[mattdowle]

Great finds. Sadly Rsnprintf is defined static which means it's not linkable to. Further, trio_vsnprintf appears in src/gnuwin32/Rdll.hide which, I guess, stops packages calling it, since it otherwise would be (no static and no attribute_hidden in its prototype).
So, trio appears to be a 3rd party library: https://daniel.haxx.se/projects/trio/