Date: Fri, 19 Aug 2005 13:10:23 GMT From: Giorgos Keramidas <keramida@FreeBSD.org> To: freebsd-doc@FreeBSD.org Subject: Re: docs/85097: [patch] devd.conf.5 lacks a lot of vital information. Message-ID: <200508191310.j7JDANAm099431@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR docs/85097; it has been noted by GNATS.
From: Giorgos Keramidas <keramida@FreeBSD.org>
To: Fredrik Lindberg <fli+freebsd@shapeshifter.se>
Cc: bug-followup@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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200508191310.j7JDANAm099431>