From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-1.sys.kth.se (smtp-1.sys.kth.se [130.237.32.175]) by krisdoz.my.domain (8.14.5/8.14.5) with ESMTP id pBP0v5vj023415 for ; Sat, 24 Dec 2011 19:57:05 -0500 (EST) Received: from mailscan-1.sys.kth.se (mailscan-1.sys.kth.se [130.237.32.91]) by smtp-1.sys.kth.se (Postfix) with ESMTP id 3207B156373 for ; Sun, 25 Dec 2011 01:56:59 +0100 (CET) X-Virus-Scanned: by amavisd-new at kth.se Received: from smtp-1.sys.kth.se ([130.237.32.175]) by mailscan-1.sys.kth.se (mailscan-1.sys.kth.se [130.237.32.91]) (amavisd-new, port 10024) with LMTP id JVY7QirH7+G2 for ; Sun, 25 Dec 2011 01:56:57 +0100 (CET) X-KTH-Auth: kristaps [81.198.4.39] X-KTH-mail-from: kristaps@bsd.lv X-KTH-rcpt-to: tech@mdocml.bsd.lv Received: from macky.local (unknown [81.198.4.39]) by smtp-1.sys.kth.se (Postfix) with ESMTP id 12A57154137 for ; Sun, 25 Dec 2011 01:56:56 +0100 (CET) Message-ID: <4EF674D8.7030508@bsd.lv> Date: Sun, 25 Dec 2011 02:56:56 +0200 From: Kristaps Dzonsons User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20111105 Thunderbird/8.0 X-Mailinglist: mdocml-tech Reply-To: tech@mdocml.bsd.lv MIME-Version: 1.0 To: tech@mdocml.bsd.lv Subject: Re: implement mandocdb -t References: <20111224164653.GD15148@iris.usta.de> <4EF65E2F.5070009@bsd.lv> <20111225003816.GI15148@iris.usta.de> In-Reply-To: <20111225003816.GI15148@iris.usta.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 25/12/2011 02:38, Ingo Schwarze wrote: > Hi Kristaps, > > thanks for the quick review! > > Kristaps Dzonsons wrote on Sun, Dec 25, 2011 at 01:20:15AM +0200: > >> I have a few issues with this patch -- not the intent or code, but >> rather with the consistency of the verbosity and warnings. >> >> First, for consistency, all warnings/errors should follow "CAUSE: >> REASON" -- including those in the getopt() switch. Thus, >> "conflicting options" messages should be prefixed by the offending >> switch case and "Too many arguments" should begin with "-C". > > Makes sense to me; fixed. > >> Second, verbosity. I think we should settle this now. BSD.lv only >> has a single `-v' and I think this should stay as such ("%s: added >> to index" or "%s: removed from index" (or whatever the message is)). >> This is completely clear and is also valuable to the user. Noting >> keywords added or removed is unnecessary when you consider that >> mandocdb(8) isn't there to debug the keywords of individual manuals: >> it's there to index them. > > Indeed, i didn't put dumping of keywords back. > > I'm ok with having only a single -v, both in the documentation and > in the long term. For now, i'd like to keep the "scanning" and > "scheduling" messages around, they help tremendously for > debugging, but can be removed when the dust has settled. > >> Regarding errors, the existences of (and not reasons for) mandoc(3) >> parse errors are printed to stderr ("parse failed"). For catpages, >> I think it's unnecessary to print anything more than the same. >> After all, it's not like the operator can do anything about the >> page, no? Just give the same error regardless the reason. Maybe an >> extra kick like "%s: parse failed (not a manpage?)". > > >> Moving on to the warnings (wrong sections, architectures, etc.). >> This shouldn't be a `-v', as this isn't a verbose message, it's a >> warning. Maybe we can pull in `-W' from mandoc(1)? I don't think >> these should be printed by default, as they don't affect parse >> status. > > Definitely, because printing them by default would clobber > the output of pkg_add(1), and espie@ would not be amused. > >> An extra "warning" flag is both consistent with other >> utilities and doesn't confuse the utility of `-v'. > > Interesting idea. > Right now, makewhatis(8) doesn't have a -W flag. > > The only use of -W i can find is in Linux man(1): > -W [Linux man] list the pathnames of matching formatted pages > which is similar to man -w, just for formatted manuals. > I don't think we have to respect that. > >> Another option is to print the warnings by default, but use `-q' to >> silence them. > > No way: pkg_add(1)! > >> But that's also confusing because one `-q' silences >> warnings, but `-qq' silences parse errors also? Ugh... > > Ugh indeed. > >> If we follow this logic, then `-t' is much clearer, as it does >> nothing but run in `add' mode without changing the database. (Isn't >> this usually termed `-n', as with make(1)? Sigh.) > > Well, both OpenBSD makewhatis(8) and Linux mandb(8) call it -t, > so i'd like to stick with that. > >> However, the `-W' flag (or whatever it becomes) could be turned on >> by default just like `-Tlint' turns on `-Wall' in mandoc(1). > > Indeed, -t without -W doesn't make much sense. > Besides, -t has to imply -W because otherwise, > we would break established behaviour of makewhatis -t. > >> `-v' would be unaffected and meaningless as no files are actually >> added or removed. >> >> Thoughts? > > Lots of useful input, see below for a patch to be applied > on top of the previous one, for easier review. > > OK in that revised form? Ingo, Thanks for making these changes so quickly! I like what I see, so please commit. You don't, by the way, need to initialise the static variables to zero. > Well, mandocdb -t is not for the operator, but for the porter, > and for the porter, the message should be as specific as possible, > because a diligent porter may wish to fix the manuals and send > patches upstream. I agree with this logic. When you've checked everything in, by the way, I've a local patch that groks everything between NAME and the next section instead of just the first line. This catches full descriptions and all names. I'll convert that to a function reading into a string buffer, then fit it into your checks. Thanks again, Kristaps -- To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv