Re: [PATCH] Introduce-alignment-mechanism-for-error-messages

[Akim Demaille]

Le 14 sept. 2009 à 23:43, Alex Rozenman a écrit :

> Hi Akim, Hi Joel, All,
>
> I would like to install the following (attached) patch in order to  
> make the
> error messages more readable. Please review the presented mechanism  
> and let
> me know if there are any objections/comments.

Hi Alex,

        * src/location.h: location_print: return number of written

This is incorrect: put the name of the entities in parens.

        characters.
        * src/location.c: Implemented here.
        * src/complain.h: New functions: warn_at_indent,
        complain_at_indent.

Likewise.  See other entries in the ChangeLog to have a model.

        * src/complain.c: Implemented here. error_message function:
        update *indent_ptr accordingly.
        * src/scan-code.l: Use warn_at_indent/complain_at_indent.
        * src/named-ref.at: Adjust tests.



Please use "res" as variable name when they are the result of the  
function (location_print for instance).

+/* Will return number of printed characters.  */
+unsigned location_print (FILE *out, location loc);

Use the imperative: Return the number...



          /* Create the explanation message. */
          obstack_init (&msg_buf);

-         obstack_fgrow1 (&msg_buf, "  possibly meant: %c", dollar_or_at);
+         obstack_fgrow1 (&msg_buf, "possibly meant: %c", dollar_or_at);
          if (contains_dot_or_dash (id))
            obstack_fgrow1 (&msg_buf, "[%s]", id);
          else


Missing translation.  Unless it is translated here:

-            warn_at (id_loc, _("%s"), (char *) obstack_finish  
(&msg_buf));
+            warn_at_indent (id_loc, &indent, _("%s"),
+                            (char *) obstack_finish (&msg_buf));

This is not good on two accounts.  First, it probably means that the  
format-string is built, which is not good for translators, as I noted  
elsewhere.  And also, there is no marker on the "possibly meant"  
string above so that the translators be warned that this string must  
be provided a translation.  Using _("...") does two things: it tells  
the gettext machinery that the string will have to be translated (sort  
of static analysis if you wish), and at run time, it really looks for  
the translation.

Here, you need to use N_.  You should read http://www.gnu.org/software/gettext/manual/gettext.html#Sources 
.



Other than that, the patch is fine.  But I think I'm changing my mind  
about the "note:" mechanism you once proposed.  I fear that Emacs will  
understand the submessages as distinct errors, instead of a single  
error.  Unfortunately the GNU Coding Standard is not really good  
enough here, we should probably ask for an update.

        http://www.gnu.org/prep/standards/standards.html#Errors