diff mbox series

Fit line in 80 columns

Message ID 20220825175653.131125-1-alx.manpages@gmail.com (mailing list archive)
State Rejected
Delegated to: BPF
Headers show
Series Fit line in 80 columns | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch, async

Commit Message

Alejandro Colomar Aug. 25, 2022, 5:56 p.m. UTC
That line is used to generate the bpf-helpers(7) manual page.  It
is a no-fill line, since it represents a command, which means that
the formatter can't break the line, and instead just runs across
the right margin (in most set-ups this means that the pager will
break the line).

Using <fmt> makes it end exactly at the 80-col right margin, both
in the header file, and also in the manual page, and also seems to
be a sensible name.

Cc: bpf <bpf@vger.kernel.org>
Cc: linux-man <linux-man@vger.kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Martin KaFai Lau <kafai@fb.com>
Cc: Song Liu <songliubraving@fb.com>
Cc: Yonghong Song <yhs@fb.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Stanislav Fomichev <sdf@google.com>
Cc: Hao Luo <haoluo@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>
Cc: Quentin Monnet <quentin@isovalent.com>
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
---

Hi,

We are adding another linter to the manual pages, which lints
about output going past the right margin of the terminal.

The following results triggered in the bpf-helpers(7) page:

$ make lint-man-groff
LINT (groff)	tmp/lint/man7/bpf-helpers.7.lint-man.groff.touch
an.tmac:man7/bpf-helpers.7:3: style: .TH missing third argument; suggest document modification date in ISO 8601 format (YYYY-MM-DD)
an.tmac:man7/bpf-helpers.7:3: style: .TH missing fourth argument; suggest package/project name and version (e.g., "groff 1.23.0")
an.tmac:man7/bpf-helpers.7:3: style: .TH missing fifth argument and second argument '7' not a recognized manual section; specify volume title
114:                        telnet-470   [001] .N.. 419421.045894: 0x00000001: <formatted msg>
2642:                                int res = bpf_probe_read_user_str(buf, sizeof(buf),
found style problems; aborting
make: *** [lib/lint-man.mk:50: tmp/lint/man7/bpf-helpers.7.lint-man.groff.touch] Error 1


This patch addresses the issue in line 114 (that line count is in
the output rendered page, not in the man(7) source file).

I'm not sure which of those files are used to produce the manual
page, but for consistency I thought it was best to fix both in the
same way.

Cheers,

Alex


 include/uapi/linux/bpf.h       | 5 ++---
 tools/include/uapi/linux/bpf.h | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

Comments

Alexei Starovoitov Aug. 25, 2022, 6:06 p.m. UTC | #1
On Thu, Aug 25, 2022 at 11:02 AM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> That line is used to generate the bpf-helpers(7) manual page.  It
> is a no-fill line, since it represents a command, which means that
> the formatter can't break the line, and instead just runs across
> the right margin (in most set-ups this means that the pager will
> break the line).
>
> Using <fmt> makes it end exactly at the 80-col right margin, both
> in the header file, and also in the manual page, and also seems to
> be a sensible name.

Nack.

We don't follow 80 char limit and are not going to because of man pages.
Andrii Nakryiko Aug. 25, 2022, 9:36 p.m. UTC | #2
On Thu, Aug 25, 2022 at 11:07 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Aug 25, 2022 at 11:02 AM Alejandro Colomar
> <alx.manpages@gmail.com> wrote:
> >
> > That line is used to generate the bpf-helpers(7) manual page.  It
> > is a no-fill line, since it represents a command, which means that
> > the formatter can't break the line, and instead just runs across
> > the right margin (in most set-ups this means that the pager will
> > break the line).
> >
> > Using <fmt> makes it end exactly at the 80-col right margin, both
> > in the header file, and also in the manual page, and also seems to
> > be a sensible name.
>
> Nack.
>
> We don't follow 80 char limit and are not going to because of man pages.

And it's questionable in general to enforce line length for verbatim
(code) block. It's verbatim for a good reason, it can't be wrapped.
Alejandro Colomar Aug. 25, 2022, 9:51 p.m. UTC | #3
Hi Andrii and Alexei,

On 8/25/22 23:36, Andrii Nakryiko wrote:
> On Thu, Aug 25, 2022 at 11:07 AM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Thu, Aug 25, 2022 at 11:02 AM Alejandro Colomar
>> <alx.manpages@gmail.com> wrote:
>>>
>>> That line is used to generate the bpf-helpers(7) manual page.  It
>>> is a no-fill line, since it represents a command, which means that
>>> the formatter can't break the line, and instead just runs across
>>> the right margin (in most set-ups this means that the pager will
>>> break the line).
>>>
>>> Using <fmt> makes it end exactly at the 80-col right margin, both
>>> in the header file, and also in the manual page, and also seems to
>>> be a sensible name.
>>
>> Nack.
>>
>> We don't follow 80 char limit and are not going to because of man pages.
> 
> And it's questionable in general to enforce line length for verbatim
> (code) block. It's verbatim for a good reason, it can't be wrapped.

That's why instead of wrapping, I reduced the length of some 
"identifier".  It's not enforced, but it's nicer if it fits.  There are 
several other cases, where it wasn't easy to make it shorter, and I left 
it exceeding the margin.

It's not so crucial to fix it, and if you prefer it like it is 
currently, it's reasonable.  This is a suggestion, to make it easier to 
read.

Cheers,

Alex
Alejandro Colomar Aug. 25, 2022, 9:54 p.m. UTC | #4
On 8/25/22 23:51, Alejandro Colomar wrote:
> On 8/25/22 23:36, Andrii Nakryiko wrote:
>> On Thu, Aug 25, 2022 at 11:07 AM Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>>> We don't follow 80 char limit and are not going to because of man pages.
>>
>> And it's questionable in general to enforce line length for verbatim
>> (code) block. It's verbatim for a good reason, it can't be wrapped.
> 
> That's why instead of wrapping, I reduced the length of some 
> "identifier".  It's not enforced, but it's nicer if it fits.  There are 
> several other cases, where it wasn't easy to make it shorter, and I left 
> it exceeding the margin.
> 
> It's not so crucial to fix it, and if you prefer it like it is 
> currently, it's reasonable.  This is a suggestion, to make it easier to 
> read.

As an example, you can see this commit recently applied to the man-pages:

<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=e5da16f10f2d3d55db0bb1ef1156afcb197d8fdc>

I only made lines shorter in cases where no info was lost (including 
format).
G. Branden Robinson Aug. 25, 2022, 10:54 p.m. UTC | #5
[sorry for the big CC]

At 2022-08-25T11:06:55-0700, Alexei Starovoitov wrote:
> Nack.
> 
> We don't follow 80 char limit and are not going to because of man
> pages.

If someone got a contract with O'Reilly or No Starch Press to write a
book on BPF and how revolutionarily awesome it is, it's conceivable they
would be faced with exposing some BPF-related function declarations in
the text.  In cases like the following, what would you have them do?

int bpf_map_update_elem(struct bpf_map *map, const void *key, const void *value, u64 flags)

Be aware that the author may not have infinite flexibility; publishers
generally impose a "house style" which restricts selection of typeface
(so they can't necessarily print at a smaller type size or with the
kerning reduced beyond a certain point to squeeze the text onto the
line).

Regards,
Branden
Alexei Starovoitov Aug. 26, 2022, 12:25 a.m. UTC | #6
On Thu, Aug 25, 2022 at 3:54 PM G. Branden Robinson
<g.branden.robinson@gmail.com> wrote:
>
> [sorry for the big CC]
>
> At 2022-08-25T11:06:55-0700, Alexei Starovoitov wrote:
> > Nack.
> >
> > We don't follow 80 char limit and are not going to because of man
> > pages.
>
> If someone got a contract with O'Reilly or No Starch Press to write a
> book on BPF and how revolutionarily awesome it is, it's conceivable they
> would be faced with exposing some BPF-related function declarations in
> the text.  In cases like the following, what would you have them do?

If they're using a script to write a book their contract
should probably be revoked.
Seth David Schoen Aug. 26, 2022, 12:45 a.m. UTC | #7
G. Branden Robinson writes:

> If someone got a contract with O'Reilly or No Starch Press to write a
> book on BPF and how revolutionarily awesome it is, it's conceivable they
> would be faced with exposing some BPF-related function declarations in
> the text.  In cases like the following, what would you have them do?
> 
> int bpf_map_update_elem(struct bpf_map *map, const void *key, const void *value, u64 flags)

I imagine that the author or editor would break it over two lines like

int bpf_map_update_elem(struct bpf_map *map, const void *key,
                        const void *value, u64 flags)

or maybe the slightly uglier

int bpf_map_update_elem(struct bpf_map *map, const void *key,
    const void *value, u64 flags)

I just looked in _Advanced Programming in the Unix Environment, Third
Edition_ by Stevens and Rago (published by Addison Wesley rather than
either of the fine publishers you mentioned), and it didn't take long
to find a C function prototype that was split across two lines in the
former style, on p. 880

sem_t     *sem_open(const char *name, int oflag, ... /* mode_t mode,
                    unsigned int value */ );

... or on the next page, a better example because of the absence of the
comment:

ssize_t    sendto(int sockfd, const void *buf, size_t nbytes, int flags,
                  const struct sockaddr *destaddr, socklen_t destlen);

It's convenient that C's rules for whitespace semantics make all of
these line-wrapped prototypes have exactly the same meaning as their
un-line-wrapped equivalents, so programmers reading the books shouldn't
have cause to be confused by this.  It could be more challenging in a
language like Python, although there, too, there are syntactically
valid ways to break up some kinds of long lines.
Greg KH Aug. 27, 2022, 7:20 a.m. UTC | #8
On Thu, Aug 25, 2022 at 05:54:25PM -0500, G. Branden Robinson wrote:
> [sorry for the big CC]
> 
> At 2022-08-25T11:06:55-0700, Alexei Starovoitov wrote:
> > Nack.
> > 
> > We don't follow 80 char limit and are not going to because of man
> > pages.
> 
> If someone got a contract with O'Reilly or No Starch Press to write a
> book on BPF and how revolutionarily awesome it is, it's conceivable they
> would be faced with exposing some BPF-related function declarations in
> the text.  In cases like the following, what would you have them do?
> 
> int bpf_map_update_elem(struct bpf_map *map, const void *key, const void *value, u64 flags)
> 
> Be aware that the author may not have infinite flexibility; publishers
> generally impose a "house style" which restricts selection of typeface
> (so they can't necessarily print at a smaller type size or with the
> kerning reduced beyond a certain point to squeeze the text onto the
> line).

As someone who has written a book for one of those publishers you
mention above, this is totally not an issue at all.  Authors and
typesetters know how to properly wrap and handle stuff like this
automatically, it's what they do and has nothing to do with how kernel
header files are layed out.

But even then, if it was an issue, we don't write kernel code for "some
potential commercial entity that can't handle long lines", you know
better than this :)

greg k-h
diff mbox series

Patch

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ef78e0e1a754..c03ae39c03b2 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1619,7 +1619,7 @@  union bpf_attr {
  *
  * 		::
  *
- * 			telnet-470   [001] .N.. 419421.045894: 0x00000001: <formatted msg>
+ * 			telnet-470   [001] .N.. 419421.045894: 0x00000001: <fmt>
  *
  * 		In the above:
  *
@@ -1636,8 +1636,7 @@  union bpf_attr {
  * 			* ``419421.045894`` is a timestamp.
  * 			* ``0x00000001`` is a fake value used by BPF for the
  * 			  instruction pointer register.
- * 			* ``<formatted msg>`` is the message formatted with
- * 			  *fmt*.
+ * 			* ``<fmt>`` is the message formatted with *fmt*.
  *
  * 		The conversion specifiers supported by *fmt* are similar, but
  * 		more limited than for printk(). They are **%d**, **%i**,
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index ef78e0e1a754..c03ae39c03b2 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1619,7 +1619,7 @@  union bpf_attr {
  *
  * 		::
  *
- * 			telnet-470   [001] .N.. 419421.045894: 0x00000001: <formatted msg>
+ * 			telnet-470   [001] .N.. 419421.045894: 0x00000001: <fmt>
  *
  * 		In the above:
  *
@@ -1636,8 +1636,7 @@  union bpf_attr {
  * 			* ``419421.045894`` is a timestamp.
  * 			* ``0x00000001`` is a fake value used by BPF for the
  * 			  instruction pointer register.
- * 			* ``<formatted msg>`` is the message formatted with
- * 			  *fmt*.
+ * 			* ``<fmt>`` is the message formatted with *fmt*.
  *
  * 		The conversion specifiers supported by *fmt* are similar, but
  * 		more limited than for printk(). They are **%d**, **%i**,