Re: [PATCH v2 2/2] bugreport: generate config whitelist based on docs
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Aug 17, 2019, 20:38 UTC
- Message-ID
- <20190817203846.31609-1-martin.agren@gmail.com>
- In-Reply-To
- <20190817003926.102944-3-emilyshaffer@google.com>
On Sat, 17 Aug 2019 at 02:42, Emily Shaffer <emilyshaffer@google.com> wrote:
Show 6 quoted lines
> > Add a new step to the build to generate a whitelist of git-config > variables which are appropriate to include in the output of > git-bugreport. New variables can be added to the whitelist by annotating > their documentation in Documentation/config with the line > "// bugreport-include".
These "// bugreport-include" show up in the rendered manpages, both with AsciiDoc and Asciidoctor. :-(
Show 6 quoted lines
> --- a/Documentation/config/sendemail.txt > +++ b/Documentation/config/sendemail.txt > @@ -1,63 +1,63 @@ > -sendemail.identity:: > +sendemail.identity:: // bugreport-exclude > A configuration identity. When given, causes values in the
If I put each comment on a line of its own (after the config option, but I suppose before would work the same way), Asciidoctor truly ignores them and everything's fine. And AsciiDoc renders this one and others like it ok.
Show 6 quoted lines
> -sendemail.aliasesFile:: > -sendemail.aliasFileType:: > -sendemail.annotate:: > +sendemail.aliasesFile:: // bugreport-exclude > +sendemail.aliasFileType:: // bugreport-exclude > +sendemail.annotate:: // bugreport-include
However, AsciiDoc (version 8.6.10) seems to effectively replace those comments with an empty line during processing, and it makes quite the difference here. Instead of these appearing in a compact comma-separated list, they are treated as individual items in the description list with no supporting content.
FWIW, I like the idea of annotating things here to make it harder to forget this whitelisting when adding a new config item.
Below is what I came up with as an alternative approach. Feel free to steal, squash and/or ignore as you see fit.
BTW, I'm not sure the grep invocation is portable (-R ? -h ? -P ? -o ?). We should probably also do the usual "create a candidate output file, then move it into place" dance for robustness.
I do think we should test the generated whitelist in some minimal way, e.g., to check that it does contain something which objectively belongs in the whitelist and -- more importantly IMHO -- does *not* contain something that shouldn't be there, such as sendemail.smtpPass.
Martin
-- >8 -- Subject: [PATCH] Use a bugreport:include/exclude macro instead
Implement a no-op "bugreport" macro for use as "bugreport:include[x]" to annotate the config keys that should be included in the automatically generated whitelist. Use "exclude" for the others.
With Asciidoctor, it's ok to say "bugreport:include[]", but AsciiDoc seems to want something between the brackets. A bit unfortunate, but not a huge problem -- we'll just provide an "x".
"doc-diff" reports that this macro doesn't render at all. That is, these are both empty after this commit:
cd Documentation ./doc-diff --asciidoctor :/"bugreport: add tool" HEAD ./doc-diff --asciidoc :/"bugreport: add tool" HEAD
Diffing the rendered HTML shows that there is some small amount of whitespace and comments added. That shouldn't be a problem.
We could perhaps let the implementation verify that the "action" is one of "include" and "exclude". For the Asciidoctor implementation that should be straightforward, but for AsciiDoc I don't immediately know how to do it. Anyway, if someone stumbles on the keyboard and writes "bugreport:icndule", they'll "only" miss out on the config key being included in the whitelist. If this were a blacklist, the consequences of a misspelled target could be a lot more severe.
Disclaimer: This is the outcome of a combination of copy-paste and trial and error -- better solutions might be possible...
Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- I suppose this could also be "annotate:bugreport[ignore]" to avoid the "[x]" and to place all of this under a more general "annotate" macro.
Documentation/asciidoc.conf | 8 +++ Documentation/asciidoctor-extensions.rb | 8 +++ Documentation/config/sendemail.txt | 68 ++++++++++++------------- bugreport-generate-config-whitelist.sh | 2 +- 4 files changed, 51 insertions(+), 35 deletions(-)
diff --git a/Documentation/asciidoc.conf b/Documentation/asciidoc.conf index 2c16c536ba..5a3c5ef028 100644 --- a/Documentation/asciidoc.conf +++ b/Documentation/asciidoc.conf @@ -6,9 +6,13 @@ # # Show Git link as: <command>(<section>); if section is defined, else just show # the command. +# +# The bugreport macro does nothing as far as rendering is +# concerned -- we just grep for it in the sources. [macros] (?su)[\\]?(?P<name>linkgit):(?P<target>\S*?)\[(?P<attrlist>.*?)\]= +(?su)[\\]?(?P<name>bugreport):(?P<action>\S*?)\[(?P<attrlist>.*?)\]= [attributes] asterisk=* @@ -28,6 +32,8 @@ ifdef::backend-docbook[] {0#<citerefentry>} {0#<refentrytitle>{target}</refentrytitle><manvolnum>{0}</manvolnum>} {0#</citerefentry>} +[bugreport-inlinemacro] +{0#} endif::backend-docbook[] ifdef::backend-docbook[] @@ -94,4 +100,6 @@ ifdef::backend-xhtml11[] git-relative-html-prefix= [linkgit-inlinemacro] <a href="{git-relative-html-prefix}{target}.html">{target}{0?({0})}</a> +[bugreport-inlinemacro] +<!-- --> endif::backend-xhtml11[] diff --git a/Documentation/asciidoctor-extensions.rb b/Documentation/asciidoctor-extensions.rb index 0089e0cfb8..54cbd5ddaf 100644 --- a/Documentation/asciidoctor-extensions.rb +++ b/Documentation/asciidoctor-extensions.rb @@ -20,9 +20,17 @@ module Git end end end + class BugReportProcessor < Asciidoctor::Extensions::InlineMacroProcessor + def process(parent, action, attrs) + "" + end + end end end Asciidoctor::Extensions.register do inline_macro Git::Documentation::LinkGitProcessor, :linkgit + # The bugreport macro does nothing as far as rendering is + # concerned -- we just grep for it in the sources. + inline_macro Git::Documentation::BugReportProcessor, :bugreport end diff --git a/Documentation/config/sendemail.txt b/Documentation/config/sendemail.txt index 69f3e4f219..92f5082013 100644 --- a/Documentation/config/sendemail.txt +++ b/Documentation/config/sendemail.txt @@ -1,63 +1,63 @@ -sendemail.identity:: // bugreport-exclude +sendemail.identity bugreport:exclude[x] :: A configuration identity. When given, causes values in the 'sendemail.<identity>' subsection to take precedence over values in the 'sendemail' section. The default identity is the value of `sendemail.identity`. -sendemail.smtpEncryption:: // bugreport-include +sendemail.smtpEncryption bugreport:include[x] :: See linkgit:git-send-email[1] for description. Note that this setting is not subject to the 'identity' mechanism. -sendemail.smtpssl (deprecated):: // bugreport-exclude +sendemail.smtpssl (deprecated) bugreport:exclude[x] :: Deprecated alias for 'sendemail.smtpEncryption = ssl'. -sendemail.smtpsslcertpath:: // bugreport-exclude +sendemail.smtpsslcertpath bugreport:exclude[x] :: Path to ca-certificates (either a directory or a single file). Set it to an empty string to disable certificate verification. -sendemail.<identity>.*:: // bugreport-exclude +sendemail.<identity>.* bugreport:exclude[x] :: Identity-specific versions of the 'sendemail.*' parameters found below, taking precedence over those when this identity is selected, through either the command-line or `sendemail.identity`. -sendemail.aliasesFile:: // bugreport-exclude -sendemail.aliasFileType:: // bugreport-exclude -sendemail.annotate:: // bugreport-include -sendemail.bcc:: // bugreport-include -sendemail.cc:: // bugreport-include -sendemail.ccCmd:: // bugreport-include -sendemail.chainReplyTo:: // bugreport-include -sendemail.confirm:: // bugreport-include -sendemail.envelopeSender:: // bugreport-include -sendemail.from:: // bugreport-include -sendemail.multiEdit:: // bugreport-include -sendemail.signedoffbycc:: // bugreport-include -sendemail.smtpPass:: // bugreport-exclude -sendemail.suppresscc:: // bugreport-include -sendemail.suppressFrom:: // bugreport-include -sendemail.to:: // bugreport-include -sendemail.tocmd:: // bugreport-include -sendemail.smtpDomain:: // bugreport-include -sendemail.smtpServer:: // bugreport-include -sendemail.smtpServerPort:: // bugreport-include -sendemail.smtpServerOption:: // bugreport-include -sendemail.smtpUser:: // bugreport-exclude -sendemail.thread:: // bugreport-include -sendemail.transferEncoding:: // bugreport-include -sendemail.validate:: // bugreport-include -sendemail.xmailer:: // bugreport-include +sendemail.aliasesFile bugreport:exclude[x] :: +sendemail.aliasFileType bugreport:exclude[x] :: +sendemail.annotate bugreport:include[x] :: +sendemail.bcc bugreport:include[x] :: +sendemail.cc bugreport:include[x] :: +sendemail.ccCmd bugreport:include[x] :: +sendemail.chainReplyTo bugreport:include[x] :: +sendemail.confirm bugreport:include[x] :: +sendemail.envelopeSender bugreport:include[x] :: +sendemail.from bugreport:include[x] :: +sendemail.multiEdit bugreport:include[x] :: +sendemail.signedoffbycc bugreport:include[x] :: +sendemail.smtpPass bugreport:exclude[x] :: +sendemail.suppresscc bugreport:include[x] :: +sendemail.suppressFrom bugreport:include[x] :: +sendemail.to bugreport:include[x] :: +sendemail.tocmd bugreport:include[x] :: +sendemail.smtpDomain bugreport:include[x] :: +sendemail.smtpServer bugreport:include[x] :: +sendemail.smtpServerPort bugreport:include[x] :: +sendemail.smtpServerOption bugreport:include[x] :: +sendemail.smtpUser bugreport:exclude[x] :: +sendemail.thread bugreport:include[x] :: +sendemail.transferEncoding bugreport:include[x] :: +sendemail.validate bugreport:include[x] :: +sendemail.xmailer bugreport:include[x] :: See linkgit:git-send-email[1] for description. -sendemail.signedoffcc (deprecated):: // bugreport-exclude +sendemail.signedoffcc (deprecated) bugreport:exclude[x] :: Deprecated alias for `sendemail.signedoffbycc`. -sendemail.smtpBatchSize:: // bugreport-include +sendemail.smtpBatchSize bugreport:include[x] :: Number of messages to be sent per connection, after that a relogin will happen. If the value is 0 or undefined, send all messages in one connection. See also the `--batch-size` option of linkgit:git-send-email[1]. -sendemail.smtpReloginDelay:: // bugreport-include +sendemail.smtpReloginDelay bugreport:include[x] :: Seconds wait before reconnecting to smtp server. See also the `--relogin-delay` option of linkgit:git-send-email[1]. diff --git a/bugreport-generate-config-whitelist.sh b/bugreport-generate-config-whitelist.sh index ca6b232024..896b838cfa 100755 --- a/bugreport-generate-config-whitelist.sh +++ b/bugreport-generate-config-whitelist.sh @@ -1,4 +1,4 @@ #!/bin/sh -grep -RhPo ".*(?=:: \/\/ bugreport-include)" Documentation/config \ +grep -RhPo ".*(?= +bugreport:include.* ::)" Documentation/config \ >git-bugreport-config-whitelist
-- 2.23.0