Re: [RFC v1] perf_event_open.2: srcfix + ffix

Michael Kerrisk (man-pages)
2020-11-13T09:21:49Z

Hi ALex,

On 11/12/20 11:55 PM, Alejandro Colomar wrote:
> Changes:
>
> - Use .RS/.RE combined with .PP, instead of .IP.
> This adds markup lines,
> but allows for more generic constructs that don't depend
> on the context.
> In case of .TP, add .RS right after the paragraph tag.
> Exception: Single-paragraph .TP entries don't use .RS/.RE
> - Separate .B or .I entries on separate lines for each.
> - Fix .BR or .IR where 'R' is not needed.
> - Use the following markup for code between paragraphs:
> .PP ((unless .IP is _really_ needed))
> .in +4n
> .EX
> //code
> .EE
> .in
>
> Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
> ---
>
> Hi Michael,
>
> This is a sample of how I would change the code for indenting.
> I also added some srcfixes that probably don't deserve a separate patch,
> such as splitting lines or removing unnecessary "" or [.IR] -> [.I].

(It would have been simpler for the discussion if those were
omitted from the current patch.)

>
> There are 29 net line additions due to splitting lines
> and indenting code with markup instead of hardcoding.
>
> So it means the remaining 44 net additions are due to [.RS/.RE].
> Not too many considering the ~2800 lines involved.
>
> Still between 1% and 2% of net additions.
>
> I've compared side to side the man page from release 5.09,
> and the page after this patch,
> and the changes are minor
> (just a forced new line for .TP entries with multiple paragraphs,
> due to [.RS], as expected).
>
> What do you think about these changes?

I think I'm *still* not really convinced. See below.

> man2/perf_event_open.2 | 463 ++++++++++++++++++++++++-----------------
> 1 file changed, 268 insertions(+), 195 deletions(-)
>
> diff --git a/man2/perf_event_open.2 b/man2/perf_event_open.2
> index eb7ab29ea..b99c9e30d 100644
> --- a/man2/perf_event_open.2
> +++ b/man2/perf_event_open.2

[...]

> @@ -1800,12 +1854,17 @@ by new.
> In overwrite mode, it might not be possible to infer where the
> new data began, and it is the consumer's job to disable
> measurement while reading to avoid possible data races.
> -.IP
> +.PP
> The
> -.IR aux_head " and " aux_tail
> +.I aux_head
> +and
> +.I aux_tail
> ring buffer pointers have the same behavior and ordering
> rules as the previous described
> -.IR data_head " and " data_tail .
> +.I data_head
> +and
> +.IR data_tail .
> +.RE
> .PP
> The following 2^n ring-buffer pages have the layout described below.
> .PP
> @@ -1845,15 +1904,15 @@ the fields with shorter descriptions are presented first.
> This indicates the size of the record.
> .TP
> .I misc
> +.RS
> The
> .I misc
> field contains additional information about the sample.

This renders as:

size This indicates the size of the record.

misc
The misc field contains additional information about the
sample.

The CPU mode can be determined from this value by masking
with PERF_RECORD_MISC_CPUMODE_MASK and looking for one of
the following (note these are not bit masks, only one can
be set at a time):

This is anomalous. We have a newline after "misc", but not after "size",
because of the proposal that we add ".RS/.RE" only in .TP blocks
with multiple paragraphs.

Whats the alternative? I guess we could *always* use .RS/.RE even inside
.TP blocks that have only one paragraph, but:

* That's even more markup
* We end up with even more white space in the rendered output.
* We almost certainly *don't* want to do this for .TP lists
that consist of multiple items where each list item is a
rendered paragraph that is juust a line or two. [1]

[...]

At this point, I feel we are devoting a lot of energy to solving a
problem that has no really good solution. The status quo is not ideal,
but so far I'm not convinced that there's any compellingly better
alternative. And moving to one of those alternatives will require
changes in a lot of pages. I'm in favor of staying with the status quo.

Thanks,

Michael

[1]
I mean, I think this:

[[
HEAD item text

HEAD item text

HEAD item text
]]

is preferable to this:

[[
HEAD
item text

HEAD
item text

HEAD
item text

HEAD
item text

]]

--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/