Documentation: refactor config variable descriptions
v2 cover Documentation: refactor config variable descriptions
This resurrects (finally) the earlier attempt at
http://mid.gmane.org/[email protected]
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
Signature · 1 line
1.7.3.1.281.g5da0b
RFC PATCH v2 3/3 Documentation: move format.* documentation to format-patch
--- Documentation/config-vars-src.txt | 56 ------------------------------- Documentation/git-format-patch.txt | 65 +++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 60 deletions(-)
Documentation/config-vars-src.txt +0 -56
@@ -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
Documentation/git-format-patch.txt +61 -4
79 lines not on this page · raw message
Signature · 1 line
1.7.3.1.281.g5da0b
A message that is not in the archive (sent off-list or not yet received): 8145782bddf60325909f328337cb76d25c4402cf.1287690696.git.trast@student.ethz.ch
Re: [RFC PATCH v2 2/3] Documentation: complete config list from other manpages
Quoted 14 lines · 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 <[email protected]>
Quoted 9 lines
--- 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.
Signature · 2 lines
Jakub Narebski Poland
Re: [RFC PATCH v2 2/3] Documentation: complete config list from other manpages
In reply to a message not in the archiveQuoted 6 lines · 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
Quoted 6 lines
* 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.
Quoted 10 lines
Signed-off-by Thomas Rast <[email protected]>--- 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
[...]
Documentation/make-config-list.perl line 1
Quoted 7 lines
Documentation/make-config-list.perl new file +1
-0@@ -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:
Documentation/make-config-list.perl line 2
Quoted 1 line
Documentation/make-config-list.perl new file +1
-0+ open my $fh, "<", $file or die "cannot open $file: $!";
vs
Documentation/make-config-list.perl
Quoted 4 lines
Documentation/make-config-list.perl new file +3
-0+ my $fp; + open $fp, '<', $name or die "open $name failed: $!"; + close $fh or die "eh? close failed: $!";
vs
Quoted 10 lines
Documentation/make-config-list.perl new file +8
-0+ close $fp or die "close $name failed: $!"; + +use strict; +use warnings; +use Getopt::Long; + +my %ignore; +
Documentation/make-config-list.perl
Quoted 10 lines
Documentation/make-config-list.perl new file +10
-0+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.
Quoted 1 line
+
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 ]
Quoted 4 lines
Documentation/make-config-list.perl new file +4
-0+sub read_varlist { + my ($file) = @_; + + open my $fh, "<", $file or die "cannot open $file: $!";
Documentation/make-config-list.perl
Quoted 10 lines
Documentation/make-config-list.perl new file +10
-0+ 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?
Documentation/make-config-list.perl
Quoted 5 lines
Documentation/make-config-list.perl new file +5
-0+ } 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?
Documentation/make-config-list.perl
Quoted 3 lines
Documentation/make-config-list.perl new file +3
-0+ } elsif (defined $v) { + push @{$mainlist{$v}}, $_; + $in_block = !$in_block if /^--$/;
O.K., easy to understand.
Documentation/make-config-list.perl
Quoted 8 lines
Documentation/make-config-list.perl new file +8
-0+ } + } + + 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).
Documentation/make-config-list.perl
Quoted 3 lines
Documentation/make-config-list.perl new file +3
-0+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.
Documentation/make-config-list.perl
Quoted 2 lines
Documentation/make-config-list.perl new file +2
-0+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?
Documentation/make-config-list.perl
Quoted 11 lines
Documentation/make-config-list.perl new file +11
-0+ 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?
Quoted 12 lines
Documentation/make-config-list.perl new file +12
-0+ 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) {
Documentation/make-config-list.perl
Quoted 10 lines
Documentation/make-config-list.perl new file +10
-0+ 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
Documentation/make-config-list.perl
Quoted 8 lines
Documentation/make-config-list.perl new file +8
-0+} + +@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.
Documentation/make-config-list.perl
Quoted 3 lines
Documentation/make-config-list.perl new file +3
-0+ } 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.
Documentation/make-config-list.perl
Quoted 1 line
Documentation/make-config-list.perl new file +1
-0+ my $sep = " ";
Not $sep = "", or "\tSee"? Otherwise you would get "\tSee linkgit:git-foo[1]" with double space.
Documentation/make-config-list.perl
Quoted 8 lines
Documentation/make-config-list.perl new file +7
-0+ 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?
Documentation/make-config-list.perl
Quoted 8 lines
Documentation/make-config-list.perl new file +8
-0+ $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.
Documentation/make-config-list.perl
Quoted 10 lines
Documentation/make-config-list.perl new file +9
-0+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).
Documentation/make-config-list.perl
Quoted 4 lines
Documentation/make-config-list.perl new file +4
-0+ 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.
Quoted 8 lines
Documentation/make-config-list.perl new file +4
-0+ +foreach my $k (@all_keys) { + print @{$out{$k}}; +}Signature · 1 line
1.7.3.1.281.g5da0b
Signature · 2 lines
Jakub Narebski Poland
PATCH 1/1 Documentation: complete config list from other manpages
In reply to a message not in the archiveAdd 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.
---
Quoted 3 lines · Jakub Narebski
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.perlDocumentation/Makefile +8 -1
@@ -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
Documentation/config-vars-src.txt renamed from Documentation/config-vars.txt +1 -1
@@ -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'.
Documentation/make-config-list.perl new file +131 -0
131 lines not on this page · raw message
Signature · 1 line
1.7.3.2.239.gbefc4
Re: [PATCH] Documentation: complete config list from other manpages
Quoted 10 lines · Thomas Rast
---
Quoted 3 lines · Jakub Narebski
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 [email protected]
Signature · 2 lines
Jakub Narebski Poland
Re: [PATCH] Documentation: complete config list from other manpages
In reply to Thomas RastThis is second review, with the same comments ad the one before, and containing proposal for simpler autogeneration script.
Quoted 2 lines · Thomas Rast
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.
Quoted 11 lines
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.
Quoted 4 lines
Signed-off-by Thomas Rast <[email protected]>---
Quoted 8 lines
Quoted 3 lines · Jakub Narebski
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).
Quoted 12 lines
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.perlDocumentation/Makefile +0
-0
Documentation/Makefile endif · line 131
Quoted 8 lines
Documentation/Makefile +1
-1@@ -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...
Documentation/Makefile howto-index.txt: howto-index.sh $(wildcard howto/*.txt) · line 327
Quoted 9 lines
Documentation/Makefile +5
-0@@ -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+ && \
Quoted 14 lines
Documentation/Makefile +3
-0+ > config-vars.txt+ && \ + mv config-vars.txt+ config-vars.txt + $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt $(QUIET_ASCIIDOC)$(ASCIIDOC) $(ASCIIDOC_EXTRA) -b xhtml11 $*.txtDocumentation/config-vars-src.txt renamed from Documentation/config-vars.txt +0
-0
Documentation/config-vars-src.txt gitcvs.dbname:: · line 942
Quoted 8 lines
Documentation/config-vars-src.txt renamed from Documentation/config-vars.txt +1
-1@@ -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.
Quoted 9 lines
Documentation/make-config-list.perl new file +3
-0@@ -0,0 +1,131 @@ +#!/usr/bin/perl + +use strict;
Documentation/make-config-list.perl line 13
Quoted 10 lines
Documentation/make-config-list.perl new file +10
-0+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?
Documentation/make-config-list.perl line 18
Quoted 5 lines
Documentation/make-config-list.perl new file +5
-0+ +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.
Documentation/make-config-list.perl line 19
Quoted 1 line
Documentation/make-config-list.perl new file +1
-0+
This subroutine needs description, I think; at least oneline comment describing it.
Documentation/make-config-list.perl line 23
Quoted 4 lines
Documentation/make-config-list.perl new file +4
-0+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.
Documentation/make-config-list.perl line 27
Quoted 4 lines
Documentation/make-config-list.perl new file +4
-0+ 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
|
vQuoted 1 line
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?
Documentation/make-config-list.perl line 30
Quoted 3 lines
Documentation/make-config-list.perl new file +3
-0+ 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.
Documentation/make-config-list.perl line 33
Quoted 3 lines
Documentation/make-config-list.perl new file +3
-0+ $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.
Documentation/make-config-list.perl line 38
Quoted 5 lines
Documentation/make-config-list.perl new file +5
-0+ } 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[]'?
Documentation/make-config-list.perl line 42
Quoted 4 lines
Documentation/make-config-list.perl new file +4
-0+ } 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).
Documentation/make-config-list.perl line 45
Quoted 3 lines
Documentation/make-config-list.perl new file +3
-0+ } + + 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: $!";
Documentation/make-config-list.perl line 51
Quoted 6 lines
Documentation/make-config-list.perl new file +6
-0+ + 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...
Quoted 1 line
+
This subroutine needs description, I think; at least oneline comment describing it. And probably better name, like read_manpage().
Documentation/make-config-list.perl line 59
Quoted 7 lines
Documentation/make-config-list.perl new file +7
-0+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.
Quoted 10 lines
Documentation/make-config-list.perl new file +10
-0+ 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*\[\]/
Documentation/make-config-list.perl line 79
Quoted 10 lines
Documentation/make-config-list.perl new file +10
-0+ && !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.
Documentation/make-config-list.perl line 88
Quoted 9 lines
Documentation/make-config-list.perl new file +9
-0+ +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.
Quoted 2 lines
+
+@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?
Quoted 31 lines
Documentation/make-config-list.perl new file +31
-0+ +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;
Documentation/make-config-list.perl line 131
Quoted 10 lines
Documentation/make-config-list.perl new file +10
-0+ } + } + 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 --
Signature · 2 lines
Jakub Narebski Poland
RFC PATCH v3 2/3 Documentation: complete config list from other manpages
From: Thomas Rast <[email protected]>
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
---
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.perlDocumentation/Makefile +5 -0
@@ -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
Documentation/config-vars-src.txt renamed from Documentation/config-vars.txt +0 -0
Documentation/make-config-list.perl new file +174 -0
174 lines not on this page · raw message
Signature · 1 line
1.7.3
Documentation/config-vars.txt renamed from Documentation/config-vars-src.txt +41 -0
94 lines not on this page · raw message
Documentation/Makefile +5 -7
@@ -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
Documentation/config-vars-src.txt +1 -1
@@ -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'.
Documentation/make-config-list.perl +121 -84
249 lines not on this page · raw message
RFC PATCH v3 2/3 Documentation: complete config list from other manpages
From: Thomas Rast <[email protected]>
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
---
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.perlDocumentation/Makefile +5 -0
@@ -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
Documentation/config-vars-src.txt renamed from Documentation/config-vars.txt +0 -0
Documentation/make-config-list.perl new file +168 -0
168 lines not on this page · raw message
Signature · 1 line
1.7.3
A message that is not in the archive (sent off-list or not yet received): c3f621cd062b2c4f80aa2e8dadcfddbc042aefaa.1287690696.git.trast@student.ethz.ch
Re: [RFC PATCH v2 1/3] Documentation: Move variables from config.txt to separate file
Quoted 6 lines · Thomas Rast
Signed-off-by Thomas Rast <[email protected]>--- 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.
Signature · 2 lines
Jakub Narebski Poland
Quoted 4 lines · Thomas Rast
This resurrects (finally) the earlier attempt at
Quoted 8 lines
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]
Quoted 4 lines
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.gitt/doc-config-extraction-v2
Yeah, I saw neither 1/3 nor 2/3 on the list.
-Peff
Quoted 7 lines · Jeff King
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...]
Quoted 7 lines
[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).
Signature · 2 lines
Thomas Rast
trast@{inf,student}.ethz.chQuoted 9 lines · Thomas Rast
Quoted 7 lines · Jeff King
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).
Quoted 6 lines
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
Quoted 16 lines · Jeff King
Quoted 4 lines · Thomas Rast
This resurrects (finally) the earlier attempt at
Quoted 8 lines
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.
[...]
Quoted 9 lines
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
Quoted 10 lines
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.txtwhich 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...).
Quoted 6 lines
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.
Quoted 9 lines
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].
Quoted 4 lines
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.
Quoted 3 lines
[1]: I assume the single line of block description is an error in
your script.
Hmmm?
Signature · 2 lines
Jakub Narebski Poland
Quoted 9 lines · Jakub Narebski
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.
Quoted 5 lines
BTW, CONFIGURATION-like means:
* Configuration
* CONFIGURATION
Quoted 8 lines
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.
Quoted 7 lines
Quoted 2 lines
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 inDocumentation/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.
Quoted 4 lines
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.
Quoted 8 lines
Quoted 4 lines
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.prettyis 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.
Quoted 4 lines
Quoted 2 lines
[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
Quoted 9 lines · Jeff King
Quoted 7 lines · Jakub Narebski
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')
Quoted 5 lines
Quoted 2 lines
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.
Quoted 14 lines
Quoted 5 lines
BTW, CONFIGURATION-like means:
* Configuration
* CONFIGURATIONQuoted 8 lines
but also
* CONFIG FILE-ONLY OPTIONS
* CONFIGURATION FILE
* Configuration Mechanism
* CONFIG VARIABLES
* CONFIGURATION VARIABLES
* Configuring database backend
Quoted 3 lines
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.
Quoted 9 lines
Quoted 7 lines
Quoted 2 lines
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 inDocumentation/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.
Quoted 7 lines
Quoted 4 lines
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.
Quoted 4 lines
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).
Signature · 2 lines
Jakub Narebski Poland
Quoted 5 lines · Jakub Narebski
Quoted 4 lines · Jeff King
Quoted 2 lines · Jakub Narebski
Quoted 2 lines
2. You recursively follow includes via include::. This means that the make rule is not accurate. E.g., try:
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
[...]
We do that: see 'doc.dep' target inDocumentation/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< --
Documentation/Makefile +9 -1
@@ -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.
Quoted 7 lines
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?
Signature · 2 lines
Jakub Narebski Poland
Am 25.10.2010 14:44, schrieb Jakub Narebski:
Quoted 11 lines
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.
Quoted 4 lines · Thomas Rast
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.
Signature · 2 lines
Jakub Narebski Poland