Re: implement mandocdb -t

[Kristaps Dzonsons <kristaps@bsd.lv>]

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