* [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions @ 2010-10-22 5:02 Thomas Rast 2010-10-22 5:02 ` [RFC PATCH v2 3/3] Documentation: move format.* documentation to format-patch Thomas Rast ` (4 more replies) 0 siblings, 5 replies; 19+ messages in thread From: Thomas Rast @ 2010-10-22 5:02 UTC (permalink / raw) To: git Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Jakub Narebski, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH v2 3/3] Documentation: move format.* documentation to format-patch 2010-10-22 5:02 [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions Thomas Rast @ 2010-10-22 5:02 ` Thomas Rast [not found] ` <c3f621cd062b2c4f80aa2e8dadcfddbc042aefaa.1287690696.git.trast@student.ethz.ch> ` (3 subsequent siblings) 4 siblings, 0 replies; 19+ messages in thread From: Thomas Rast @ 2010-10-22 5:02 UTC (permalink / raw) To: git Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Jakub Narebski, Sverre Rabbelier 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
[parent not found: <c3f621cd062b2c4f80aa2e8dadcfddbc042aefaa.1287690696.git.trast@student.ethz.ch>]
* Re: [RFC PATCH v2 1/3] Documentation: Move variables from config.txt to separate file [not found] ` <c3f621cd062b2c4f80aa2e8dadcfddbc042aefaa.1287690696.git.trast@student.ethz.ch> @ 2010-10-22 8:18 ` Jakub Narebski 0 siblings, 0 replies; 19+ messages in thread From: Jakub Narebski @ 2010-10-22 8:18 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-22 5:02 [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions Thomas Rast 2010-10-22 5:02 ` [RFC PATCH v2 3/3] Documentation: move format.* documentation to format-patch Thomas Rast [not found] ` <c3f621cd062b2c4f80aa2e8dadcfddbc042aefaa.1287690696.git.trast@student.ethz.ch> @ 2010-10-22 15:17 ` Jakub Narebski 2010-10-22 15:53 ` Jeff King [not found] ` <8145782bddf60325909f328337cb76d25c4402cf.1287690696.git.trast@student.ethz.ch> 4 siblings, 0 replies; 19+ messages in thread From: Jakub Narebski @ 2010-10-22 15:17 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-22 5:02 [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions Thomas Rast ` (2 preceding siblings ...) 2010-10-22 15:17 ` [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions Jakub Narebski @ 2010-10-22 15:53 ` Jeff King 2010-10-24 1:24 ` Thomas Rast 2010-10-25 12:44 ` Jakub Narebski [not found] ` <8145782bddf60325909f328337cb76d25c4402cf.1287690696.git.trast@student.ethz.ch> 4 siblings, 2 replies; 19+ messages in thread From: Jeff King @ 2010-10-22 15:53 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Jakub Narebski, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-22 15:53 ` Jeff King @ 2010-10-24 1:24 ` Thomas Rast 2010-10-25 15:04 ` Jeff King 2010-10-25 12:44 ` Jakub Narebski 1 sibling, 1 reply; 19+ messages in thread From: Thomas Rast @ 2010-10-24 1:24 UTC (permalink / raw) To: Jeff King Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Jakub Narebski, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-24 1:24 ` Thomas Rast @ 2010-10-25 15:04 ` Jeff King 0 siblings, 0 replies; 19+ messages in thread From: Jeff King @ 2010-10-25 15:04 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Jakub Narebski, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-22 15:53 ` Jeff King 2010-10-24 1:24 ` Thomas Rast @ 2010-10-25 12:44 ` Jakub Narebski 2010-10-25 15:11 ` Jeff King 2010-11-02 13:42 ` Jens Lehmann 1 sibling, 2 replies; 19+ messages in thread From: Jakub Narebski @ 2010-10-25 12:44 UTC (permalink / raw) To: Jeff King Cc: Thomas Rast, git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-25 12:44 ` Jakub Narebski @ 2010-10-25 15:11 ` Jeff King 2010-10-25 15:49 ` Jakub Narebski 2010-11-02 13:42 ` Jens Lehmann 1 sibling, 1 reply; 19+ messages in thread From: Jeff King @ 2010-10-25 15:11 UTC (permalink / raw) To: Jakub Narebski Cc: Thomas Rast, git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-25 15:11 ` Jeff King @ 2010-10-25 15:49 ` Jakub Narebski 2010-10-27 10:56 ` Jakub Narebski 0 siblings, 1 reply; 19+ messages in thread From: Jakub Narebski @ 2010-10-25 15:49 UTC (permalink / raw) To: Jeff King Cc: Thomas Rast, git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-25 15:49 ` Jakub Narebski @ 2010-10-27 10:56 ` Jakub Narebski 0 siblings, 0 replies; 19+ messages in thread From: Jakub Narebski @ 2010-10-27 10:56 UTC (permalink / raw) To: Jeff King Cc: Thomas Rast, git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions 2010-10-25 12:44 ` Jakub Narebski 2010-10-25 15:11 ` Jeff King @ 2010-11-02 13:42 ` Jens Lehmann 1 sibling, 0 replies; 19+ messages in thread From: Jens Lehmann @ 2010-11-02 13:42 UTC (permalink / raw) To: Jakub Narebski Cc: Jeff King, Thomas Rast, git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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. ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <8145782bddf60325909f328337cb76d25c4402cf.1287690696.git.trast@student.ethz.ch>]
* Re: [RFC PATCH v2 2/3] Documentation: complete config list from other manpages [not found] ` <8145782bddf60325909f328337cb76d25c4402cf.1287690696.git.trast@student.ethz.ch> @ 2010-10-22 6:19 ` Jakub Narebski 2010-10-22 14:31 ` Jakub Narebski 2010-10-23 22:24 ` [PATCH] " Thomas Rast 2 siblings, 0 replies; 19+ messages in thread From: Jakub Narebski @ 2010-10-22 6:19 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH v2 2/3] Documentation: complete config list from other manpages [not found] ` <8145782bddf60325909f328337cb76d25c4402cf.1287690696.git.trast@student.ethz.ch> 2010-10-22 6:19 ` [RFC PATCH v2 2/3] Documentation: complete config list from other manpages Jakub Narebski @ 2010-10-22 14:31 ` Jakub Narebski 2010-10-23 22:24 ` [PATCH] " Thomas Rast 2 siblings, 0 replies; 19+ messages in thread From: Jakub Narebski @ 2010-10-22 14:31 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] Documentation: complete config list from other manpages [not found] ` <8145782bddf60325909f328337cb76d25c4402cf.1287690696.git.trast@student.ethz.ch> 2010-10-22 6:19 ` [RFC PATCH v2 2/3] Documentation: complete config list from other manpages Jakub Narebski 2010-10-22 14:31 ` Jakub Narebski @ 2010-10-23 22:24 ` Thomas Rast 2010-10-23 22:30 ` Jakub Narebski 2010-10-24 14:36 ` Jakub Narebski 2 siblings, 2 replies; 19+ messages in thread From: Thomas Rast @ 2010-10-23 22:24 UTC (permalink / raw) To: git Cc: Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Jakub Narebski, Sverre Rabbelier 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] Documentation: complete config list from other manpages 2010-10-23 22:24 ` [PATCH] " Thomas Rast @ 2010-10-23 22:30 ` Jakub Narebski 2010-10-24 14:36 ` Jakub Narebski 1 sibling, 0 replies; 19+ messages in thread From: Jakub Narebski @ 2010-10-23 22:30 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Documentation: complete config list from other manpages 2010-10-23 22:24 ` [PATCH] " Thomas Rast 2010-10-23 22:30 ` Jakub Narebski @ 2010-10-24 14:36 ` Jakub Narebski 2010-10-24 20:44 ` [RFC PATCH v3 2/3] " Jakub Narebski 1 sibling, 1 reply; 19+ messages in thread From: Jakub Narebski @ 2010-10-24 14:36 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH v3 2/3] Documentation: complete config list from other manpages 2010-10-24 14:36 ` Jakub Narebski @ 2010-10-24 20:44 ` Jakub Narebski 2010-10-24 20:55 ` [RFC PATCH v3 2/3 (amend)] " Jakub Narebski 0 siblings, 1 reply; 19+ messages in thread From: Jakub Narebski @ 2010-10-24 20:44 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier [-- 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__ ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH v3 2/3 (amend)] Documentation: complete config list from other manpages 2010-10-24 20:44 ` [RFC PATCH v3 2/3] " Jakub Narebski @ 2010-10-24 20:55 ` Jakub Narebski 0 siblings, 0 replies; 19+ messages in thread From: Jakub Narebski @ 2010-10-24 20:55 UTC (permalink / raw) To: Thomas Rast Cc: git, Ævar Arnfjörð Bjarmason, Jonathan Nieder, Junio C Hamano, Sverre Rabbelier 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 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-11-02 13:44 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-22 5:02 [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions Thomas Rast 2010-10-22 5:02 ` [RFC PATCH v2 3/3] Documentation: move format.* documentation to format-patch Thomas Rast [not found] ` <c3f621cd062b2c4f80aa2e8dadcfddbc042aefaa.1287690696.git.trast@student.ethz.ch> 2010-10-22 8:18 ` [RFC PATCH v2 1/3] Documentation: Move variables from config.txt to separate file Jakub Narebski 2010-10-22 15:17 ` [RFC PATCH v2 0/3] Documentation: refactor config variable descriptions Jakub Narebski 2010-10-22 15:53 ` Jeff King 2010-10-24 1:24 ` Thomas Rast 2010-10-25 15:04 ` Jeff King 2010-10-25 12:44 ` Jakub Narebski 2010-10-25 15:11 ` Jeff King 2010-10-25 15:49 ` Jakub Narebski 2010-10-27 10:56 ` Jakub Narebski 2010-11-02 13:42 ` Jens Lehmann [not found] ` <8145782bddf60325909f328337cb76d25c4402cf.1287690696.git.trast@student.ethz.ch> 2010-10-22 6:19 ` [RFC PATCH v2 2/3] Documentation: complete config list from other manpages Jakub Narebski 2010-10-22 14:31 ` Jakub Narebski 2010-10-23 22:24 ` [PATCH] " Thomas Rast 2010-10-23 22:30 ` Jakub Narebski 2010-10-24 14:36 ` Jakub Narebski 2010-10-24 20:44 ` [RFC PATCH v3 2/3] " Jakub Narebski 2010-10-24 20:55 ` [RFC PATCH v3 2/3 (amend)] " Jakub Narebski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).