[RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Thomas Rast]

This resurrects (finally) the earlier attempt at

  http://mid.gmane.org/cover.1280169048.git.trast@student.ethz.ch

It tries the inverse approach: teaching the script how to find config
variable blocks in each manpage, and then linking them from the main
list.  (Obviously just inserting them into the main list could also
work.)

In other words, it attempts to push out the "original" documentation
of each variable from the main list to the individual manpage, which
is exactly opposite from v1.

This has the advantage that it does not make the source .txt for each
manpage "unreadable by themselves", as the earlier approach did (and
Jonathan noticed).

It also has the advantage that it will shrink config-vars.txt over
time.

Instead of going through all manpages again, I just tacked on a sample
patch as 3/3 that shows what the use/effect could be.  Once we settle
on the direction (v1 or v2) of the refactoring, I'll make more patches
or resurrect the old ones.

I'm afraid 1/3 (semantically unchanged from the equivalent patch in
v1) will again not make it through, so I again pushed this out:

  git://repo.or.cz/git/trast.git t/doc-config-extraction-v2

(incidentally v1 is still there as t/doc-config-extraction).  I based
it on master; 1/3 is prone to conflicts but can easily be recreated
from scratch.


Thomas Rast (3):
  Documentation: Move variables from config.txt to separate file
  Documentation: complete config list from other manpages
  Documentation: move format.* documentation to format-patch

 Documentation/Makefile              |    9 +-
 Documentation/config-vars-src.txt   | 1691 +++++++++++++++++++++++++++++++++
 Documentation/config.txt            | 1748 +----------------------------------
 Documentation/git-format-patch.txt  |   65 ++-
 Documentation/make-config-list.perl |  131 +++
 5 files changed, 1892 insertions(+), 1752 deletions(-)
 create mode 100644 Documentation/config-vars-src.txt
 create mode 100755 Documentation/make-config-list.perl

-- 
1.7.3.1.281.g5da0b

[RFC PATCH v2 3/3] Documentation: move format.* documentation to format-patch

[Thomas Rast]

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/config-vars-src.txt  |   56 -------------------------------
 Documentation/git-format-patch.txt |   65 +++++++++++++++++++++++++++++++++--
 2 files changed, 61 insertions(+), 60 deletions(-)

diff --git a/Documentation/config-vars-src.txt b/Documentation/config-vars-src.txt
index 949259c..e12ff6e 100644
--- a/Documentation/config-vars-src.txt
+++ b/Documentation/config-vars-src.txt
@@ -775,67 +775,11 @@ fetch.unpackLimit::
 	especially on slow filesystems.  If not set, the value of
 	`transfer.unpackLimit` is used instead.
 
-format.attach::
-	Enable multipart/mixed attachments as the default for
-	'format-patch'.  The value can also be a double quoted string
-	which will enable attachments as the default and set the
-	value as the boundary.  See the --attach option in
-	linkgit:git-format-patch[1].
-
-format.numbered::
-	A boolean which can enable or disable sequence numbers in patch
-	subjects.  It defaults to "auto" which enables it only if there
-	is more than one patch.  It can be enabled or disabled for all
-	messages by setting it to "true" or "false".  See --numbered
-	option in linkgit:git-format-patch[1].
-
-format.headers::
-	Additional email headers to include in a patch to be submitted
-	by mail.  See linkgit:git-format-patch[1].
-
-format.to::
-format.cc::
-	Additional recipients to include in a patch to be submitted
-	by mail.  See the --to and --cc options in
-	linkgit:git-format-patch[1].
-
-format.subjectprefix::
-	The default for format-patch is to output files with the '[PATCH]'
-	subject prefix. Use this variable to change that prefix.
-
-format.signature::
-	The default for format-patch is to output a signature containing
-	the git version number. Use this variable to change that default.
-	Set this variable to the empty string ("") to suppress
-	signature generation.
-
-format.suffix::
-	The default for format-patch is to output files with the suffix
-	`.patch`. Use this variable to change that suffix (make sure to
-	include the dot if you want it).
-
 format.pretty::
 	The default pretty format for log/show/whatchanged command,
 	See linkgit:git-log[1], linkgit:git-show[1],
 	linkgit:git-whatchanged[1].
 
-format.thread::
-	The default threading style for 'git format-patch'.  Can be
-	a boolean value, or `shallow` or `deep`.  `shallow` threading
-	makes every mail a reply to the head of the series,
-	where the head is chosen from the cover letter, the
-	`\--in-reply-to`, and the first patch mail, in this order.
-	`deep` threading makes every mail a reply to the previous one.
-	A true boolean value is the same as `shallow`, and a false
-	value disables threading.
-
-format.signoff::
-    A boolean value which lets you enable the `-s/--signoff` option of
-    format-patch by default. *Note:* Adding the Signed-off-by: line to a
-    patch should be a conscious act and means that you certify you have
-    the rights to submit this work under the same open source license.
-    Please see the 'SubmittingPatches' document for further discussion.
-
 gc.aggressiveWindow::
 	The window size parameter used in the delta compression
 	algorithm used by 'git gc --aggressive'.  This defaults
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 9dcafc6..6154e98 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -211,10 +211,9 @@ you can use `--suffix=-patch` to get `0001-description-of-my-change-patch`.
 
 CONFIGURATION
 -------------
-You can specify extra mail header lines to be added to each message,
-defaults for the subject prefix and file suffix, number patches when
-outputting more than one patch, add "To" or "Cc:" headers, configure
-attachments, and sign off patches with configuration variables.
+
+'git format-patch' has a lot of configuration variables.  A short
+example:
 
 ------------
 [format]
@@ -226,8 +225,66 @@ attachments, and sign off patches with configuration variables.
 	cc = <email>
 	attach [ = mime-boundary-string ]
 	signoff = true
+	signature = "Hello, world!\n"
+	thread = shallow
 ------------
 
+The individual variables are as follows:
+
+format.attach::
+	Enable multipart/mixed attachments as the default for
+	'format-patch'.  The value can also be a double quoted string
+	which will enable attachments as the default and set the
+	value as the boundary.  See the --attach option.
+
+format.numbered::
+	A boolean which can enable or disable sequence numbers in patch
+	subjects.  It defaults to "auto" which enables it only if there
+	is more than one patch.  It can be enabled or disabled for all
+	messages by setting it to "true" or "false".  See the --numbered
+	option.
+
+format.headers::
+	Additional email headers to include in a patch to be submitted
+	by mail.
+
+format.to::
+format.cc::
+	Additional recipients to include in a patch to be submitted
+	by mail.  See the --to and --cc options above.
+
+format.subjectprefix::
+	The default for format-patch is to output files with the '[PATCH]'
+	subject prefix. Use this variable to change that prefix.
+
+format.signature::
+	The default for format-patch is to output a signature containing
+	the git version number. Use this variable to change that default.
+	Set this variable to the empty string ("") to suppress
+	signature generation.
+
+format.suffix::
+	The default for format-patch is to output files with the suffix
+	`.patch`. Use this variable to change that suffix (make sure to
+	include the dot if you want it).
+
+format.thread::
+	The default threading style for 'git format-patch'.  Can be
+	a boolean value, or `shallow` or `deep`.  `shallow` threading
+	makes every mail a reply to the head of the series,
+	where the head is chosen from the cover letter, the
+	`\--in-reply-to`, and the first patch mail, in this order.
+	`deep` threading makes every mail a reply to the previous one.
+	A true boolean value is the same as `shallow`, and a false
+	value disables threading.
+
+format.signoff::
+    A boolean value which lets you enable the `-s/--signoff` option of
+    format-patch by default. *Note:* Adding the Signed-off-by: line to a
+    patch should be a conscious act and means that you certify you have
+    the rights to submit this work under the same open source license.
+    Please see the 'SubmittingPatches' document for further discussion.
+
 
 EXAMPLES
 --------
-- 
1.7.3.1.281.g5da0b

Re: [RFC PATCH v2 2/3] Documentation: complete config list from other manpages

[Jakub Narebski]

Dnia piątek 22. października 2010 07:02, Thomas Rast napisał:
> Add an autogeneration script that inserts links to other manpages
> describing config variables, as follows:
> 
> * parse config-vars-src.txt (as it now needs to be renamed) to find
>   out its current contents
> 
> * parse each manpage source (following includes) for config variable
>   headers (the blocks are ignored)
> 
> * assemble a new config-vars.txt that completes the original list with
>   "See linkgit:git-foo[1]" entries for all variables that were not in
>   it.
> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>  Documentation/Makefile              |    9 +-
>  Documentation/config-vars-src.txt   | 1747 +++++++++++++++++++++++++++++++++++
>  Documentation/config-vars.txt       | 1747 -----------------------------------
>  Documentation/make-config-list.perl |  131 +++
>  4 files changed, 1886 insertions(+), 1748 deletions(-)
>  create mode 100644 Documentation/config-vars-src.txt
>  delete mode 100644 Documentation/config-vars.txt
>  create mode 100755 Documentation/make-config-list.perl

Could you please resend this patch using rename detection 
(git format-patch -M)?  It would make it clear what the difference
between config-vars and config-vars-src is, if any.

-- 
Jakub Narebski
Poland

Re: [RFC PATCH v2 1/3] Documentation: Move variables from config.txt to separate file

[Jakub Narebski]

On Fri, 22 Oct 2010, Thomas Rast wrote:

> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>  Documentation/config-vars.txt | 1747 ++++++++++++++++++++++++++++++++++++++++
>  Documentation/config.txt      | 1748 +----------------------------------------
>  2 files changed, 1748 insertions(+), 1747 deletions(-)
>  create mode 100644 Documentation/config-vars.txt

I like it, even if the rest of this series would not get accepted.

-- 
Jakub Narebski
Poland

Re: [RFC PATCH v2 2/3] Documentation: complete config list from other manpages

[Jakub Narebski]

On Fri, 22 Oct 2010 07:02, Thomas Rast wrote:

> Add an autogeneration script that inserts links to other manpages
> describing config variables, as follows:
>
> * parse config-vars-src.txt (as it now needs to be renamed) to find
>   out its current contents
> 
> * parse each manpage source (following includes) for config variable
>   headers (the blocks are ignored)
> 
> * assemble a new config-vars.txt that completes the original list with
>   "See linkgit:git-foo[1]" entries for all variables that were not in
>   it.

So is this script about automatically managing links from git-config
manpage to manpages of individual commands?  Does it mean that for 
something like the following in Documentation/config-vars-src.txt

  foo.bar::
  	Lorem ipsum dolor sit amet, consectetur adipisicing elit,
  	sed do eiusmod tempor incididunt ut labore et dolore magna
  	aliqua.

and if `foo.bar` is referenced in Documentation/git-foo.txt in some way
(from reading source of mentioned autogeneration script it considers
only 'foo.bar::', like in Documentation/git-send-email.txt), then it
adds

  See linkgit:git-foo[1]

at the end of description of variable in Documentation/config-vars-src.txt
(taking into account continuation blocks)?  What about config variables
mentioned in different ways, e.g. '`foo.bar`', like in git-update-index
documentation?  Does it checks that 'See linkgit:git-foo[1]' already
exists, e.g. in extended form 'See linkgit:git-foo[1].  True by default.'?
 
Or is it about automatically creating and updating blocks like this:

  sendemail.aliasesfile::
  sendemail.aliasfiletype::
  sendemail.bcc::
  sendemail.cc::
  [...]
  sendemail.thread::
  sendemail.validate::
  	See linkgit:git-send-email[1] for description.


See also comments below, where I realized what this scipt does...
This really should be in commit message.

> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>  Documentation/Makefile              |    9 +-
>  Documentation/config-vars-src.txt   | 1747 +++++++++++++++++++++++++++++++++++
>  Documentation/config-vars.txt       | 1747 -----------------------------------
>  Documentation/make-config-list.perl |  131 +++
>  4 files changed, 1886 insertions(+), 1748 deletions(-)
>  create mode 100644 Documentation/config-vars-src.txt
>  delete mode 100644 Documentation/config-vars.txt
>  create mode 100755 Documentation/make-config-list.perl

[...]

> diff --git a/Documentation/make-config-list.perl b/Documentation/make-config-list.perl
> new file mode 100755
> index 0000000..f086867
> --- /dev/null
> +++ b/Documentation/make-config-list.perl
> @@ -0,0 +1,131 @@
> +#!/usr/bin/perl

While other helper scripts do not include comments describing them, it
would be nice to have here description what script does and for what it
is used.

Comments in code would also be nice.


The code lacks consistency:

> +	open my $fh, "<", $file or die "cannot open $file: $!";
vs
> +	my $fp;
> +	open $fp, '<', $name or die "open $name failed: $!";

> +	close $fh or die "eh? close failed: $!";
vs
> +	close $fp or die "close $name failed: $!";


> +
> +use strict;
> +use warnings;
> +use Getopt::Long;
> +
> +my %ignore;
> +
> +my $rc = GetOptions(
> +	"mainlist=s" => \my $mainlistfile,
> +	"ignore=s" => sub { $ignore{$_[1]} = 1 },
> +	"no-sort" => \my $no_sort,
> +	);
> +
> +if (!$rc or (!-r $mainlistfile)) {
> +	print "$0 --mainlist=<mainlist> [--ignore=<ignore>...] <asciidoc_manpage>...\n";
> +	exit 1;
> +}

The names <mainlist> (which is file with list of configuration variables
to modify), <ignore> (which is about ignoring some asciidoc_manpage files,
but only when recursing / following linkgit links) doesn't tell us much
by themselves.  It really needs either better names, or comment describing
what they mean, or both.

> +

It would be good to have comment what this subroutine does.  It reads
and parses file with list of config variables and their description,
and returns reference to list of variables, in the order they were in
file, and reference to hash with contents:
  variable => [ lines of description of variable ]

> +sub read_varlist {
> +	my ($file) = @_;
> +
> +	open my $fh, "<", $file or die "cannot open $file: $!";
> +	my (%mainlist, @mainvars);
> +
> +	my ($v, $last_v);
> +	my $in_block = 0;
> +	while (<$fh>) {
> +		if (/^(\S+)::/) {
> +			$v = lc $1;
> +			$in_block = 0;
> +			push @{$mainlist{$v}}, $_;
> +			push @mainvars, $v;

O.K., that is easy to understand (if one remembers about autovivification
in Perl).  But shouldn't we check if we are not in literal block, just
in case?

> +		} elsif (/^$/ && !$in_block) {
> +			if (defined $last_v && !$#{$mainlist{$last_v}}) {
> +				$mainlist{$last_v} = $mainlist{$v};
> +			}
> +			$last_v = $v;

What is this branch / this code about?

> +		} elsif (defined $v) {
> +			push @{$mainlist{$v}}, $_;
> +			$in_block = !$in_block if /^--$/;

O.K., easy to understand.

> +		}
> +	}
> +
> +	close $fh or die "eh? close failed: $!";
> +
> +	return \%mainlist, \@mainvars;
> +}
> +

It would be nice to have description what those variables should contain
(what structure would they have).

> +my %vars;
> +my %sections;
> +

It would be good to have comment what this subroutine does; better name
than 'read_file' would also be good.

This subroutine does *two* things: finds _manual_ section that manpage
belongs to, to create correct link (e.g. 1 for git-send-email(1), 5 for
gitattributes(5), 7 for gitcli(7)), and finds all manpages that refer
to config variable using '^foo.bar::': $vars{'foo.bar'} = [ 'git-foo', ... ]

It would also automatically follow includes, excluding ignored files
from following 'include::<filename>[]' links.

> +sub read_file {
> +	my ($name, $main_name) = @_;

$name is name of current file, $main_name is name of file that included
it, isn't it?

> +	if (!defined $main_name) {
> +		$main_name = $name;
> +	}
> +	my $manpage_name = $main_name;
> +	$manpage_name =~ s/\.txt//;
> +	my $fp;
> +	open $fp, '<', $name or die "open $name failed: $!";
> +	while (<$fp>) {
> +		if ($. < 5 && /^$manpage_name\((\d+)\)/) {
> +			$sections{$manpage_name} = $1;
> +		}

Shouldn't it be always first line of manpage?

> +		if (/^([a-z0-9]+\.[a-z<>0-9.]+)::/) {
> +			push @{$vars{$1}}, $manpage_name;
> +		}
> +		if (/^include::\s*(\S+)\s*\[\]/
> +		    && !exists $ignore{$1}) {
> +			read_file($1, $main_name);
> +		}
> +	}
> +	close $fp or die "close $name failed: $!";
> +}
> +
> +foreach my $name (@ARGV) {
> +	read_file($name);
> +}
> +
> +my ($mainlist, $mainvars) = read_varlist($mainlistfile);
> +
> +my @all_keys = @$mainvars;
> +foreach my $k (keys %vars) {
> +	if (!exists $mainlist->{$k}) {
> +		push @all_keys, $k;
> +	}

Nice, so it would detect config variables which are missing from
config-vars-src.txt

> +}
> +
> +@all_keys = sort @all_keys unless $no_sort;
> +
> +my %out;
> +foreach my $k (@all_keys) {
> +	if (exists $mainlist->{$k}) {
> +		push @{$out{$k}}, @{$mainlist->{$k}}, "\n";

Ah, so it doesn't add 'See linkgit:git-foo[1]' if 'foo.bar' is present
in config-vars-src.txt, but only generate notice about config variable
in git-config manpage, refering to the page where it is defined and
described.

This should be make more clear in commit message.


> +	} elsif (exists $vars{$k}) {
> +		push @{$out{$k}}, $k . "::\n";
> +		push @{$out{$k}}, "\tSee ";

Wouldn't this result in something like

  sendemail.aliasesfile::
  	See linkgit:git-send-email[1]

  sendemail.aliasfiletype::
  	See linkgit:git-send-email[1]

instead of

  sendemail.aliasesfile::
  sendemail.aliasfiletype::
  	See linkgit:git-send-email[1]

in the case where config-vars-src.txt is missing some variable?
Ah, I see that it is collapsed later.

> +		my $sep = " ";

Not $sep = "", or "\tSee"?  Otherwise you would get "\tSee  linkgit:git-foo[1]"
with double space.

> +		foreach my $p (sort @{$vars{$k}}) {
> +			next if ($p =~ /$mainlistfile/);

> +			if (!exists $sections{$p}) {
> +				warn "section for $p unknown";
> +				$sections{$p} = 1;
> +			}
> +			push @{$out{$k}}, $sep . "linkgit:" . $p . "[" . $sections{$p} . "]";

A bit of possible (but perhaps not necessary) refactoring:

  +			push @{$out{$k}}, $sep . gen_linkgit($p);

(or something like that).


Besides wouldn't it be better to do collapsing based on data, not on 
formatted string?

> +			$sep = ", ";
> +		}
> +		push @{$out{$k}}, ".\n\n";
> +	} else {
> +		die "can't happen: $k not in any source";
> +	}
> +}
> +


A comment what this loop does would be nice.

Note that we don't really have to do this collapsing for existing 
contents; only for contents that was generated by this script.

> +for (my $i = 0; $i < $#all_keys; $i++) {

> +	next if $#{$out{$all_keys[$i]}} != $#{$out{$all_keys[$i+1]}};
> +	my $same = 1;
> +	for (my $j = 1; $j <= $#{$out{$all_keys[$i]}}; $j++) {
> +		if ($out{$all_keys[$i]}[$j] ne $out{$all_keys[$i+1]}[$j]) {
> +			$same = 0;
> +			last;
> +		}
> +	}

A bit of possible (but perhaps not necessary) refactoring:

  +	next unless eq_deeply($out{$all_keys[$i]}, $out{$all_keys[$i+1]});

(or eq_deeply_arr).

> +	if ($same) {
> +		$out{$all_keys[$i]} = [$out{$all_keys[$i]}[0]];
> +	}
> +}

I really think that we can do this in an easier, more elegant, and not
that convoluted way.

First, we don't really need to store 'foo.bar::' as first element in
$out{'foo.bar'}.  Second, then compacting is just grouping hash by value.

> +
> +foreach my $k (@all_keys) {
> +	print @{$out{$k}};
> +}
> -- 
> 1.7.3.1.281.g5da0b
> 
> 

-- 
Jakub Narebski
Poland

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Jakub Narebski]

On Fri, 22 Oct 2010, Thomas Rast wrote:

> Thomas Rast (3):
>   Documentation: Move variables from config.txt to separate file
>   Documentation: complete config list from other manpages
>   Documentation: move format.* documentation to format-patch

Note that patches 1/3 and 2/3 didn't made it to git mailing list
beause they were too large for vger anti-SPAM filter.  The 2/3
can use rename detection (-M) to be much smaller.

-- 
Jakub Narebski
Poland

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Jeff King]

On Fri, Oct 22, 2010 at 07:02:28AM +0200, Thomas Rast wrote:

> This resurrects (finally) the earlier attempt at
> 
>   http://mid.gmane.org/cover.1280169048.git.trast@student.ethz.ch
> 
> It tries the inverse approach: teaching the script how to find config
> variable blocks in each manpage, and then linking them from the main
> list.  (Obviously just inserting them into the main list could also
> work.)
> 
> In other words, it attempts to push out the "original" documentation
> of each variable from the main list to the individual manpage, which
> is exactly opposite from v1.

Thanks for working on this.

I think this approach is much saner for writers and readers of the
source files, and I think the output is much better (in particular, the
giant list having "see X" pointers instead of the actual description
blocks).

Your 2/3 doesn't seem to have made it through to the list, but I pulled
from your repository and looked at it. I have two comments on the
approach:

  1. It looks like you're more or less just parsing "::" list keys from
     all of the manpages. This seems somewhat fragile, as there could be
     other non-config lists. Can we at least restrict it to
     CONFIGURATION sections? It would be nice if we could put in some
     kind of semantic markup, but I'm not sure how painful that would be
     with asciidoc (we could always resort to comments in the source,
     but that would probably get unreadable quickly).

  2. You recursively follow includes via include::. This means that the
     make rule is not accurate. E.g., try:

       rm cmds-mainporcelain.txt config-vars.txt
       make config-vars.txt

     which should fail, as we actually depend on cmds-mainporcelain.txt.
     Doing it accurately and automatically would mean making .depend
     files for make.

     But I wonder if the recursive lookup is really required. Some of
     the includes with config files can just go away (e.g.,
     merge-config.txt is included only by config-vars-src.txt and
     git-merge.txt; it can just be merged straight into git-merge.txt
     once this system exists). Others, like pretty-formats.txt, should,
     IMHO, get their own user-visible page. Right now with your script
     you get[1]:

       format.pretty::
               The default pretty format for log/show/whatchanged command,
               See linkgit:git-log[1], linkgit:git-show[1],
               linkgit:git-whatchanged[1].

     but I would rather see[2]:

       format.pretty::
               See linkgit:gitpretty[7].

     [1]: I assume the single line of block description is an error in
          your script.

     [2]: Actually, as I mentioned a long time ago, I think it would be
          nicer to have a table like:

             format.attach         linkgit:git-format-patch[1]
             format.cc             linkgit:git-format-patch[1]
             format.headers        linkgit:git-format-patch[1]
             format.pretty         linkgit:gitpretty[7]

> I'm afraid 1/3 (semantically unchanged from the equivalent patch in
> v1) will again not make it through, so I again pushed this out:
> 
>   git://repo.or.cz/git/trast.git t/doc-config-extraction-v2

Yeah, I saw neither 1/3 nor 2/3 on the list.

-Peff

[PATCH] Documentation: complete config list from other manpages

[Thomas Rast]

Add an autogeneration script that inserts links to other manpages
describing config variables, as follows:

* parse config-vars-src.txt (as it now needs to be renamed) to find
  out its current contents

* parse each manpage source (following includes) for config variable
  headers (the blocks are ignored)

* assemble a new config-vars.txt that completes the original list with
  "See linkgit:git-foo[1]" entries for all variables that were not in
  it.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Jakub Narebski wrote:
> Could you please resend this patch using rename detection 
> (git format-patch -M)?  It would make it clear what the difference
> between config-vars and config-vars-src is, if any.

Right, sorry about that.  There wasn't supposed to be any; I'm
resending what I pushed out for everyone's convenience, but the stray
change will be gone in the final version.


 Documentation/Makefile                             |    9 ++-
 .../{config-vars.txt => config-vars-src.txt}       |    2 +-
 Documentation/make-config-list.perl                |  131 ++++++++++++++++++++
 3 files changed, 140 insertions(+), 2 deletions(-)
 rename Documentation/{config-vars.txt => config-vars-src.txt} (99%)
 create mode 100755 Documentation/make-config-list.perl

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e117bc4..747b849 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -125,7 +125,7 @@ endif
 
 SHELL_PATH ?= $(SHELL)
 # Shell quote;
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) #'
 
 #
 # Please note that there is a minor bug in asciidoc.
@@ -320,6 +320,13 @@ howto-index.txt: howto-index.sh $(wildcard howto/*.txt)
 	'$(SHELL_PATH_SQ)' ./howto-index.sh $(wildcard howto/*.txt) >$@+ && \
 	mv $@+ $@
 
+config-vars.txt: config-vars-src.txt $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
+	./make-config-list.perl --mainlist=config-vars-src.txt \
+		--ignore=config-vars.txt \
+		$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT) \
+		> config-vars.txt+ && \
+	mv config-vars.txt+ config-vars.txt
+
 $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
 	$(QUIET_ASCIIDOC)$(ASCIIDOC) $(ASCIIDOC_EXTRA) -b xhtml11 $*.txt
 
diff --git a/Documentation/config-vars.txt b/Documentation/config-vars-src.txt
similarity index 99%
rename from Documentation/config-vars.txt
rename to Documentation/config-vars-src.txt
index a8d37a7..949259c 100644
--- a/Documentation/config-vars.txt
+++ b/Documentation/config-vars-src.txt
@@ -936,7 +936,7 @@ gitcvs.dbname::
 
 gitcvs.dbdriver::
 	Used Perl DBI driver. You can specify any available driver
-        for this here, but it might not work. git-cvsserver is tested
+	for this here, but it might not work. git-cvsserver is tested
 	with 'DBD::SQLite', reported to work with 'DBD::Pg', and
 	reported *not* to work with 'DBD::mysql'. Experimental feature.
 	May not contain double colons (`:`). Default: 'SQLite'.
diff --git a/Documentation/make-config-list.perl b/Documentation/make-config-list.perl
new file mode 100755
index 0000000..f086867
--- /dev/null
+++ b/Documentation/make-config-list.perl
@@ -0,0 +1,131 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Getopt::Long;
+
+my %ignore;
+
+my $rc = GetOptions(
+	"mainlist=s" => \my $mainlistfile,
+	"ignore=s" => sub { $ignore{$_[1]} = 1 },
+	"no-sort" => \my $no_sort,
+	);
+
+if (!$rc or (!-r $mainlistfile)) {
+	print "$0 --mainlist=<mainlist> [--ignore=<ignore>...] <asciidoc_manpage>...\n";
+	exit 1;
+}
+
+sub read_varlist {
+	my ($file) = @_;
+
+	open my $fh, "<", $file or die "cannot open $file: $!";
+	my (%mainlist, @mainvars);
+
+	my ($v, $last_v);
+	my $in_block = 0;
+	while (<$fh>) {
+		if (/^(\S+)::/) {
+			$v = lc $1;
+			$in_block = 0;
+			push @{$mainlist{$v}}, $_;
+			push @mainvars, $v;
+		} elsif (/^$/ && !$in_block) {
+			if (defined $last_v && !$#{$mainlist{$last_v}}) {
+				$mainlist{$last_v} = $mainlist{$v};
+			}
+			$last_v = $v;
+		} elsif (defined $v) {
+			push @{$mainlist{$v}}, $_;
+			$in_block = !$in_block if /^--$/;
+		}
+	}
+
+	close $fh or die "eh? close failed: $!";
+
+	return \%mainlist, \@mainvars;
+}
+
+my %vars;
+my %sections;
+
+sub read_file {
+	my ($name, $main_name) = @_;
+	if (!defined $main_name) {
+		$main_name = $name;
+	}
+	my $manpage_name = $main_name;
+	$manpage_name =~ s/\.txt//;
+	my $fp;
+	open $fp, '<', $name or die "open $name failed: $!";
+	while (<$fp>) {
+		if ($. < 5 && /^$manpage_name\((\d+)\)/) {
+			$sections{$manpage_name} = $1;
+		}
+		if (/^([a-z0-9]+\.[a-z<>0-9.]+)::/) {
+			push @{$vars{$1}}, $manpage_name;
+		}
+		if (/^include::\s*(\S+)\s*\[\]/
+		    && !exists $ignore{$1}) {
+			read_file($1, $main_name);
+		}
+	}
+	close $fp or die "close $name failed: $!";
+}
+
+foreach my $name (@ARGV) {
+	read_file($name);
+}
+
+my ($mainlist, $mainvars) = read_varlist($mainlistfile);
+
+my @all_keys = @$mainvars;
+foreach my $k (keys %vars) {
+	if (!exists $mainlist->{$k}) {
+		push @all_keys, $k;
+	}
+}
+
+@all_keys = sort @all_keys unless $no_sort;
+
+my %out;
+foreach my $k (@all_keys) {
+	if (exists $mainlist->{$k}) {
+		push @{$out{$k}}, @{$mainlist->{$k}}, "\n";
+	} elsif (exists $vars{$k}) {
+		push @{$out{$k}}, $k . "::\n";
+		push @{$out{$k}}, "\tSee ";
+		my $sep = " ";
+		foreach my $p (sort @{$vars{$k}}) {
+			next if ($p =~ /$mainlistfile/);
+			if (!exists $sections{$p}) {
+				warn "section for $p unknown";
+				$sections{$p} = 1;
+			}
+			push @{$out{$k}}, $sep . "linkgit:" . $p . "[" . $sections{$p} . "]";
+			$sep = ", ";
+		}
+		push @{$out{$k}}, ".\n\n";
+	} else {
+		die "can't happen: $k not in any source";
+	}
+}
+
+for (my $i = 0; $i < $#all_keys; $i++) {
+	next if $#{$out{$all_keys[$i]}} != $#{$out{$all_keys[$i+1]}};
+	my $same = 1;
+	for (my $j = 1; $j <= $#{$out{$all_keys[$i]}}; $j++) {
+		if ($out{$all_keys[$i]}[$j] ne $out{$all_keys[$i+1]}[$j]) {
+			$same = 0;
+			last;
+		}
+	}
+	if ($same) {
+		$out{$all_keys[$i]} = [$out{$all_keys[$i]}[0]];
+	}
+}
+
+foreach my $k (@all_keys) {
+	print @{$out{$k}};
+}
-- 
1.7.3.2.239.gbefc4

Re: [PATCH] Documentation: complete config list from other manpages

[Jakub Narebski]

Thomas Rast wrote:

> ---
> 
> Jakub Narebski wrote:
> > Could you please resend this patch using rename detection 
> > (git format-patch -M)?  It would make it clear what the difference
> > between config-vars and config-vars-src is, if any.
> 
> Right, sorry about that.  There wasn't supposed to be any; I'm
> resending what I pushed out for everyone's convenience, but the stray
> change will be gone in the final version.

And because it is now below vger's anti-SPAM limit on size of message,
it is now visible at git@vger.kernel.org

-- 
Jakub Narebski
Poland

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Thomas Rast]

Jeff King wrote:
>   1. It looks like you're more or less just parsing "::" list keys from
>      all of the manpages. This seems somewhat fragile, as there could be
>      other non-config lists. Can we at least restrict it to
>      CONFIGURATION sections? It would be nice if we could put in some
>      kind of semantic markup, but I'm not sure how painful that would be
>      with asciidoc (we could always resort to comments in the source,
>      but that would probably get unreadable quickly).

I figured for consistency and ease of lookup *all* configuration docs
should name the variables in the same format.  It can still be helpful
to mention them elsewhere, e.g. in the option documentations, but the
main docs should be a CONFIGURATION section formatted like this.

Or do you think that would be a bad thing?

As for false positives, we could do the CONFIGURATION but in any case
I was hoping to avoid a special markup by using an asciidoc markup to
mark false positives if they arise (there currently aren't any).
E.g., it should be possible to make a {noconfig} attribute that
expands to nothing or so.  [Then again the same trick could be used
for all configs...]

>      [2]: Actually, as I mentioned a long time ago, I think it would be
>           nicer to have a table like:
> 
>              format.attach         linkgit:git-format-patch[1]
>              format.cc             linkgit:git-format-patch[1]
>              format.headers        linkgit:git-format-patch[1]
>              format.pretty         linkgit:gitpretty[7]

True, you said that.  I'll hack it into this format, since it's easy
to do once the parsers are stable and can then just say something like
"herein" for all the ones actually in git-config(1).

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH] Documentation: complete config list from other manpages

[Jakub Narebski]

This is second review, with the same comments ad the one before,
and containing proposal for simpler autogeneration script.

On Sat, 24 Oct 2010, Thomas Rast wrote:

> Add an autogeneration script that inserts links to other manpages
> describing config variables,

Actualy it is "Add ... and run it to generate config-vars.txt",
or "Create ... and use it to ...", isn't it.

> as follows: 
> 
> * parse config-vars-src.txt (as it now needs to be renamed) to find
>   out its current contents
> 
> * parse each manpage source (following includes) for config variable
>   headers (the blocks are ignored)
> 
> * assemble a new config-vars.txt that completes the original list with
>   "See linkgit:git-foo[1]" entries for all variables that were not in
>   it.

I find this itemized list slightly hard to read (as you can see by the
confusion shown in previous review of this patch).  I think it would be
better to describe _goal_ of this script first, and only then how it 
does that.

For example:

  The Documentation/make-config-list.perl script finds all config
  variables that are defined in individual manpages that are missing
  from the list of all config variable (in git-config(1) manpage).
  It then generates stub[1] documentation for those missing config
  variables at appropriate place in Documentation/config-vars.txt,
  using the following form:

    foo.bar::
    foo.baz::
    	See linkgit:git-foo[1].

  It does that by parsing config-vars-src.txt (now source for 
  config-vars.txt) getting list of all config variables to find later
  which ones are missing, parsing each manpage source provided on 
  command line (following includes) for config variable headers to
  find which config variables are described directly on individual
  manpages, then it assembles config-vars.txt with missing variables
  added[2].

  NOTE that as a side-effect list of config variables would be sorted
  in alphabetical order[3]

[1] Not a best word, but I couldn't think of a better one.
[2] This paragraph could be done as itemized list, as before.
[3] In my proposal it is not necessary; note that some of config 
    variables are grouped _by function_ to be close together, and
    your script would destroy that.

> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
> 
> Jakub Narebski wrote:
> > Could you please resend this patch using rename detection 
> > (git format-patch -M)?  It would make it clear what the difference
> > between config-vars and config-vars-src is, if any.
> 
> Right, sorry about that.  There wasn't supposed to be any; I'm
> resending what I pushed out for everyone's convenience, but the stray
> change will be gone in the final version.

Stray changes (there are 2 of them).

>  Documentation/Makefile                             |    9 ++-
>  .../{config-vars.txt => config-vars-src.txt}       |    2 +-
>  Documentation/make-config-list.perl                |  131 ++++++++++++++++++++
>  3 files changed, 140 insertions(+), 2 deletions(-)
>  rename Documentation/{config-vars.txt => config-vars-src.txt} (99%)
>  create mode 100755 Documentation/make-config-list.perl
> 
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index e117bc4..747b849 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -125,7 +125,7 @@ endif
>  
>  SHELL_PATH ?= $(SHELL)
>  # Shell quote;
> -SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
> +SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) #'
>  
>  #
>  # Please note that there is a minor bug in asciidoc.

Stray change, doesn't belong in this commit (and probably should be
skipped altogether).

BTW. there was a proposal to do something like that by saving quote
in a variable, and using it in shell quoting...

> @@ -320,6 +320,13 @@ howto-index.txt: howto-index.sh $(wildcard howto/*.txt)
>  	'$(SHELL_PATH_SQ)' ./howto-index.sh $(wildcard howto/*.txt) >$@+ && \
>  	mv $@+ $@
>  
> +config-vars.txt: config-vars-src.txt $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
> +	./make-config-list.perl --mainlist=config-vars-src.txt \
> +		--ignore=config-vars.txt \
> +		$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT) \
> +		> config-vars.txt+ && \


Very, very minor nitpick: wouldn't it read better as the following?

  +	./make-config-list.perl \
  +		--mainlist=config-vars-src.txt \
  +		--ignore=config-vars.txt \
  +		$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT) \
  +		> config-vars.txt+ && \

> +		> config-vars.txt+ && \
> +	mv config-vars.txt+ config-vars.txt
> +
>  $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
>  	$(QUIET_ASCIIDOC)$(ASCIIDOC) $(ASCIIDOC_EXTRA) -b xhtml11 $*.txt
>  
> diff --git a/Documentation/config-vars.txt b/Documentation/config-vars-src.txt
> similarity index 99%
> rename from Documentation/config-vars.txt
> rename to Documentation/config-vars-src.txt
> index a8d37a7..949259c 100644
> --- a/Documentation/config-vars.txt
> +++ b/Documentation/config-vars-src.txt
> @@ -936,7 +936,7 @@ gitcvs.dbname::
>  
>  gitcvs.dbdriver::
>  	Used Perl DBI driver. You can specify any available driver
> -        for this here, but it might not work. git-cvsserver is tested
> +	for this here, but it might not work. git-cvsserver is tested
>  	with 'DBD::SQLite', reported to work with 'DBD::Pg', and
>  	reported *not* to work with 'DBD::mysql'. Experimental feature.
>  	May not contain double colons (`:`). Default: 'SQLite'.

Stray change (spaces to tab), which should be done as separate 
"whitespace cleanup" patch.

> diff --git a/Documentation/make-config-list.perl b/Documentation/make-config-list.perl
> new file mode 100755
> index 0000000..f086867
> --- /dev/null
> +++ b/Documentation/make-config-list.perl
> @@ -0,0 +1,131 @@
> +#!/usr/bin/perl
> +
> +use strict;
> +use warnings;
> +use Getopt::Long;
> +
> +my %ignore;
> +
> +my $rc = GetOptions(
> +	"mainlist=s" => \my $mainlistfile,
> +	"ignore=s" => sub { $ignore{$_[1]} = 1 },
> +	"no-sort" => \my $no_sort,
> +	);

Errr, wouldn't it be better to do it this way:

  +my $sort = 1;
  +my $rc = GetOptions(
  +	"mainlist=s" => \my $mainlistfile,
  +	"ignore=s" => \my @ignore,
  +	"sort!" => \$sort,
  +);
  +my %ignore = map { $_ => 1 } @ignore;

Though it is not necessary; I guess it is a matter of taste.

BTW. does your sript work correctly with `--no-sort`?

> +
> +if (!$rc or (!-r $mainlistfile)) {
> +	print "$0 --mainlist=<mainlist> [--ignore=<ignore>...] <asciidoc_manpage>...\n";
> +	exit 1;
> +}

I'd rather you did a better error handling:

  +if (!$rc || !defined $mainlistfile) {
  +	print "$0 --mainlist=<mainlist> [--ignore=<ignore>...] <asciidoc_manpage>...\n";
  +	exit 1;
  +}

and leave handling '!-r $mainlistfile' to read_varlist() subroutine.

> +

This subroutine needs description, I think; at least oneline comment
describing it.

> +sub read_varlist {
> +	my ($file) = @_;
> +
> +	open my $fh, "<", $file or die "cannot open $file: $!";

You are a bit inconsistent here.  read_varlist() has

   	open my $fh, "<", $file or die "cannot open $file: $!";

while read_file() has

   	my $fp;
   	open $fp, '<', $name or die "open $name failed: $!";

Note that you can use the same name of filehandle (e.g. $fh), as it is
local to the subroutine.

> +	my (%mainlist, @mainvars);
> +
> +	my ($v, $last_v);
> +	my $in_block = 0;

I would done this differently, and IMHO in a simpler way.  Instead of
generating %mainvars structure, that holds for each variable the contents
of its description (in the form of list of lines), adding missing 
variables to it, and then (re)generating config-vars.txt from it:

  --mainlist=config-vars-src.txt
          |
          v
  @mainlist, %mainvars
          |
          |  <-- %vars <-- <asciidoc_manpage>...
          |
          v
  @all_keys, %out
          |
          v
  > config-vars.txt+

wouldn't it be better to simply store where description of each variable
begins in a file (line number, or file position), and then use it to 
inject generated description for missing variables in correct place?

> +	while (<$fh>) {
> +		if (/^(\S+)::/) {
> +			$v = lc $1;

Note: because you are (re)generating config-vars.txt from those data
structures this has an unfortunate side-effect of lowercasing all config
variable names in final config-bars.txt, e.g. it would replace
`core.ignoreCygwinFSTricks` with `core.ignorecygwinfstricks`.

> +			$in_block = 0;
> +			push @{$mainlist{$v}}, $_;
> +			push @mainvars, $v;

So in the proposed implementation it would simply be:

  +			$mainlist{lc($v)} = $.;
  +			push @mainvars, $v;

I think lc($v) is needed only in %mainlist keys.

> +		} elsif (/^$/ && !$in_block) {
> +			if (defined $last_v && !$#{$mainlist{$last_v}}) {
> +				$mainlist{$last_v} = $mainlist{$v};
> +			}
> +			$last_v = $v;

Besides, if I understand it correctly, that any empty line which does
not precede config variable definition, and is not inside literal block,
is a *bug* in AsciiDoc markup.  It should be "^+$" (continuation),
not "^$".

This branch of conditional would be simply not required in proposed
alternate solution.


By the way, why you do not follow includes in this file (for example
'include::merge-config.txt[]'?

> +		} elsif (defined $v) {
> +			push @{$mainlist{$v}}, $_;
> +			$in_block = !$in_block if /^--$/;
> +		}

This branch of conditional would be simply not required in proposed
alternate solution.

Well... assuming that we do not have lines matching /^(\S+)::/ in literal
blocks (which we don't, and which you also assume).

> +	}
> +
> +	close $fh or die "eh? close failed: $!";

You are a bit inconsistent here.  read_varlist() has

   	close $fh or die "eh? close failed: $!";

while read_file() has

   	close $fp or die "close $name failed: $!";

> +
> +	return \%mainlist, \@mainvars;
> +}
> +
> +my %vars;
> +my %sections;

I think those variables needs better names, for example
%vars_manpages (as it is mapping from config variable name found in
manpage to list of manpages it was found in) and %manpage_section
(as it is mapping from manpage name to man section, for purpose of
linking).

Though perhaps %vars could be left as is...

> +

This subroutine needs description, I think; at least oneline comment
describing it.  And probably better name, like read_manpage().

> +sub read_file {
> +	my ($name, $main_name) = @_;
> +	if (!defined $main_name) {
> +		$main_name = $name;
> +	}
> +	my $manpage_name = $main_name;
> +	$manpage_name =~ s/\.txt//;

Wouldn't it be simpler to pass $filename, $manpage as arguments to
read_file(), rather than $filename/$name, $main_name?

It would read as:

  +# Parse manpages for config variables and sections, following includes
  +sub read_manpage {
  +	my ($filename, $manpage) = @_;
  +	if (!defined $manpage) {
  +		$manpage = $filename;
  +		$manpage =~ s/\.txt//;
  +	}

And of course s/$name/$filename/ and s/$manpage_name/$manpage/.  Well, 
the change of variable names is not really necessary.

> +	my $fp;
> +	open $fp, '<', $name or die "open $name failed: $!";
> +	while (<$fp>) {
> +		if ($. < 5 && /^$manpage_name\((\d+)\)/) {
> +			$sections{$manpage_name} = $1;
> +		}
> +		if (/^([a-z0-9]+\.[a-z<>0-9.]+)::/) {
> +			push @{$vars{$1}}, $manpage_name;
> +		}
> +		if (/^include::\s*(\S+)\s*\[\]/
> +		    && !exists $ignore{$1}) {
> +			read_file($1, $main_name);
> +		}
> +	}
> +	close $fp or die "close $name failed: $!";
> +}
> +
> +foreach my $name (@ARGV) {
> +	read_file($name);
> +}

O.K. nicely done.  This would be unchanged in proposed alternate 
solution.

> +
> +my ($mainlist, $mainvars) = read_varlist($mainlistfile);
> +
> +my @all_keys = @$mainvars;
> +foreach my $k (keys %vars) {
> +	if (!exists $mainlist->{$k}) {
> +		push @all_keys, $k;
> +	}
> +}

I propose the following:

  +my @missing_vars = ();
  +foreach my $k (keys %vars) {
  +	if (!exists $mainlist->{$k}) {
  +		push @missing_vars, $k;
  +	}
  +}

or even

  +foreach my $k (keys %vars) {
  +	if (exists $mainlist->{$k}) {
  +		delete $vars{$k};
  +	}
  +}

so that %vars would now contain only missing variables.

> +
> +@all_keys = sort @all_keys unless $no_sort;

So if you pass `--no-sort` to this script, all missing variables would
be put at the end, and if you pass it, the order of variables would be
changed, isn't it?

> +
> +my %out;
> +foreach my $k (@all_keys) {
> +	if (exists $mainlist->{$k}) {
> +		push @{$out{$k}}, @{$mainlist->{$k}}, "\n";
> +	} elsif (exists $vars{$k}) {
> +		push @{$out{$k}}, $k . "::\n";
> +		push @{$out{$k}}, "\tSee ";
> +		my $sep = " ";
> +		foreach my $p (sort @{$vars{$k}}) {
> +			next if ($p =~ /$mainlistfile/);
> +			if (!exists $sections{$p}) {
> +				warn "section for $p unknown";
> +				$sections{$p} = 1;
> +			}
> +			push @{$out{$k}}, $sep . "linkgit:" . $p . "[" . $sections{$p} . "]";
> +			$sep = ", ";
> +		}
> +		push @{$out{$k}}, ".\n\n";
> +	} else {
> +		die "can't happen: $k not in any source";
> +	}
> +}
> +
> +for (my $i = 0; $i < $#all_keys; $i++) {
> +	next if $#{$out{$all_keys[$i]}} != $#{$out{$all_keys[$i+1]}};
> +	my $same = 1;
> +	for (my $j = 1; $j <= $#{$out{$all_keys[$i]}}; $j++) {
> +		if ($out{$all_keys[$i]}[$j] ne $out{$all_keys[$i+1]}[$j]) {
> +			$same = 0;
> +			last;
> +		}
> +	}
> +	if ($same) {
> +		$out{$all_keys[$i]} = [$out{$all_keys[$i]}[0]];
> +	}
> +}
> +
> +foreach my $k (@all_keys) {
> +	print @{$out{$k}};
> +}

Instead of that, I would propose the following UNTESTED code:

-- 8< --
%vars = find_insertion_points($mainlist, \%vars);
# %vars has now the following format:
# 'name' => {
# 	'lineno' => line of $mainfile where description should be inserted
# 	'manpages' => [ <manpage_name>... ]
# }

# group by insertion point
my %insertion_point; ## or just %insert
foreach my $v (keys %vars) {
	push @{$insertion_point{$v->{'lineno'}}}, $v;
}

open my $fh, '<', $mainfile
	or die "cannot open '$mainfile': $!";
while (my $line = <$fh>) {
	if (exists $insertion_point{$.}) {
		print document_vars($insertion_point{$.}, \%vars);
		print "\n";
	}
	print $line;
}
# special case: insertion after last line in $mainfile
print document_vars($insertion_point{-1}, \%vars)
	if exists $insertion_point{-1};
close $fh
	or die "cannot close '$mainfile': $!";

exit 0;

## those subroutines should probably be defined earlier

sub find_insertion_points {
	my ($mainlist, $missing_vars) = @_;

	my %res; ## not required if we modify %$missing_vars in-place

	my %all_vars = (%$mainlist, %$missing_vars);
	my $lineno = -1; # means after last line

	# reverse order because we want to find a place before which to insert
	# generated documentation; it is easy to find where description
	# of variable begins, but in general harder to find where it ends.
	foreach my $key (reverse sort { lc($a) cmp lc($b) } keys %all_vars) {
		my $val = $all_vars{$key};
		if (ref $val) {
			# this came from %$missing_vars
			$res{$v} = { 'lineno' => $lineno, 'manpages' => $val };
			## or $missing_vars->{$v} instead of $res{$v} if in-place
		} else {
			# this came from %$mainlist
			$lineno = $value;
		}
	}
	return %res; ## not required if in-place
}

## this is single place where we need to change code if output changes,
## e.g. to
##
##   format.attach         linkgit:git-format-patch[1]
##   format.cc             linkgit:git-format-patch[1]
##   format.headers        linkgit:git-format-patch[1]
##   format.pretty         linkgit:gitpretty[7]
sub document_vars {
	my ($keylist, $vars) = @_;

	# generate output for each key now, to compact output, because it is
	# easier to compare strings than arrays
	foreach my $k (@$keylist) {
		$vars->{$k}{'output'} = "\tSee: ".gen_links($vars->{$k}{'manpages'}).".\n";
	}

	my $output = '';
	## the below could be done in different way, e.g. using new array for
	## sorted keys, and using 'my $k = pop @keys' etc.
	$keylist = [ sort @$keylist ];
	for (my $i = 0; $i < @$keylist; $i++) {
		my $k = $keylist->[$i];
		$output .= "$k::\n";
		unless ($i <= $#@$keylist &&
		        $keylist->[$i]{'output'} eq $keylist->[$i+1]{'output'}) {
			$output .= $k->{'output'};
		}
	}

	return $output;
}

## we can make it smarter in the future, for example:
## 'linkgit:git-foo[1], linkgit:git-bar[1] and linkgit:git-baz[1] for details';
sub gen_links {
	my $manpages = shift;
	return join(", ", map { linkgit($_) } @$manpages);
}

sub linkgit {
	my $manpage = shift;

	if (!exists $manpage_section{$manpage}) {
		warn "section for $manpage unknown\n";
		$manpage_section{$manpage} = 1;
	}
	return "linkgit:${manpage}[$manpage_section{$manpage}]";
}

__END__
-- >8 --

-- 
Jakub Narebski
Poland

[RFC PATCH v3 2/3] Documentation: complete config list from other manpages

[Jakub Narebski]

[-- Attachment #1: Type: text/plain, Size: 8542 bytes --]

From: Thomas Rast <trast@student.ethz.ch>

Add an autogeneration script Documentation/make-config-list.perl
that complete list of config variables with missing variables
from other manpages.

This script generates minimal documentation for those missing config
variables at appropriate place in Documentation/config-vars.txt,
using the following form:

    foo.bar::
    foo.baz::
        See linkgit:git-foo[1].


It does that as follows:

* parse config-vars-src.txt (was config-vars.txt, which is now generated)
  to find out config variables it contains

* parse each manpage source (following includes) for config variable
  headers

* assemble a new config-vars.txt that completes the original list with
  "See linkgit:git-foo[1]" entries for all variables that were not in
  it config-vars-src.txt

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is my take on commit message and on autogeneration script.  I have
attached diff between config-vars-src.txt and generated config-vars.txt.

The differences from v2 by Thomas Rast:
* Removed stray changes to Documentation/Makefile and 
  Documentation/config-vars-src.txt

* The config-vars.txt target now depends on $(cmds_txt), which are 
  included by git.txt (without it, and without 'make doc' ran, the
  generation of "make -C Documentation config-vars.txt" failed because
  it couldn't find IIRC cmds-mainporcelain.txt).  See below.

* The config-vars.txt target now uses $(QUIET_GEN) and $(PERL_PATH),
  and makes use of make's automatic variables ($@, $<), like other
  targets that run *.perl scripts.  It uses $(MAN_TXT) in place of
  its definition, i.e. '$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)'

* The make-config-vars.perl invocation in config-vars.txt target
  now has extra '--ignore=merge-config.txt' because config-vars.src.txt
  contains 'include::merge-config.txt[]'.  See below.

* The make-config-vars.perl got rewritten according to proposals in
  parent email.  In general this means that instead of decomposing
  config-vars-src.txt, adding documentation of missing variables,
  and then recomposing it into config-vars.txt (with side-effects
  such as lowercasing variable names, and sorting variables), it
  simply finds places where to insert missing documentation, and
  generates and inserts it there.

  This means that read_varlist() got simplified, and main code got
  rewritten.

The patch with differences between Thomas patch and mine is attached
as text/plain to this email.


Issues to be solved (aka why this is an RFC):
* Dependencies for config-vars.txt target; probably just needs 
  modification to build-docdep.perl script.

* read_varlist does not follow includes, and that is why we had to
  manually ignore merge-config.txt

* Either sorting or inserting generated documentation could be
  improved; as can be seen in attached config-vars.txt the variables
  from manpage for git-http-backend got split.


 Documentation/Makefile                             |    5 +
 .../{config-vars.txt => config-vars-src.txt}       |    0
 Documentation/make-config-list.perl                |  174 ++++++++++++++++++++
 3 files changed, 179 insertions(+), 0 deletions(-)
 rename Documentation/{config-vars.txt => config-vars-src.txt} (100%)
 create mode 100755 Documentation/make-config-list.perl

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e117bc4..be35bf3 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -320,6 +320,11 @@ howto-index.txt: howto-index.sh $(wildcard howto/*.txt)
 	'$(SHELL_PATH_SQ)' ./howto-index.sh $(wildcard howto/*.txt) >$@+ && \
 	mv $@+ $@
 
+config-vars.txt: config-vars-src.txt $(MAN_TXT) $(cmds_txt)
+	$(QUIET_GEN)$(PERL_PATH) ./make-config-list.perl \
+		--mainlist=$< --ignore=$@ --ignore=merge-config.txt $(MAN_TXT) >$@+ && \
+	mv $@+ $@
+
 $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
 	$(QUIET_ASCIIDOC)$(ASCIIDOC) $(ASCIIDOC_EXTRA) -b xhtml11 $*.txt
 
diff --git a/Documentation/config-vars.txt b/Documentation/config-vars-src.txt
similarity index 100%
rename from Documentation/config-vars.txt
rename to Documentation/config-vars-src.txt
diff --git a/Documentation/make-config-list.perl b/Documentation/make-config-list.perl
new file mode 100755
index 0000000..fe7ac70
--- /dev/null
+++ b/Documentation/make-config-list.perl
@@ -0,0 +1,174 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Getopt::Long;
+use Cwd;
+use Data::Dumper;
+
+
+my %ignore;
+my $rc = GetOptions(
+	"mainlist=s" => \my $mainlistfile,
+	"ignore=s" => sub { $ignore{$_[1]} = 1 },
+);
+
+if (!$rc || !defined $mainlistfile) {
+	print "$0 --mainlist=<mainlist> [--ignore=<ignore>...] <asciidoc_manpage>...\n";
+	exit 1;
+}
+
+
+sub read_varlist {
+	my ($filename) = @_;
+
+	open my $fh, '<', $filename
+		or die "Couldn't open '$filename' for reading: $!";
+
+	my (%mainlist, @mainvars);
+	while (<$fh>) {
+		if (/^(\S+)::/) {
+			my $v = $1;
+			push @mainvars, $v;
+			$mainlist{lc($v)} = $.;
+		}
+	}
+
+	close $fh
+		or die "Couldn't close '$filename': $!";
+
+	return \%mainlist, \@mainvars;
+}
+
+my %var_manpages;
+my %manpage_section;
+
+sub read_man_txt {
+	my ($filename, $manpage) = @_;
+	if (!defined $manpage) {
+		$manpage = $filename;
+		$manpage =~ s/\.txt//;
+	}
+
+	open my $fh, '<', $filename
+		or die "Couldn't open '$filename' for reading: $!";
+	while (<$fh>) {
+		if ($. < 5 && /^$manpage\((\d+)\)/) {
+			$manpage_section{$manpage} = $1;
+		}
+		if (/^([a-z0-9]+\.[a-zA-Z<>0-9.]+)::/) {
+			push @{$var_manpages{$1}}, $manpage;
+		}
+		if (/^include::\s*(\S+)\s*\[\]/ &&
+		    !exists $ignore{$1}) {
+			read_man_txt($1, $manpage);
+		}
+	}
+	close $fh
+		or die "Couldn't close '$filename': $!";
+}
+
+sub find_insertion_points {
+	my ($mainlist, $missing_vars) = @_;
+	my %insert;
+
+	my %all_vars = (%$mainlist, %$missing_vars);
+	my $lineno = -1; # means after last line
+
+	# reverse order because we want to find a place before which to insert
+	# generated documentation; it is easy to find where description
+	# of variable begins, but in general harder to find where it ends.
+	my @sorted_vars = reverse sort { lc($a) cmp lc($b) } keys %all_vars;
+	#print STDERR join("\n", @sorted_vars)."\n";
+	foreach my $key (@sorted_vars) {
+		my $val = $all_vars{$key};
+		if (ref $val) {
+			# this came from %$missing_vars
+			push @{$insert{$lineno}}, $key;
+			print STDERR "$key missing ($lineno)\n";
+		} else {
+			# this came from %$mainlist
+			if ($lineno < 0) {
+				# $lineno < 0 means after end of file (special case)
+				$lineno = $val;
+			} else {
+				# this is in case of unsorted entries in $mainlistfile
+				$lineno = $val < $lineno ? $val : $lineno; # min($val, $lineno)
+			}
+			print STDERR "$key $val $lineno\n";
+		}
+	}
+	return %insert;
+}
+
+sub vars_documentation {
+	my ($keylist, $vars) = @_;
+	my @keys = sort @$keylist;
+	my %out;
+
+	# generate output for each key now, because it is easier to compare
+	# strings than arrays; comparing which is needed for compacting output
+	foreach my $k (@keys) {
+		$out{$k} = "\tSee: ".gen_links($vars->{$k}).".\n";
+	}
+
+	my $output = '';
+	while (my $k = pop @keys) {
+		$output .= $k."::\n";
+		unless (@keys && $out{$k} eq $out{$keys[0]}) {
+			$output .= $out{$k};
+		}
+	}
+	return $output;
+}
+
+sub gen_links {
+	my $manpages = shift;
+	return join(", ", map { linkgit($_) } @$manpages);
+}
+
+sub linkgit {
+	my $manpage = shift;
+
+	if (!exists $manpage_section{$manpage}) {
+		warn "section for $manpage unknown\n";
+		$manpage_section{$manpage} = 1;
+	}
+	return "linkgit:${manpage}[$manpage_section{$manpage}]";
+}
+
+
+foreach my $name (@ARGV) {
+	read_man_txt($name);
+}
+
+my ($mainlist, $mainvars) = read_varlist($mainlistfile);
+
+my @missing_vars =
+	grep { !exists $mainlist->{lc($_)} } keys %var_manpages;
+my %missing_vars =
+	map { $_ => $var_manpages{$_} } @missing_vars;
+
+my %insert = find_insertion_points($mainlist, \%missing_vars);
+$Data::Dumper::Indent = 0;
+$Data::Dumper::Sortkeys = 1;
+$Data::Dumper::Terse = 1;
+#print STDERR Dumper(\%insert);
+
+open my $fh, '<', $mainlistfile
+	or die "Couldn't open '$mainlistfile' for reading: $!";
+while (<$fh>) {
+	if (exists $insert{$.}) {
+		print vars_documentation($insert{$.}, \%missing_vars);
+		print "\n";
+	}
+	print;
+}
+# special case: insertion after last line in $mainlistfile
+print vars_documentation($insert{-1}, \%missing_vars)
+	if exists $insert{-1};
+close $fh
+	or die "Couldn't close '$mainlistfile': $!";
+
+exit 0;
+__END__
-- 
1.7.3


[-- Attachment #2: config-vars.diff.txt --]
[-- Type: text/plain, Size: 3419 bytes --]

diff --git 1/Documentation/config-vars-src.txt 2/Documentation/config-vars.txt
index a8d37a7..e8c7eb3 100644
--- 1/Documentation/config-vars-src.txt
+++ 2/Documentation/config-vars.txt
@@ -680,6 +680,9 @@ commit.template::
 	"{tilde}/" is expanded to the value of `$HOME` and "{tilde}user/" to the
 	specified user's home directory.
 
+cvsexportcommit.cvsdir::
+	See: linkgit:git-cvsexportcommit[1].
+
 diff.autorefreshindex::
 	When using 'git diff' to compare with work tree
 	files, do not consider stat-only change as changed.
@@ -699,6 +702,9 @@ diff.external::
 	you want to use an external diff program only on a subset of
 	your files, you	might want to use linkgit:gitattributes[5] instead.
 
+diff.guitool::
+	See: linkgit:git-difftool[1].
+
 diff.mnemonicprefix::
 	If set, 'git diff' uses a prefix pair that is different from the
 	standard "a/" and "b/" depending on what is being compared.  When
@@ -897,6 +903,10 @@ gitcvs.commitmsgannotation::
 	Append this string to each commit message. Set to empty string
 	to disable this feature. Defaults to "via git-CVS emulator".
 
+gitcvs.dbuser::
+gitcvs.dbpass::
+	See: linkgit:git-cvsserver[1].
+
 gitcvs.enabled::
 	Whether the CVS server interface is enabled for this repository.
 	See linkgit:git-cvsserver[1].
@@ -1084,11 +1094,17 @@ help.autocorrect::
 	value is 0 - the command will be just shown but not executed.
 	This is the default.
 
+http.getanyfile::
+	See: linkgit:git-http-backend[1].
+
 http.proxy::
 	Override the HTTP proxy, normally configured using the 'http_proxy'
 	environment variable (see linkgit:curl[1]).  This can be overridden
 	on a per-remote basis; see remote.<name>.proxy
 
+http.receivepack::
+	See: linkgit:git-http-backend[1].
+
 http.sslVerify::
 	Whether to verify the SSL certificate when fetching or pushing
 	over HTTPS. Can be overridden by the 'GIT_SSL_NO_VERIFY' environment
@@ -1150,6 +1166,9 @@ http.noEPSV::
 	support EPSV mode. Can be overridden by the 'GIT_CURL_FTP_NO_EPSV'
 	environment variable. Default is false (curl will use EPSV).
 
+http.uploadpack::
+	See: linkgit:git-http-backend[1].
+
 http.useragent::
 	The HTTP USER_AGENT string presented to an HTTP server.  The default
 	value represents the version of the client git such as git/1.7.1.
@@ -1174,6 +1193,17 @@ imap::
 	The configuration variables in the 'imap' section are described
 	in linkgit:git-imap-send[1].
 
+imap.user::
+imap.tunnel::
+imap.sslverify::
+imap.preformattedHTML::
+imap.port::
+imap.pass::
+imap.host::
+imap.folder::
+imap.authMethod::
+	See: linkgit:git-imap-send[1].
+
 init.templatedir::
 	Specify the directory from which templates will be copied.
 	(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
@@ -1249,6 +1279,10 @@ man.<tool>.path::
 
 include::merge-config.txt[]
 
+merge.summary::
+merge.log::
+	See: linkgit:git-fmt-merge-msg[1].
+
 mergetool.<tool>.path::
 	Override the path for the given tool.  This is useful in case
 	your tool is not in the PATH.
@@ -1688,6 +1722,13 @@ submodule.<name>.ignore::
 	both settings can be overridden on the command line by using the
 	"--ignore-submodules" option.
 
+svn.useSvnsyncProps::
+svn.useSvmProps::
+svn.pathnameencoding::
+svn.noMetadata::
+svn.brokenSymlinkWorkaround::
+	See: linkgit:git-svn[1].
+
 tar.umask::
 	This variable can be used to restrict the permission bits of
 	tar archive entries.  The default is 0002, which turns off the

[-- Attachment #3: diff-v2.txt --]
[-- Type: text/plain, Size: 8445 bytes --]

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 747b849..8ce75d2 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -125,7 +125,7 @@ endif
 
 SHELL_PATH ?= $(SHELL)
 # Shell quote;
-SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) #'
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 
 #
 # Please note that there is a minor bug in asciidoc.
@@ -320,12 +320,10 @@ howto-index.txt: howto-index.sh $(wildcard howto/*.txt)
 	'$(SHELL_PATH_SQ)' ./howto-index.sh $(wildcard howto/*.txt) >$@+ && \
 	mv $@+ $@
 
-config-vars.txt: config-vars-src.txt $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
-	./make-config-list.perl --mainlist=config-vars-src.txt \
-		--ignore=config-vars.txt \
-		$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT) \
-		> config-vars.txt+ && \
-	mv config-vars.txt+ config-vars.txt
+config-vars.txt: config-vars-src.txt $(MAN_TXT) $(cmds_txt)
+	$(QUIET_GEN)$(PERL_PATH) ./make-config-list.perl \
+		--mainlist=$< --ignore=$@ --ignore=merge-config.txt $(MAN_TXT) >$@+ && \
+	mv $@+ $@
 
 $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
 	$(QUIET_ASCIIDOC)$(ASCIIDOC) $(ASCIIDOC_EXTRA) -b xhtml11 $*.txt
diff --git a/Documentation/config-vars-src.txt b/Documentation/config-vars-src.txt
index 949259c..a8d37a7 100644
--- a/Documentation/config-vars-src.txt
+++ b/Documentation/config-vars-src.txt
@@ -936,7 +936,7 @@ gitcvs.dbname::
 
 gitcvs.dbdriver::
 	Used Perl DBI driver. You can specify any available driver
-	for this here, but it might not work. git-cvsserver is tested
+        for this here, but it might not work. git-cvsserver is tested
 	with 'DBD::SQLite', reported to work with 'DBD::Pg', and
 	reported *not* to work with 'DBD::mysql'. Experimental feature.
 	May not contain double colons (`:`). Default: 'SQLite'.
diff --git a/Documentation/make-config-list.perl b/Documentation/make-config-list.perl
index f086867..2894d96 100755
--- a/Documentation/make-config-list.perl
+++ b/Documentation/make-config-list.perl
@@ -4,128 +4,165 @@ use strict;
 use warnings;
 use Getopt::Long;
 
+
 my %ignore;
-
 my $rc = GetOptions(
 	"mainlist=s" => \my $mainlistfile,
 	"ignore=s" => sub { $ignore{$_[1]} = 1 },
-	"no-sort" => \my $no_sort,
-	);
+);
 
-if (!$rc or (!-r $mainlistfile)) {
+if (!$rc || !defined $mainlistfile) {
 	print "$0 --mainlist=<mainlist> [--ignore=<ignore>...] <asciidoc_manpage>...\n";
 	exit 1;
 }
 
+my %var_manpages;
+my %manpage_section;
+
+foreach my $name (@ARGV) {
+	read_man_txt($name);
+}
+
+my ($mainlist, $mainvars) = read_varlist($mainlistfile);
+
+my @missing_vars =
+	grep { !exists $mainlist->{lc($_)} } keys %var_manpages;
+my %missing_vars =
+	map { $_ => $var_manpages{$_} } @missing_vars;
+
+my %insert = find_insertion_points($mainlist, \%missing_vars);
+
+open my $fh, '<', $mainlistfile
+	or die "Couldn't open '$mainlistfile' for reading: $!";
+while (<$fh>) {
+	if (exists $insert{$.}) {
+		print vars_documentation($insert{$.}, \%missing_vars);
+		print "\n";
+	}
+	print;
+}
+# special case: insertion after last line in $mainlistfile
+print vars_documentation($insert{-1}, \%missing_vars)
+	if exists $insert{-1};
+close $fh
+	or die "Couldn't close '$mainlistfile': $!";
+
+exit 0;
+
+# ----------------------------------------------------------------------
+# ----------------------------------------------------------------------
+# ----------------------------------------------------------------------
+
 sub read_varlist {
-	my ($file) = @_;
+	my ($filename) = @_;
+
+	open my $fh, '<', $filename
+		or die "Couldn't open '$filename' for reading: $!";
 
-	open my $fh, "<", $file or die "cannot open $file: $!";
 	my (%mainlist, @mainvars);
-
-	my ($v, $last_v);
-	my $in_block = 0;
 	while (<$fh>) {
 		if (/^(\S+)::/) {
-			$v = lc $1;
-			$in_block = 0;
-			push @{$mainlist{$v}}, $_;
+			my $v = $1;
 			push @mainvars, $v;
-		} elsif (/^$/ && !$in_block) {
-			if (defined $last_v && !$#{$mainlist{$last_v}}) {
-				$mainlist{$last_v} = $mainlist{$v};
-			}
-			$last_v = $v;
-		} elsif (defined $v) {
-			push @{$mainlist{$v}}, $_;
-			$in_block = !$in_block if /^--$/;
+			$mainlist{lc($v)} = $.;
 		}
 	}
 
-	close $fh or die "eh? close failed: $!";
+	close $fh
+		or die "Couldn't close '$filename': $!";
 
 	return \%mainlist, \@mainvars;
 }
 
-my %vars;
-my %sections;
-
-sub read_file {
-	my ($name, $main_name) = @_;
-	if (!defined $main_name) {
-		$main_name = $name;
+sub read_man_txt {
+	my ($filename, $manpage) = @_;
+	if (!defined $manpage) {
+		$manpage = $filename;
+		$manpage =~ s/\.txt//;
 	}
-	my $manpage_name = $main_name;
-	$manpage_name =~ s/\.txt//;
-	my $fp;
-	open $fp, '<', $name or die "open $name failed: $!";
-	while (<$fp>) {
-		if ($. < 5 && /^$manpage_name\((\d+)\)/) {
-			$sections{$manpage_name} = $1;
+
+	open my $fh, '<', $filename
+		or die "Couldn't open '$filename' for reading: $!";
+	while (<$fh>) {
+		if ($. < 5 && /^$manpage\((\d+)\)/) {
+			$manpage_section{$manpage} = $1;
 		}
-		if (/^([a-z0-9]+\.[a-z<>0-9.]+)::/) {
-			push @{$vars{$1}}, $manpage_name;
+		if (/^([a-z0-9]+\.[a-zA-Z<>0-9.]+)::/) {
+			push @{$var_manpages{$1}}, $manpage;
 		}
-		if (/^include::\s*(\S+)\s*\[\]/
-		    && !exists $ignore{$1}) {
-			read_file($1, $main_name);
+		if (/^include::\s*(\S+)\s*\[\]/ &&
+		    !exists $ignore{$1}) {
+			read_man_txt($1, $manpage);
 		}
 	}
-	close $fp or die "close $name failed: $!";
-}
-
-foreach my $name (@ARGV) {
-	read_file($name);
+	close $fh
+		or die "Couldn't close '$filename': $!";
 }
 
-my ($mainlist, $mainvars) = read_varlist($mainlistfile);
-
-my @all_keys = @$mainvars;
-foreach my $k (keys %vars) {
-	if (!exists $mainlist->{$k}) {
-		push @all_keys, $k;
-	}
-}
+sub find_insertion_points {
+	my ($mainlist, $missing_vars) = @_;
+	my %insert;
 
-@all_keys = sort @all_keys unless $no_sort;
+	my %all_vars = (%$mainlist, %$missing_vars);
+	my $lineno = -1; # means after last line
 
-my %out;
-foreach my $k (@all_keys) {
-	if (exists $mainlist->{$k}) {
-		push @{$out{$k}}, @{$mainlist->{$k}}, "\n";
-	} elsif (exists $vars{$k}) {
-		push @{$out{$k}}, $k . "::\n";
-		push @{$out{$k}}, "\tSee ";
-		my $sep = " ";
-		foreach my $p (sort @{$vars{$k}}) {
-			next if ($p =~ /$mainlistfile/);
-			if (!exists $sections{$p}) {
-				warn "section for $p unknown";
-				$sections{$p} = 1;
+	# reverse order because we want to find a place before which to insert
+	# generated documentation; it is easy to find where description
+	# of variable begins, but in general harder to find where it ends.
+	my @sorted_vars = reverse sort { lc($a) cmp lc($b) } keys %all_vars;
+	foreach my $key (@sorted_vars) {
+		my $val = $all_vars{$key};
+		if (ref $val) {
+			# this came from %$missing_vars
+			push @{$insert{$lineno}}, $key;
+		} else {
+			# this came from %$mainlist
+			if ($lineno < 0) {
+				# $lineno < 0 means after end of file (special case)
+				$lineno = $val;
+			} else {
+				# this is in case of unsorted entries in $mainlistfile
+				$lineno = $val < $lineno ? $val : $lineno; # min($val, $lineno)
 			}
-			push @{$out{$k}}, $sep . "linkgit:" . $p . "[" . $sections{$p} . "]";
-			$sep = ", ";
 		}
-		push @{$out{$k}}, ".\n\n";
-	} else {
-		die "can't happen: $k not in any source";
 	}
+	return %insert;
 }
 
-for (my $i = 0; $i < $#all_keys; $i++) {
-	next if $#{$out{$all_keys[$i]}} != $#{$out{$all_keys[$i+1]}};
-	my $same = 1;
-	for (my $j = 1; $j <= $#{$out{$all_keys[$i]}}; $j++) {
-		if ($out{$all_keys[$i]}[$j] ne $out{$all_keys[$i+1]}[$j]) {
-			$same = 0;
-			last;
-		}
+sub vars_documentation {
+	my ($keylist, $vars) = @_;
+	my @keys = sort @$keylist;
+	my %out;
+
+	# generate output for each key now, because it is easier to compare
+	# strings than arrays; comparing which is needed for compacting output
+	foreach my $k (@keys) {
+		$out{$k} = "\tSee: ".gen_links($vars->{$k}).".\n";
 	}
-	if ($same) {
-		$out{$all_keys[$i]} = [$out{$all_keys[$i]}[0]];
+
+	my $output = '';
+	while (my $k = pop @keys) {
+		$output .= $k."::\n";
+		unless (@keys && $out{$k} eq $out{$keys[0]}) {
+			$output .= $out{$k};
+		}
 	}
+	return $output;
 }
 
-foreach my $k (@all_keys) {
-	print @{$out{$k}};
+sub gen_links {
+	my $manpages = shift;
+	return join(", ", map { linkgit($_) } @$manpages);
 }
+
+sub linkgit {
+	my $manpage = shift;
+
+	if (!exists $manpage_section{$manpage}) {
+		warn "section for $manpage unknown, assuming '1'\n";
+		$manpage_section{$manpage} = 1;
+	}
+	return "linkgit:${manpage}[$manpage_section{$manpage}]";
+}
+
+__END__

[RFC PATCH v3 2/3 (amend)] Documentation: complete config list from other manpages

[Jakub Narebski]

From: Thomas Rast <trast@student.ethz.ch>

Add an autogeneration script Documentation/make-config-list.perl
that complete list of config variables with missing variables
from other manpages.

This script generates minimal documentation for those missing config
variables at appropriate place in Documentation/config-vars.txt,
using the following form:

    foo.bar::
    foo.baz::
        See linkgit:git-foo[1].


It does that as follows:

* parse config-vars-src.txt (was config-vars.txt, which is now generated)
  to find out config variables it contains

* parse each manpage source (following includes) for config variable
  headers

* assemble a new config-vars.txt that completes the original list with
  "See linkgit:git-foo[1]" entries for all variables that were not in
  it config-vars-src.txt

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This version has removed stray commented out remains of debugging the
make-config-list.perl script, and has it slightly reordered for better
readibility.

I have accidentally included old version of a patch.  I am very sorry.

Below there comments from previous email (without references to
attachements, which are not included in this email)
...

The differences from v2 by Thomas Rast:
* Removed stray changes to Documentation/Makefile and 
  Documentation/config-vars-src.txt

* The config-vars.txt target now depends on $(cmds_txt), which are 
  included by git.txt (without it, and without 'make doc' ran, the
  generation of "make -C Documentation config-vars.txt" failed because
  it couldn't find IIRC cmds-mainporcelain.txt).  See below.

* The config-vars.txt target now uses $(QUIET_GEN) and $(PERL_PATH),
  and makes use of make's automatic variables ($@, $<), like other
  targets that run *.perl scripts.  It uses $(MAN_TXT) in place of
  its definition, i.e. '$(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)'

* The make-config-vars.perl invocation in config-vars.txt target
  now has extra '--ignore=merge-config.txt' because config-vars.src.txt
  contains 'include::merge-config.txt[]'.  See below.

* The make-config-vars.perl got rewritten according to proposals in
  parent email.  In general this means that instead of decomposing
  config-vars-src.txt, adding documentation of missing variables,
  and then recomposing it into config-vars.txt (with side-effects
  such as lowercasing variable names, and sorting variables), it
  simply finds places where to insert missing documentation, and
  generates and inserts it there.

  This means that read_varlist() got simplified, and main code got
  rewritten.


Issues to be solved (aka why this is an RFC):
* Dependencies for config-vars.txt target; probably just needs 
  modification to build-docdep.perl script.

* read_varlist does not follow includes, and that is why we had to
  manually ignore merge-config.txt

* Either sorting or inserting generated documentation could be
  improved; as can be seen in attached config-vars.txt the variables
  from manpage for git-http-backend got split.

 Documentation/Makefile                             |    5 +
 .../{config-vars.txt => config-vars-src.txt}       |    0
 Documentation/make-config-list.perl                |  168 ++++++++++++++++++++
 3 files changed, 173 insertions(+), 0 deletions(-)
 rename Documentation/{config-vars.txt => config-vars-src.txt} (100%)
 create mode 100755 Documentation/make-config-list.perl

diff --git a/Documentation/Makefile b/Documentation/Makefile
index e117bc4..8ce75d2 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -320,6 +320,11 @@ howto-index.txt: howto-index.sh $(wildcard howto/*.txt)
 	'$(SHELL_PATH_SQ)' ./howto-index.sh $(wildcard howto/*.txt) >$@+ && \
 	mv $@+ $@
 
+config-vars.txt: config-vars-src.txt $(MAN_TXT) $(cmds_txt)
+	$(QUIET_GEN)$(PERL_PATH) ./make-config-list.perl \
+		--mainlist=$< --ignore=$@ --ignore=merge-config.txt $(MAN_TXT) >$@+ && \
+	mv $@+ $@
+
 $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
 	$(QUIET_ASCIIDOC)$(ASCIIDOC) $(ASCIIDOC_EXTRA) -b xhtml11 $*.txt
 
diff --git a/Documentation/config-vars.txt b/Documentation/config-vars-src.txt
similarity index 100%
rename from Documentation/config-vars.txt
rename to Documentation/config-vars-src.txt
diff --git a/Documentation/make-config-list.perl b/Documentation/make-config-list.perl
new file mode 100755
index 0000000..2894d96
--- /dev/null
+++ b/Documentation/make-config-list.perl
@@ -0,0 +1,168 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Getopt::Long;
+
+
+my %ignore;
+my $rc = GetOptions(
+	"mainlist=s" => \my $mainlistfile,
+	"ignore=s" => sub { $ignore{$_[1]} = 1 },
+);
+
+if (!$rc || !defined $mainlistfile) {
+	print "$0 --mainlist=<mainlist> [--ignore=<ignore>...] <asciidoc_manpage>...\n";
+	exit 1;
+}
+
+my %var_manpages;
+my %manpage_section;
+
+foreach my $name (@ARGV) {
+	read_man_txt($name);
+}
+
+my ($mainlist, $mainvars) = read_varlist($mainlistfile);
+
+my @missing_vars =
+	grep { !exists $mainlist->{lc($_)} } keys %var_manpages;
+my %missing_vars =
+	map { $_ => $var_manpages{$_} } @missing_vars;
+
+my %insert = find_insertion_points($mainlist, \%missing_vars);
+
+open my $fh, '<', $mainlistfile
+	or die "Couldn't open '$mainlistfile' for reading: $!";
+while (<$fh>) {
+	if (exists $insert{$.}) {
+		print vars_documentation($insert{$.}, \%missing_vars);
+		print "\n";
+	}
+	print;
+}
+# special case: insertion after last line in $mainlistfile
+print vars_documentation($insert{-1}, \%missing_vars)
+	if exists $insert{-1};
+close $fh
+	or die "Couldn't close '$mainlistfile': $!";
+
+exit 0;
+
+# ----------------------------------------------------------------------
+# ----------------------------------------------------------------------
+# ----------------------------------------------------------------------
+
+sub read_varlist {
+	my ($filename) = @_;
+
+	open my $fh, '<', $filename
+		or die "Couldn't open '$filename' for reading: $!";
+
+	my (%mainlist, @mainvars);
+	while (<$fh>) {
+		if (/^(\S+)::/) {
+			my $v = $1;
+			push @mainvars, $v;
+			$mainlist{lc($v)} = $.;
+		}
+	}
+
+	close $fh
+		or die "Couldn't close '$filename': $!";
+
+	return \%mainlist, \@mainvars;
+}
+
+sub read_man_txt {
+	my ($filename, $manpage) = @_;
+	if (!defined $manpage) {
+		$manpage = $filename;
+		$manpage =~ s/\.txt//;
+	}
+
+	open my $fh, '<', $filename
+		or die "Couldn't open '$filename' for reading: $!";
+	while (<$fh>) {
+		if ($. < 5 && /^$manpage\((\d+)\)/) {
+			$manpage_section{$manpage} = $1;
+		}
+		if (/^([a-z0-9]+\.[a-zA-Z<>0-9.]+)::/) {
+			push @{$var_manpages{$1}}, $manpage;
+		}
+		if (/^include::\s*(\S+)\s*\[\]/ &&
+		    !exists $ignore{$1}) {
+			read_man_txt($1, $manpage);
+		}
+	}
+	close $fh
+		or die "Couldn't close '$filename': $!";
+}
+
+sub find_insertion_points {
+	my ($mainlist, $missing_vars) = @_;
+	my %insert;
+
+	my %all_vars = (%$mainlist, %$missing_vars);
+	my $lineno = -1; # means after last line
+
+	# reverse order because we want to find a place before which to insert
+	# generated documentation; it is easy to find where description
+	# of variable begins, but in general harder to find where it ends.
+	my @sorted_vars = reverse sort { lc($a) cmp lc($b) } keys %all_vars;
+	foreach my $key (@sorted_vars) {
+		my $val = $all_vars{$key};
+		if (ref $val) {
+			# this came from %$missing_vars
+			push @{$insert{$lineno}}, $key;
+		} else {
+			# this came from %$mainlist
+			if ($lineno < 0) {
+				# $lineno < 0 means after end of file (special case)
+				$lineno = $val;
+			} else {
+				# this is in case of unsorted entries in $mainlistfile
+				$lineno = $val < $lineno ? $val : $lineno; # min($val, $lineno)
+			}
+		}
+	}
+	return %insert;
+}
+
+sub vars_documentation {
+	my ($keylist, $vars) = @_;
+	my @keys = sort @$keylist;
+	my %out;
+
+	# generate output for each key now, because it is easier to compare
+	# strings than arrays; comparing which is needed for compacting output
+	foreach my $k (@keys) {
+		$out{$k} = "\tSee: ".gen_links($vars->{$k}).".\n";
+	}
+
+	my $output = '';
+	while (my $k = pop @keys) {
+		$output .= $k."::\n";
+		unless (@keys && $out{$k} eq $out{$keys[0]}) {
+			$output .= $out{$k};
+		}
+	}
+	return $output;
+}
+
+sub gen_links {
+	my $manpages = shift;
+	return join(", ", map { linkgit($_) } @$manpages);
+}
+
+sub linkgit {
+	my $manpage = shift;
+
+	if (!exists $manpage_section{$manpage}) {
+		warn "section for $manpage unknown, assuming '1'\n";
+		$manpage_section{$manpage} = 1;
+	}
+	return "linkgit:${manpage}[$manpage_section{$manpage}]";
+}
+
+__END__
-- 
1.7.3

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Jakub Narebski]

On Fri, 22 Oct 2010, Jeff King wrote:
> On Fri, Oct 22, 2010 at 07:02:28AM +0200, Thomas Rast wrote:
> 
> > This resurrects (finally) the earlier attempt at
> > 
> >   http://mid.gmane.org/cover.1280169048.git.trast@student.ethz.ch
> > 
> > It tries the inverse approach: teaching the script how to find config
> > variable blocks in each manpage, and then linking them from the main
> > list.  (Obviously just inserting them into the main list could also
> > work.)
> > 
> > In other words, it attempts to push out the "original" documentation
> > of each variable from the main list to the individual manpage, which
> > is exactly opposite from v1.
> 
> Thanks for working on this.
 
[...]

> I have two comments on the approach:
> 
>   1. It looks like you're more or less just parsing "::" list keys from
>      all of the manpages. This seems somewhat fragile, as there could be
>      other non-config lists. Can we at least restrict it to
>      CONFIGURATION sections? It would be nice if we could put in some
>      kind of semantic markup, but I'm not sure how painful that would be
>      with asciidoc (we could always resort to comments in the source,
>      but that would probably get unreadable quickly).

The regexp for catching config variables is quite strict, but tests
done with my version of script modifier further to check for 
CONFIGURATION-like sections (matching /^Config/) shown that we have
problems either way.

1. Without checking for CONFIGURATION sections, we have what are 
   probably false positives from gitmodules.txt:

     submodule.<name>.path::
     submodule.<name>.url::
     submodule.<name>.update::
     submodule.<name>.ignore::

   I say *probably* because while gitmodules manpage describes those 
   variables as going into .gitmodules, if I understand it correctly
   those variables can got to .git/config as an override.

2. With checking for CONFIGURATION-like, we would miss the following
   configuration variables:

     http.getanyfile:: (for git-http-backend, in 'SERVICES')
     http.uploadpack:: (for git-http-backend, in 'SERVICES')
     http.receivepack:: (for git-http-backend, in 'SERVICES')

   These are in git-http-backend manpage, in 'SERVICES' section, which 
   probably should be named then 'CONFIGURING SERVICES'.

   BTW, CONFIGURATION-like means:

    * Configuration
    * CONFIGURATION
   
  but also

    * CONFIG FILE-ONLY OPTIONS
    * CONFIGURATION FILE
    * Configuration Mechanism
    * CONFIG VARIABLES
    * CONFIGURATION VARIABLES
    * Configuring database backend

> 
>   2. You recursively follow includes via include::. This means that the
>      make rule is not accurate. E.g., try:
> 
>        rm cmds-mainporcelain.txt config-vars.txt
>        make config-vars.txt
> 
>      which should fail, as we actually depend on cmds-mainporcelain.txt.
>      Doing it accurately and automatically would mean making .depend
>      files for make.

We do that: see 'doc.dep' target in Documentation/Makefile.  We just
need for this target to also add dependencies for config-vars.txt
(perhaps separate mode for make-config-list.perl, or perhaps 
build-docdep.perl should be config-vars-src.txt aware...).

> 
>      But I wonder if the recursive lookup is really required. Some of
>      the includes with config files can just go away (e.g.,
>      merge-config.txt is included only by config-vars-src.txt and
>      git-merge.txt; it can just be merged straight into git-merge.txt
>      once this system exists).

merge-config.txt is the only included file with config variables.
Other includes does not contain config variables.  And because 
merge-config.txt is also included in config-vars-src.txt, therefore
the whole recursive lookup is not necessary.

Note however that make-config-list.perl only creates minimal documentation,
just link(s) to appropriate mapage(s).  Include-ing merge-config.txt both
in git-merge.txt and config-vars-src.txt means that we have merg config
variables defined in git-config(1) manpage, which I think is nice to have.

>      Others, like pretty-formats.txt, should, 
>      IMHO, get their own user-visible page. Right now with your script
>      you get[1]:
> 
>        format.pretty::
>                The default pretty format for log/show/whatchanged command,
>                See linkgit:git-log[1], linkgit:git-show[1],
>                linkgit:git-whatchanged[1].
> 
>      but I would rather see[2]:
> 
>        format.pretty::
>                See linkgit:gitpretty[7].

Actually the above block describing `format.pretty` is from beginning in
config-vars-src.txt, and is not added / created by said script.

> 
>      [1]: I assume the single line of block description is an error in
>           your script.

Hmmm?

-- 
Jakub Narebski
Poland

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Jeff King]

On Sun, Oct 24, 2010 at 03:24:42AM +0200, Thomas Rast wrote:

> Jeff King wrote:
> >   1. It looks like you're more or less just parsing "::" list keys from
> >      all of the manpages. This seems somewhat fragile, as there could be
> >      other non-config lists. Can we at least restrict it to
> >      CONFIGURATION sections? It would be nice if we could put in some
> >      kind of semantic markup, but I'm not sure how painful that would be
> >      with asciidoc (we could always resort to comments in the source,
> >      but that would probably get unreadable quickly).
> 
> I figured for consistency and ease of lookup *all* configuration docs
> should name the variables in the same format.  It can still be helpful
> to mention them elsewhere, e.g. in the option documentations, but the
> main docs should be a CONFIGURATION section formatted like this.
> 
> Or do you think that would be a bad thing?

No, I think everything should be in a CONFIGURATION section. So checking
for that is probably enough. I was more concerned about false positives
creeping in. Checking just the CONFIGURATION section would probably make
that unlikely, though.

-Peff

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Jeff King]

On Mon, Oct 25, 2010 at 02:44:06PM +0200, Jakub Narebski wrote:

> 2. With checking for CONFIGURATION-like, we would miss the following
>    configuration variables:
> 
>      http.getanyfile:: (for git-http-backend, in 'SERVICES')
>      http.uploadpack:: (for git-http-backend, in 'SERVICES')
>      http.receivepack:: (for git-http-backend, in 'SERVICES')
> 
>    These are in git-http-backend manpage, in 'SERVICES' section, which 
>    probably should be named then 'CONFIGURING SERVICES'.

I would argue those should probably go in a CONFIGURATION section for
consistency with the rest of the manpages.

>    BTW, CONFIGURATION-like means:
> 
>     * Configuration
>     * CONFIGURATION
>    
>   but also
> 
>     * CONFIG FILE-ONLY OPTIONS
>     * CONFIGURATION FILE
>     * Configuration Mechanism
>     * CONFIG VARIABLES
>     * CONFIGURATION VARIABLES
>     * Configuring database backend

Again, I think for consistency for the reader, it may make sense to
switch them all to CONFIGURATION. I'd have to look at each page and see
how appropraite that is, though.

> >   2. You recursively follow includes via include::. This means that the
> >      make rule is not accurate. E.g., try:
> [...]
> We do that: see 'doc.dep' target in Documentation/Makefile.  We just
> need for this target to also add dependencies for config-vars.txt
> (perhaps separate mode for make-config-list.perl, or perhaps 
> build-docdep.perl should be config-vars-src.txt aware...).

Yeah, that would definitely work.

> Note however that make-config-list.perl only creates minimal documentation,
> just link(s) to appropriate mapage(s).  Include-ing merge-config.txt both
> in git-merge.txt and config-vars-src.txt means that we have merg config
> variables defined in git-config(1) manpage, which I think is nice to have.

I disagree. I think one of the benefits of this exercise is generating a
more concise list. That being said, I don't think there's any reason we
can't have a terse list in gitconfig(7) and a much larger one in
gitconfigfull(7) or something like that (or even put it later in the
manpage of gitconfig(7), or whatever).

If you're going to do that, though, there's no point in having
merge-options separate. make-config-list should just generate both
lists.

> >        format.pretty::
> >                The default pretty format for log/show/whatchanged command,
> >                See linkgit:git-log[1], linkgit:git-show[1],
> >                linkgit:git-whatchanged[1].
> [...]
> 
> Actually the above block describing `format.pretty` is from beginning in
> config-vars-src.txt, and is not added / created by said script.

Oh, you're right. I was browsing the output and just assumed it was
created by the script, since it is of a similar form.

> >      [1]: I assume the single line of block description is an error in
> >           your script.
> 
> Hmmm?

The comma at the end made it look to me like a sentence had been cut off
during parsing. But looking at config.txt, it is simply a typo.  The
comma should be a period.

-Peff

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Jakub Narebski]

On Mon, 25 Oct 2010, Jeff King wrote:
> On Mon, Oct 25, 2010 at 02:44:06PM +0200, Jakub Narebski wrote:
> 
> > 2. With checking for CONFIGURATION-like, we would miss the following
> >    configuration variables:
> > 
> >      http.getanyfile:: (for git-http-backend, in 'SERVICES')
> >      http.uploadpack:: (for git-http-backend, in 'SERVICES')
> >      http.receivepack:: (for git-http-backend, in 'SERVICES')
> > 
> >    These are in git-http-backend manpage, in 'SERVICES' section, which 
> >    probably should be named then 'CONFIGURING SERVICES'.
> 
> I would argue those should probably go in a CONFIGURATION section for
> consistency with the rest of the manpages.

What consistency ;-)?  But I agree, if we can switch all the following 
variants to 'CONFIGURATION', it should also go in 'CONFIGURATION'
section.  If we cannot, then 'CONFIGURING SERVICES' as section name,
and /^Config/i as a regexp detecting if we are in relevant section.

> >    BTW, CONFIGURATION-like means:
> > 
> >     * Configuration
> >     * CONFIGURATION
> >    
> >   but also
> > 
> >     * CONFIG FILE-ONLY OPTIONS
> >     * CONFIGURATION FILE
> >     * Configuration Mechanism
> >     * CONFIG VARIABLES
> >     * CONFIGURATION VARIABLES
> >     * Configuring database backend
> 
> Again, I think for consistency for the reader, it may make sense to
> switch them all to CONFIGURATION. I'd have to look at each page and see
> how appropriate that is, though.

Right.

> > >   2. You recursively follow includes via include::. This means that the
> > >      make rule is not accurate. E.g., try:
> > [...]
> > We do that: see 'doc.dep' target in Documentation/Makefile.  We just
> > need for this target to also add dependencies for config-vars.txt
> > (perhaps separate mode for make-config-list.perl, or perhaps 
> > build-docdep.perl should be config-vars-src.txt aware...).
> 
> Yeah, that would definitely work.

Though simpler would be just to not use or turn off following includes,
as it turned out that it doesn't matter to follow includes in manpages:
if we include with config variables, it is to also include it in 
config-vars-src.txt.

Well, assuming that we don't have to follow includes in config-vars-src.txt;
otherwise we have to generate line in doc.dep for that include anyway.

> > Note however that make-config-list.perl only creates minimal documentation,
> > just link(s) to appropriate mapage(s).  Include-ing merge-config.txt both
> > in git-merge.txt and config-vars-src.txt means that we have merg config
> > variables defined in git-config(1) manpage, which I think is nice to have.
> 
> I disagree. I think one of the benefits of this exercise is generating a
> more concise list. 

I can agree with eliminating those blocks that consist only of list of
variables and reference to main page, like the following[1]

  sendemail.<identity>.*::
        Identity-specific versions of the 'sendemail.*' parameters
        found below, taking precedence over those when the this
        identity is selected, through command-line or
        'sendemail.identity'.

  sendemail.aliasesfile::
  sendemail.aliasfiletype::
  sendemail.bcc::
  sendemail.cc::
  [...]
  sendemail.thread::
  sendemail.validate::
        See linkgit:git-send-email[1] for description.


and perhaps also this block

  imap::
        The configuration variables in the 'imap' section are described
        in linkgit:git-imap-send[1].

[1] Though only if we can ensure that they are added below 
'sendemail.<identity>.*', which refers to them.

> That being said, I don't think there's any reason we
> can't have a terse list in gitconfig(7) and a much larger one in
> gitconfigfull(7) or something like that (or even put it later in the
> manpage of gitconfig(7), or whatever).

You can mechanically move or copy description of config variables from
config-vars-src.txt to individual manpages.  Moving in reverse direction,
like in your autogenerated gitconfigfull(7) proposal, is not that easy:
description of config variables in individual manpages can refer to
other parts of said manpage (and barring AI you cannot reliably detect
such situation).

-- 
Jakub Narebski
Poland

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Jakub Narebski]

On Mon, 25 Oct 2010, Jakub Narebski wrote:
> On Mon, 25 Oct 2010, Jeff King wrote:
>> On Mon, Oct 25, 2010 at 02:44:06PM +0200, Jakub Narebski wrote:

>>>>   2. You recursively follow includes via include::. This means that the
>>>>      make rule is not accurate. E.g., try:
>>> [...]
>>> We do that: see 'doc.dep' target in Documentation/Makefile.  We just
>>> need for this target to also add dependencies for config-vars.txt
>>> (perhaps separate mode for make-config-list.perl, or perhaps 
>>> build-docdep.perl should be config-vars-src.txt aware...).
>> 
>> Yeah, that would definitely work.

The part of patch (commit) touching Documentation/Makefile could look
like this:

-- 8< --
diff --git a/Documentation/Makefile b/Documentation/Makefile
index e117bc4..351aa9c 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -212,9 +212,12 @@ install-html: html
 #
 # Determine "include::" file references in asciidoc files.
 #
-doc.dep : $(wildcard *.txt) build-docdep.perl
+doc.dep : $(wildcard *.txt) build-docdep.perl make-config-list.perl
 	$(QUIET_GEN)$(RM) $@+ $@ && \
 	$(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
+	$(PERL_PATH) ./make-config-list.perl --deps=config-vars.txt \
+		--mainlist=config-vars-src.txt $(MAN_TXT) \
+		>>$@+ $(QUIET_STDERR) && \
 	mv $@+ $@
 
 -include doc.dep
@@ -320,6 +323,11 @@ howto-index.txt: howto-index.sh $(wildcard howto/*.txt)
 	'$(SHELL_PATH_SQ)' ./howto-index.sh $(wildcard howto/*.txt) >$@+ && \
 	mv $@+ $@
 
+config-vars.txt: config-vars-src.txt $(MAN_TXT) make-config-list.perl
+	$(PERL_PATH) ./make-config-list.perl \
+		--mainlist=$< --ignore=$@ $(MAN_TXT) >$@+ && \
+	mv $@+ $@
+
 $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt
 	$(QUIET_ASCIIDOC)$(ASCIIDOC) $(ASCIIDOC_EXTRA) -b xhtml11 $*.txt
 
-- >8 --

Note that while in rule for config-vars.txt target we can use automatic
variables, we have to spell filenames in full in the part of doc.dep
rules that generate dependency for config-vars.txt.  Note also that we
have to keep those explicit and implicit filenames in sync across those
two rules.
 
> Though simpler would be just to not use or turn off following includes,
> as it turned out that it doesn't matter to follow includes in manpages:
> if we include with config variables, it is to also include it in 
> config-vars-src.txt.
> 
> Well, assuming that we don't have to follow includes in config-vars-src.txt;
> otherwise we have to generate line in doc.dep for that include anyway.

Decision time!!!

There are two possible approaches:

1. Don't follow includes in manpages (in manpage sources), and don't follow
   includes in config-vars-src.txt

   Advantages:
   * Less code to write and maintain
   * No need to keep filenames in rule for config-vars.txt and for doc.dep
     in sync.

   Disadvantages:
   * Theoretically fragile: it depends on two assumptions:

     A. If file included in manpage contains description of config vars,
        then this file would be included also in config-vars-src.txt

     B. For config variables inside file(s) included by config-vars-src.txt,
        if they are present in individual manpage, it is done by inclusion
        of said file.

     Those assumptions are two sides of the same coin, and can be written
     as: file included in manpagee can contain config variables if and only
     if it is included in config-vars-src.txt

2. Follow includes both in manpages and in config-vars-src.txt, and
   generate dependencies for config-vars.txt

   Advantages:
   * More generic solution

   Disadvantages:
   * Need to keep rules for doc.dep and for config-vars.txt in sync, as
     described above
   * config-vars.txt rule has more dependencies.

Now, which one to choose?
-- 
Jakub Narebski
Poland

Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions

[Jens Lehmann]

Am 25.10.2010 14:44, schrieb Jakub Narebski:
> 1. Without checking for CONFIGURATION sections, we have what are 
>    probably false positives from gitmodules.txt:
> 
>      submodule.<name>.path::
>      submodule.<name>.url::
>      submodule.<name>.update::
>      submodule.<name>.ignore::
> 
>    I say *probably* because while gitmodules manpage describes those 
>    variables as going into .gitmodules, if I understand it correctly
>    those variables can got to .git/config as an override.

AFAICS only 'path' won't show up in .git/config. 'url' and 'update'
are copied into .git/config by "git submodule init", 'ignore' might
be added by a developer to override the setting from .gitmodules.