docs/85097: [patch] devd.conf.5 lacks a lot of vital information.

[Giorgos Keramidas]

The following reply was made to PR docs/85097; it has been noted by GNATS.

From: Giorgos Keramidas <keramida at FreeBSD.org>
To: Fredrik Lindberg <fli+freebsd at shapeshifter.se>
Cc: bug-followup at FreeBSD.org
Subject: Re: docs/85097: [patch] devd.conf.5 lacks a lot of vital information.
Date: Fri, 19 Aug 2005 16:01:08 +0300

 On 2005-08-18 21:13, Fredrik Lindberg <fli+freebsd at shapeshifter.se> wrote:
 > The manual page for devd.conf(5) is very sparse.  This patch adds
 > information on how you actually configure devd and explains the syntax
 > and meaning of the avaiable substatements.
 
 Good idea.  The patch has some minor details that we may have to fix
 before it's committed:
 
     * EOL spaces must be removed from the "new" lines
     * Some syntax and grammar mistakes have to be fixed (see below)
 
 % +If two statements matches the same event, only the action of the statement with
 % +highest priority will be carried out. In this way generic statements can be
 % +overridden for devices/notifications that requires special attention. 
 
     "If two statements match the same event, only the action of the
     statement with the highest priority will be carried out.  This way
     generic statements can be overriden for events that require special
     attention."
 
 % +.Pp
 % +The general syntax to create a statement is as follows
 
 Missing ':' at the end of the line.
 
 % +.Ss Substatements
 % +The following statements are supported within the 
 % +.Ql options
 % +statement.
 % +.Bl -tag -width ".Ic directory"
 % +.It Ic directory \*q/some/path\*q; 
 % +Adds the given directory to the list of directories from which devd will read
 % +configuration files. Any number of this directive is valid.
 
 We should replace inline "devd" text with an .Xr reference to the
 devd(8) manpage:
 
     from which
     .Xr devd 8
     will read
 
 % +.It Ic pid-file \*q/var/run/devd.pid\*q;
 % +Specifies pid file.
 
 This is too terse.  What pid file?  Why pid file?  Whose pidfile?  It
 may be obvious by looking at the filename of the sample pidfile, but
 it's ok to expand this description a little, I think.
 
 % +.It Ic set regexp-name \*q(some|regexp)\*q;
 % +Creates a regular expression and assigns it to the variable
 % +regexp-name, this variable is then avaiable through out the rest of 
 % +the configuration file.
 % +All regular expressions have an implicit ^$ around them.
 
 The "regexp-name" string should be tagged with .Va (variable).  The two
 sentences that are joined now by a comma are not really a single
 sentence, but two complete, distinct sentences.  "Throughout" is only
 one word.  The above should probably be rewritten as:
 
     .It Ic set regexp-name \*q(some|regexp)\*q;
     Creates a regular expression and assigns it to the variable
     .Va regexp-name .
     This variable is then available through out the rest of the
     .Pa devd.conf
     file.
     All regular expressions have an implicit
     .Dq Li ^$
     around them.
 
 % +.El
 % +.Pp
 % +The following statements are supported within the 
 % +.Ql attach
 % +and
 % +.Ql detach
 % +statements.
 
 The final fullstop should really be a ':' character here.
 
 % +.Bl -tag -width ".Ic directory"
 % +.It Ic device-name \*qstring\*q;
 % +Actually a shorthand to `match device-name'. Matches a device named string.
 
 Literal quote characters shouldn't be present in mdoc manuals.  The word
 'actually' sounds a bit superficial.
 
     .It Ic device-name \*qstring\*q;
     A shorthand for
     .Dq Li "match device-name" .
    This matches a device named
    .Dq Li string .
 
 It may even be worth the effort to tag "string" with an .Ar macro.
 
     A shorthand for
     .Dq Li "match device-name" .
     This matches a device named
     .Ar string .
 
 % +string is allowed to be a regular expression or a variable previously created
 % +containing a regular expression.
 
     The
     .Ar string
     argument is allowed to be a regular expression...
 
 % +The variable $device-name is avaiable for later use with the action-statement.
 
 This has a typo (avaiable) and lacks some markup:
 
     The
     .Va "$device-name"
     variable can latter be used within the argument of the
     .Dq Li action
     statement and it will be replaced by the actual name of the device
     that matched the
     .Dq Li device-name
     argument.
 
 % +.It Ic match \*qvariable\*q \*qvalue\*q;
 % +Matches the content of value against variable. value can be a regular expression.
 % +Not really needed during attach/detach events since the device-name statement
 % +takes care of all device matching.
 % +For a partial list of variables, see below.
 
 Some formatting is missing here too.
 
     Matches the content of
     .Ar value
     against
     .Ar variable .
     The
     .Ar value
     argument may be a regular expression (see
     .Xr re_format 7 ).
     .Ic match
     statements are not very useful for setting up handlers for device
     attach or detach events, since this can be handled by
     .Ic device-name
     statements.
     See below for a partial list of available variable names.
 
 We might also want to replace "below" with a section or subsection name.
 
 % +.It Ic action \*qcommand\*q;
 % +Command to execute upon a successful match. 
 % +Example /etc/pccard_ether $device-name start
 
 A lot of formatting is missing here too:
 
     Execute
     .Ar command
     upon a successful match.
     An example of such a command is:
     .Bd -literal -offset indent
     action "/etc/pccard_ether $device-name start";
     .Ed
 
 % +.El
 % +.Pp
 % +The following statements are supported within the 
 % +.Ql nomatch
 % +statement. 
 
 EOL spaces should be removed and the '.' character right before a list
 should be replaced with ':'.
 
 % +.Bl -tag -width ".Ic directory"
 % +.It Ic match \*qvariable\*q \*qvalue\*q;
 % +Matches the content of value against variable. value can be a regular expression.
 % +For a partial list of variables, see below.
 
     Match the content of
     .Ar value
     against
     .Ar variable .
     The
     .Ar value
     argument can be a regular expression (see
     .Xr re_format 7 ).
     See below for a partial list of available variable names.
 
 % +.It Ic action \*qcommand\*q;
 % +Same as above.
 
 Same as what 'above'?  This is a duplicate of an existing description of
 the "action" element, so it may be removed IMHO.
 
 % +.Pp
 % +The following statements are supported within the 
 % +.Ql notify 
 % +statement.
 
 Colon instead of a fullstop before the beginning of a list, please.
 
 It's also a good idea to use .Dq Li for multicharacter arguments,
 instead of .Ql:
 
     The
     .Dq Li notify
     statement supports the following substatements:
 
 % +The variable
 % +.Ql $notify
 % +is avaiable inside this statement and contains, possibly, a value depending
 % +on which system and subsystem that delivered the event. 
 
 This is misplaced, it has a typo, it uses .Ql with a multi-character
 word (there's a tendency to use .Ql for single character arguments and
 ``.Dq Li foo'' for arguments that have more than 1 character), and does
 not protect `$' (which may be a good idea)..  When the reader sees:
 
 	The supported options of ``foo'' are:
 
 then it's obvious that a list of the supported options follows.  Seeing
 a second sentence is a bit unexpected and introduces an unwanted pause
 between the announcement of a list and the list itself.  How about?
 
     Depending on the system and subsystem that delivers an event, the
     .Va notify
 
 % +.Bl -tag -width ".Ic directory"
 % +.It Ic match \*qsystem|subsystem|type\*q \*qvalue\*q;
 % +Any number of match-statements can exists within a notify-statement.
 % +value can be either a fixed string or a regular expression. 
 % +Below is a list of avaiable systems, subsystems and types.
 % +.It Ic action \*qcommand\*q;
 % +Command to execute upon a successful match. For example 
 % +/etc/rc.d/power_profile $notify
 % +.El
 
 
 % +.Ss Variables that can be used with the match-statement
 
 Too long title.  How about?
 
     .Ss Predefined match-statement variable names
 
 % +Partial list of variables and their possible values that can be used together
 % +with the
 % +.Ql match
 % +statement.
 
 "together with the match statement" seems a bit strange.  It's also
 pretty useless to duplicate the title in slightly different words, just
 to make sure that a short decription precedes the list.  If we add a
 description at all, we might as well go all the way and write something
 like this:
 
     The arguments of a
     .Dq Li match
     statement support expansion of variable names starting with a dollar
     sign
     .Pq Ql \$ .
     The predefined variable names are:
     .Bl -tag -width "manufacturer"
     ...
     .El
 
 % +.Pp
 % +.Bl -tag -width "manufacturer" -compact
 
 There's no need for a .Pp before the .Bl request here.
 
 % +.It Ic Variable
 % +.Ic Possible value
 
 .Ic is an "interactive command".  I'm sure this is not the right tag for
 either the list item or the item description here.
 
 % +.It bus
 % +pccard[0-9]+, cardbus[0-9]+
 
 It would be nice if regexps are tagged as "literal text" or quoted, or
 both.
 
 % +.Ss Notify matching 
 % +Partial list of systems, subsystems and types used within the
 % +.Ql notify
 % +mechanism. 
 
 As above, .Ql is not good for quoting multicharacter strings.  It's
 better to use ``.Dq Li foo'' in this case.
 
 % +.It ACAD
 % +AC Line state ($notify=0 is offline, 1 is online)
 
 Since ``$notify'' is a variable, it should be tagged as one.  The
 capitalization of "Line" seems strange too.
 
     .It ACAD
     AC line state.  Offline when
     .Dq Li \$notify
     is 0, online when
     .Dq Li \$notify
     is 1.
 
 % +.It Button
 % +Button state ($notify=0 is power, 1 is sleep)
 % +.It CMBAT
 % +Battery events.
 % +.It Lid
 % +Lid state ($notify=0 is closed, 1 is open)
 % +.It Thermal
 % +Thermal zone events.
 % +.El
 
 See above for ``.Dq Li \$notify'' changes.
 
 % +.El 
 % +.El
 % +.El
 
 Triply nested lists are usually a sign that this may be split in at
 least two sections.  This may not be the case here, but I didn't get a
 chance to patch the manpage locally and see now the output looks like in
 nroff with 80-column terminals.  I suspect the lines of the innermost
 list are far too narrow to be readable.
 
 % +.Sh EXAMPLES 
 % +The file
 % +.Pa /etc/devd.conf
 % +contains numerous of different examples.
 
 It's probably better to write:
 
 	The "foo" file contains...
 
 I'm not sure about this one, though.  It's just a vague feeling that
 placing the "file" after the filename reads more natural.
 
 You have made significant and very useful additions to the original
 manpage, but there's still some mdoc work that needs to be done before
 we can commit this.  Other, more experienced mdoc people from
 freebsd-doc may have to look at the comments I made too, in case I
 missed or misunderstood something.
 
 - Giorgos