diff mbox

[v2] Complete example in error message documentation.

Message ID 544CA0EB.8030205@pacific.net
State New
Headers show

Commit Message

Rical Jasan Oct. 26, 2014, 7:21 a.m. UTC
Previously unquoted #include at beginning of line caused it to disappear
from commit message. Diff unchanged.

----

The manual gives "an example showing how to handle failure to open a
file correctly."  The example function, open_sesame, uses the
newly-introduced strerror function and errno and
program_invocation_short_name variables.  It fails to specify GNU
extensions, however, so attempts to use it in the following way:

    int main (void) {open_sesame ("badname");}

fail during compilation with "error: ‘program_invocation_short_name’
undeclared", indicating the example is incomplete.  The presence of
"#include"s suggest everything neccesary for the function to work should
be present.  For completeness, the example is lacking the following line:

    #define _GNU_SOURCE

as the declarations of program_invocation_*name in errno.h are wrapped
in an "#ifdef __USE_GNU" conditional.

The documentation of the variables is also expanded, adding that their
definition lies in errno.h and noting specifically they are GNU
extensions.

    manual/errno.texi: Complete example function and expand
    documentation of program_invocation*_name variables.

2014-10-25  Rical Jasan  <redacted>

	* manual/errno.texi (Error Messages): Complete example function
	by adding missing #define.
	(program_invocation_name): Add statement indicating GNU
	extension and reference which header file declares the variable.
	(program_invocation_short_name): Likewise.

Comments

Mike Frysinger March 5, 2015, 7:08 p.m. UTC | #1
On 26 Oct 2014 00:21, ricaljasan wrote:
> 2014-10-25  Rical Jasan  <redacted>

that's not how it works.  i guess you want to use ricaljasan@pacific.net ?

patch looks fine to me otherwise.
-mike
Rical Jasan March 6, 2015, 2:35 a.m. UTC | #2
On 03/05/2015 11:08 AM, Mike Frysinger wrote:
> On 26 Oct 2014 00:21, ricaljasan wrote:
>> 2014-10-25  Rical Jasan  <redacted>
> 
> that's not how it works.  i guess you want to use ricaljasan@pacific.net ?
> 
> patch looks fine to me otherwise.
> -mike
> 

Fair enough.  That address is fine with me.  :)

OOC, is there a lifetime on submitted patches?  I see you must be going
through pending or otherwise queued submissions.  I have been working
more or less regularly on grammar and formatting patches for the manual,
but the glibc manual is quite a large document to read, let alone edit.
 I had some other manual patches submitted, but those may have been a
little (too) early in the editing process, so I'd actually prefer they
be forgotten.  It's taken a while to develop a consistent style and
scope and I'd rather submit a more cohesive set later.  I can
reply/forward with a NACK, if that's the best thing to do.

This patch is fine to submit, as it is independent of the other work I'm
doing.  Thank you for following up.

Rical Jasan
Mike Frysinger March 6, 2015, 5:56 a.m. UTC | #3
On 05 Mar 2015 18:35, ricaljasan wrote:
> OOC, is there a lifetime on submitted patches?

not really.  i happened to not have deleted your e-mails as they looked like no 
one followed up, and i've been clearing out some old unread posts to this list.  
we have patchwork to try and prevent old patches from getting lost/ignored:
	http://patchwork.sourceware.org/project/glibc/list/

> I have been working
> more or less regularly on grammar and formatting patches for the manual,
> but the glibc manual is quite a large document to read, let alone edit.

if you're looking at doing more work, then we probably should make sure you have 
copyright assignment in place.  have you gone through that process yet ?

>  I had some other manual patches submitted, but those may have been a
> little (too) early in the editing process, so I'd actually prefer they
> be forgotten.  It's taken a while to develop a consistent style and
> scope and I'd rather submit a more cohesive set later.  I can
> reply/forward with a NACK, if that's the best thing to do.

ok, i'll delete all the pending ones from you in my inbox and purge them from 
patchwork:
	http://patchwork.sourceware.org/project/glibc/list/?submitter=431

> This patch is fine to submit, as it is independent of the other work I'm
> doing.  Thank you for following up.

i'll push this one since it's just deleting lines and you don't really need 
copyright assignment to *delete* things :).
-mike
Rical Jasan March 6, 2015, 6:25 a.m. UTC | #4
On 03/05/2015 09:56 PM, Mike Frysinger wrote:
> if you're looking at doing more work, then we probably should make sure you have 
> copyright assignment in place.  have you gone through that process yet ?

Nope; probably since the other submissions were never committed.  (Which
does make this my first patch. :)  Happy to get the ball rolling though.

> i'll push this one since it's just deleting lines and you don't really need 
> copyright assignment to *delete* things :).

I thought you reviewed this.  I made at least one line of material
change - now the example function works.  ;)

Thanks for pointing out patchwork, btw.  So much easier to read patches
there than the digest.  And thanks again for following up and taking
care of the loose ends.

Sincerely,
Rical Jasan
Mike Frysinger March 6, 2015, 6:53 a.m. UTC | #5
On 05 Mar 2015 22:25, ricaljasan wrote:
> On 03/05/2015 09:56 PM, Mike Frysinger wrote:
> > if you're looking at doing more work, then we probably should make sure you have 
> > copyright assignment in place.  have you gone through that process yet ?
> 
> Nope; probably since the other submissions were never committed.  (Which
> does make this my first patch. :)  Happy to get the ball rolling though.

https://sourceware.org/glibc/wiki/Contribution%20checklist?highlight=%28checklist%29#FSF_copyright_Assignment

basically follow this form:
http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future

> > i'll push this one since it's just deleting lines and you don't really need 
> > copyright assignment to *delete* things :).
> 
> I thought you reviewed this.  I made at least one line of material
> change - now the example function works.  ;)

sorry, i confused your patch with the one i wrote while reviewing yours
-mike
Rical Jasan March 6, 2015, 8:44 a.m. UTC | #6
On 03/05/2015 10:53 PM, Mike Frysinger wrote:
> On 05 Mar 2015 22:25, ricaljasan wrote:
>> On 03/05/2015 09:56 PM, Mike Frysinger wrote:
>>> if you're looking at doing more work, then we probably should make sure you have 
>>> copyright assignment in place.  have you gone through that process yet ?
>>
>> Nope; probably since the other submissions were never committed.  (Which
>> does make this my first patch. :)  Happy to get the ball rolling though.
> 
> https://sourceware.org/glibc/wiki/Contribution%20checklist?highlight=%28checklist%29#FSF_copyright_Assignment
> 
> basically follow this form:
> http://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/request-assign.future

That process is now started.

> 
>>> i'll push this one since it's just deleting lines and you don't really need 
>>> copyright assignment to *delete* things :).
>>
>> I thought you reviewed this.  I made at least one line of material
>> change - now the example function works.  ;)
> 
> sorry, i confused your patch with the one i wrote while reviewing yours
> -mike
> 

Is that the general approach I should take while fixing the manual,
then?  Cleaner diffs, I suppose, than:

  sed '/subjective cruft/{s/.*/@c &/}'

Rical Jasan
Mike Frysinger March 6, 2015, 9:14 a.m. UTC | #7
On 06 Mar 2015 00:44, ricaljasan wrote:
> Is that the general approach I should take while fixing the manual,
> then?  Cleaner diffs, I suppose, than:
> 
>   sed '/subjective cruft/{s/.*/@c &/}'

honestly, the smaller/more self contained your patches, the more likely they are 
to get reviewed/merged.  larger patches that combine multiple changes into one 
are significantly less likely to get reviewed.  especially changes to the 
manual.  will power is a commodity.
-mike
Rical Jasan March 7, 2015, 7:37 a.m. UTC | #8
On 03/06/2015 01:14 AM, Mike Frysinger wrote:
> honestly, the smaller/more self contained your patches, the more likely they are 
> to get reviewed/merged.  larger patches that combine multiple changes into one 
> are significantly less likely to get reviewed.  especially changes to the 
> manual.  will power is a commodity.

And how.  Not to make this an eternal thread, but you touched on one of
my major editing struggles, and this patch holds a perfect example.
I've managed to narrow the scope of my edits to (what I consider)
essential grammar, and keep a few to-do lists for other clean-ups when
I'm done.  I'm not so concerned that I will have too large a scope of
edits - there are going to be a lot no matter what - but I recognize the
need to keep the diffs easily readable.  This is somewhat for posterity,
esp. given how patchwork displays the thread with the patch, which I can
use for illustration, but I'm also curious if you or any other developer
who might care to weigh in would consider my final conclusion appropriate.

Example.  If I were to redo the patch from this thread, one thing that
bugged me was the reformatting of the entire compatibility note after
the deletion of the first sentence.  Having to read that after a few
months, it took me a few glances back and forth to see what I did -
which wound up only being a change to the first line.  Reformatting the
whole paragraph actually induces a need to inspect the entire thing more
closely than normal, looking for changes that aren't even there, when
one could have simply glanced and seen it all.

I think the prevailing rationale at the time was
obey-the-laws-of-the-land ...and anything you touch, bring into
72-column widths.  I swear I read that once somewhere, specifically for
the manual, and have never been able to find it again.  In reality, the
law of manual-land is something like, "Eh, 72-ish is OK."  Were I to add
new content (not happening in the proofing phase anyway), I would format
my paragraphs uniformly, but I think edits are fine to only touch what
they need and line-lengths be damned. (<-conclusion)

To support my 72-ish hypothesis, I couldn't help writing a quick
histogram in awk.  The following will provide the full graph; attached
is a slice from line lengths of 50 to 85, where you can see a sharp
preference for 72.

awk '
{
 len=length();
 if (len>=0) {
  hist[len]++;
 }
}
END {
 printf "len cnt\n--- ---\n";
 for (i=0; i<length(hist); i++) {
  printf "%3d %d\n", i, hist[i];
 }
}' manual/*.texi
len cnt
--- ---
 50 292
 51 303
 52 277
 53 279
 54 316
 55 306
 56 317
 57 406
 58 458
 59 497
 60 604
 61 600
 62 753
 63 922
 64 1143
 65 1351
 66 1635
 67 2055
 68 2443
 69 2972
 70 3357
 71 3097
 72 3063
 73 621
 74 452
 75 278
 76 184
 77 158
 78 126
 79 77
 80 40
 81 49
 82 37
 83 33
 84 32
 85 28
diff mbox

Patch

diff --git a/manual/errno.texi b/manual/errno.texi
index eb9617e..8d575a7 100644
--- a/manual/errno.texi
+++ b/manual/errno.texi
@@ -1380,6 +1380,8 @@  This variable's value is the name that was used to invoke the program
 running in the current process.  It is the same as @code{argv[0]}.  Note
 that this is not necessarily a useful file name; often it contains no
 directory names.  @xref{Program Arguments}.
+
+This variable is a GNU extension and is declared in @file{errno.h}.
 @end deftypevar
 
 @comment errno.h
@@ -1389,17 +1391,19 @@  This variable's value is the name that was used to invoke the program
 running in the current process, with directory names removed.  (That is
 to say, it is the same as @code{program_invocation_name} minus
 everything up to the last slash, if any.)
+
+This variable is a GNU extension and is declared in @file{errno.h}.
 @end deftypevar
 
 The library initialization code sets up both of these variables before
 calling @code{main}.
 
-@strong{Portability Note:} These two variables are GNU extensions.  If
-you want your program to work with non-GNU libraries, you must save the
-value of @code{argv[0]} in @code{main}, and then strip off the directory
-names yourself.  We added these extensions to make it possible to write
-self-contained error-reporting subroutines that require no explicit
-cooperation from @code{main}.
+@strong{Portability Note:} If you want your program to work with
+non-GNU libraries, you must save the value of @code{argv[0]} in
+@code{main}, and then strip off the directory names yourself.  We
+added these extensions to make it possible to write self-contained
+error-reporting subroutines that require no explicit cooperation from
+@code{main}.
 
 Here is an example showing how to correctly handle failure to open a
 file.  The function @code{open_sesame} tries to open the named file
@@ -1413,6 +1417,8 @@  save it in a local variable instead, because those other library
 functions might overwrite @code{errno} in the meantime.
 
 @smallexample
+#define _GNU_SOURCE
+
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>