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@xxxxxxxxxxxxxxx > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html