On Sat, Aug 17, 2019 at 10:38:46PM +0200, Martin Ågren wrote: > On Sat, 17 Aug 2019 at 02:42, Emily Shaffer <emilyshaffer@xxxxxxxxxx> wrote: > > > > 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. :-( Hmm, I don't see it in the troff or the HTML with asciidoc using `make` - but I do see with asciidoctor, or `asciidoc -d html5`. Nice catch, thanks. > > > --- 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. > > > -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. This is cool - I'll add this to the commit chain and build on top of it. Thanks! > > BTW, I'm not sure the grep invocation is portable (-R ? -h ? -P ? -o ?). Yeah, I'll see what I can do about that (recursive, no filename, Perl-compatible regex, match only) - I can probably replace this with something like `find | xargs pgrep` - I'll keep digging. Thanks. > 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. Good point, I'll add a test for this. Thanks very much for this! - Emily
