Re: [PATCH v2] Improve debugging for manual safety annotations

[Juan Manuel Torres Palma <j dot m dot torrespalma at gmail dot com>]

> I'll try to help.

Your help is really appreciated :)

> > +check_and_set_error ()
> > +{
> > +  if [ -n "$error_ln" ]
> > +  then
> > +    echo "$error_ln:Error $error_msg"
> 
> Should be "$error_ln: $error_msg".  Needs a space after the colon and
> "Error" wouldn't be capitalized in any case, but simply using the
> uncapitalized message below is correct, per GNU Coding Standards [1].

I wasn't really aware of the error format by the GNU Coding Standards, so was
a mistake by my side, fixed in the next version of the patch.

> > +error_msg="unexpected tag between @deftypefun and @safety."
> 
> No period at end of message [1].

Likewise.

> > -grep -n '^@safety' "$@" |
> > +error_ln=$(grep -n '^@safety' "$@" |
> 
> In the awk script, you matched on "^@safety{", which seemed like a wise
> choice.

It was inherited code from the previous version, but yeah, if I'm changing it I
can definitely add the '{'.

> immediately followed by a pipe), but it is still technically possible to
> provide a comment in the @prelim macro even though it won't be rendered,
> so it is valid to have "@prelim{foo}" somewhere.
> 

I didn't know it was possible, so added to new patch.

> > +
> > +error_msg="tags are uncorrectly set. Check that every "\
> 
> incorrectly
> 

As you can see some mistakes are caused by bad English.

> > +"remark is placed in the right tag."
> 
> [1] doesn't explicitly address multiple phrases/sentences in the
> message, but I'd assume "Check" is correct here, in which case both
> periods would be correct, because there is a conceptual sentence
> (although [1] seems adamant about not ending with a period).  Thoughts
> otherwise?

My solution so far has been shortening the messages and separate statements
with comma. A couple of examples of this are:

error_msg="tags are incorrectly set, a remark may be placed "\
"in the wrong tag"

error_msg="invalid @safety command, a remark may be missing "\
"or in incorrect order"

Maybe not the best solution but probably more compact and sticks to error
reporting standard better.

> > +#! /usr/local/bin/gawk -f
> 
> Do we really assume gawk is in /usr/local?  I see one other script does
> (manual/xtract-typefun.awk), and 3 others use /usr/bin/awk, and only
> those 4 of 36 *.awk scripts have the shebang.
> 

Coincidentally, I took manual/xtract-typefun.awk as a model to create this one.
Actually I only used the shebang for debugging since it's called from
manual/check-safety.sh as 'awk -f chk-typefun.awk', so we don't really need it.

> > +# Checks that every @deftypefun is follown by a @safety tag.
> 
> followed
> 
> I think "an @safety tag" is correct, because it's read, "followed by an
> at-safety tag."  (I believe they're called "at-commands".)  Again, maybe
> too nit-picky, since using "a" implies you've chosen to pronounce
> "@safety" as "safety" and not "at-safety".
> 

I used to read it as "safety", not "at-safety", but I get your point. I'll add
it to next patch.

> > +  if (!match (inp, "^@safety{")) {
> > +    printf "%s:%d\n", FILENAME, FNR
> > +    exit 1
> > +  }
> > +
> > +  # If we get here is because tags were correctly
> > +  # placed.
> > +}
> 
> I like it.  If we're putting this in a variable (error_ln) that is
> getting prefixed to a message, do we really want/need the trailing newline?

As before, I was using it for debugging and forgot to delete it for the final
version, thanks for pointing out.

-- 
Juan Manuel Torres Palma
Student of MS in Computer Engineering at Universidad de Granada