bison-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Dlang] Internationalisation WIP


From: Adela Vais
Subject: Re: [Dlang] Internationalisation WIP
Date: Tue, 5 Jan 2021 18:19:46 +0200

Hello!

The following 4 commits regard the internationalisation, and the removal
of: yytnamerr, $end, and some unused imports caused by the
internationalisation (now the import of std.conv is at global scope, not
only used by lookahead correction).

În mar., 8 dec. 2020 la 16:31, Staniloiu Eduard <edi33416@gmail.com> a
scris:

> This can definitely be done with introspection.
> I wrote the gist of it below:
> ```
> static if (!is(typeof(YY_))) {
> version(YYENABLE_NLS)
> {
>         version(ENABLE_NLS)
>         {
>             alias YY_ = partial!(dgettext, "bison-runtime");
>         }
> }
>     static if (!is(typeof(YY_)))
>     {
>         pragma(inline, true)
>         string YY_(string msg) { return msg; }
>     }
> }
> static if (!is(typeof(N_)))
> {
>     pragma(inline, true)
>     string N_(string msg) { return msg; }
> }
> ```
> To use YYENABLE_NLS and ENABLE_NLS, one would use the `-version` compiler
> option:
> `-version=YYENABLE_NLS -version=ENABLE_NLS`
>
>
I used Edi's suggestion from above.

>
>> This looks like yytnamerr, something the old skeletons have to live with
>> that does not make sense in D.  In the other skeletons, this is triggered
>> with "parse.error verbose" that you should not support.  Support only
>> "parse.error detailed".  You should remove all this.
>>
>> Done using yysymbol_name .

> In C, for instance, i18n is handled in yysymbol_name.
>>
>> You should really eliminate all this code, even in the other cases.
>> I suggest that first you change this commit to not add this piece
>> of code for i18n, and in an later commit, you remove this from the
>> other cases too.
>
> Done.

> >        else if (yystr == "$end")
>> > -      {
>> > -        put(sink, "end of input");
>> > +      {]b4_has_translations_if([[
>> > +        put(sink, _("end of input"));]],[[
>> > +        put(sink, "end of input");]])[
>> >          return;
>>
>> This is hairy, and is no longer needed at all.  Java no longer
>> uses that either.  The translation of $end is provided by bison itself,
>> like any other token, the skeleton should not play with it at all.
>> Have a look at the other skeletons.
>>
> Done.

>
>> > +/**
>> > + * Handle error message internationalisation
>>
>> Please end your comments with a period.
>>
> Done.

>
>> > + */
>> > +extern(C) char* dgettext(const char*, const char*);
>> > +string YY_(const char* s)
>> > +{
>> > +  char* res = dgettext("bison-runtime", s);
>> > +  return to!string(res);
>>
>> Why this intermediate variable?  Also, can't you declare dcgettext
>> within the function itself?  I like scopes to be as small as possible.
>>
> The C import only works at global scope.
I removed the variable.

>
>> > +}
>> > +]b4_has_translations_if([
>> > +/**
>> > + * User error message internationalisation
>> > + */
>> > +extern(C) char* gettext(const char*);
>> > +string _(string s)
>> > +{
>> > +  char* res = gettext(s.ptr);
>> > +  return to!string(res);
>>
>> Ditto.
>>
> Done.

>
>> > +}
>> > +])[
>> >  /**
>> >   * A Bison parser, automatically generated from
>> <tt>]m4_bpatsubst(b4_file_name, [^"\(.*\)"$], [\1])[</tt>.
>> >   *
>> > @@ -704,20 +725,41 @@ m4_popdef([b4_at_dollar])])dnl
>> >        immutable int argmax = 5;
>> >        SymbolKind[] yyarg = new SymbolKind[argmax];
>> >        int yycount = yysyntaxErrorArguments(yyctx, yyarg, argmax);
>> > -      string res = "syntax error, unexpected ";
>> > -      res ~= format!"%s"(yyarg[0]);
>> > -      if (yycount < argmax + 1)
>> > +      string[] yystr = new string[yycount];
>> > +      for (int yyi = 0; yyi < yycount; yyi++)
>> > +        yystr[yyi] = format!"%s"(yyarg[yyi]);
>> > +      string res, yyformat;
>> > +      import std.string;
>> > +      switch (yycount)
>> >        {
>> > -        for (int yyi = 1; yyi < yycount; yyi++)
>> > -        {
>> > -          res ~= yyi == 1 ? ", expecting " : " or ";
>> > -          res ~= format!"%s"(SymbolKind(yyarg[yyi]));
>> > -        }
>> > +        case  1:
>> > +          yyformat = YY_("syntax error, unexpected %s");
>> > +          res = format(yyformat, yystr[0]);
>> > +         break;
>>
>> I don't see the point of using format twice: once to convert the symbol
>> kinds to strings, and then to build the full error message.  How about
>> not using yystr at all?
>>
> I removed it.

> > +The internationalization in D is very simmilar to the one in C. The D
>> > +parser uses gettext for user messages and dgettext for Bison messages.
>>
>> Use @code for function names.  But I'm not sure we should go into such
>> details here.
>>
>> > +If gettext is not present on your system,
>>
>> Use Title Case for package names, but...
>>
>> > install it and re-run Bison's
>> > +configuration.
>>
>> The manual is about an installed Bison.  Configuration and installation
>> is irrelevant here.
>>
>> > +
>> > +For enabling internationalisation, import bindtextdomain and textdomain
>>
>> Use @code for function names.  I feel unconformable with "For enabling"
>> instead of "To enable", but I'm not a native.
>>
>> > +from C:
>> > +
>> > +@example
>> > +extern(C) char* bindtextdomain(const char* domainname, const char*
>> dirname);
>> > +extern(C) char* textdomain(const char* domainname);
>> > +@end example
>> > +
>> > +The main function should load the translation catalogues, similarly to
>> the
>> > +@file{c/bistromathic} example:
>> > +
>> > +@example
>> > +int main()
>> > +@{
>> > +  import core.stdc.locale;
>> > +
>> > +  // Set up internationalization.
>> > +  setlocale(LC_ALL, "");
>> > +  // Use Bison's standard translation catalogue for error messages
>> > +  // (the generated messages).
>> > +  bindtextdomain("bison-runtime", BISON_LOCALEDIR);
>> > +  // For the translation catalogue of your own project, use the
>> > +  // name of your project.
>> > +  bindtextdomain("bison", LOCALEDIR);
>> > +  textdomain("bison");
>> > +
>> > +  // usual main content
>> > +  ...
>> > +@}
>> > +@end example
>> > +
>> > +String aliases may be marked for internationalization (@pxref{Token
>> > +I18n}):
>>
>> We should avoid repeating in each language the common bits.  I don't
>> these bits are needed it, is it?
>>
> I modified the documentation.

> > +%code {
>> > +]AT_TOKEN_TRANSLATE_IF([[
>> > +  static string _(string s)
>> > +  {
>> > +    if (s == "end of input")
>> > +      return "end of file";
>> > +    else if (s == "number")
>> > +      return "nombre";
>> > +    return s;
>>
>> I personally prefer that we maintain the "else if" chain, instead of
>> relying on return to break the execution flow.  Make it more
>> functional-style
>> and add the last else.  Currently the style is even inconsistent, with
>> just
>> one else.
>>
>> And if D supports 'switch' over string, well, even better :)
>>
> I used a switch.

Thank you for all the help, Akim and Edi!

Adela

Attachment: 0001-d-add-internationalisation-support.patch
Description: Source code patch

Attachment: 0004-d-remove-unnecessary-imports.patch
Description: Source code patch

Attachment: 0002-d-remove-special-case-from-SymbolKind.toString.patch
Description: Source code patch

Attachment: 0003-d-remove-yytnamerr-usage.patch
Description: Source code patch


reply via email to

[Prev in Thread] Current Thread [Next in Thread]