| 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 |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch, async |
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.
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.
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
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).
[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
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.
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.
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 --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**,
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(-)