| Message ID | 20210902220854.198850-2-corbet@lwn.net |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | ext4: docs: de-uglify Documentation/ext4/orphan.rst | expand |
Hi Jon, On Thu, 2 Sep 2021 16:08:53 -0600, Jonathan Corbet wrote: > Commit 3a6541e97c03 (Add documentation about the orphan file feature) added > a new document on orphan files, which is great. But the use of > "list-table" results in documents that are absolutely unreadable in their > plain-text form. Switch this file to the regular RST table format instead; > the rendered (HTML) output is identical. In the "list tables" section of doc-guide/sphinx.rst, the first paragraph starts with the sentence: We recommend the use of list table formats. Yes, the disadvantage of list tables is mentioned later in the paragraph: Compared to the ASCII-art they might not be as comfortable for readers of the text files. , but I still see list-table is meant as the preferred format. If you prefer the ASCII-art form for simple tables, you might need to de-emphasize the above mentioned recommendation as well. Thanks, Akira
Akira Yokosawa <akiyks@gmail.com> writes: [Adding Mauro] > On Thu, 2 Sep 2021 16:08:53 -0600, Jonathan Corbet wrote: > >> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added >> a new document on orphan files, which is great. But the use of >> "list-table" results in documents that are absolutely unreadable in their >> plain-text form. Switch this file to the regular RST table format instead; >> the rendered (HTML) output is identical. > > In the "list tables" section of doc-guide/sphinx.rst, the first paragraph > starts with the sentence: > > We recommend the use of list table formats. > > Yes, the disadvantage of list tables is mentioned later in the paragraph: > > Compared to the ASCII-art they might not be as comfortable for readers > of the text files. > > , but I still see list-table is meant as the preferred format. Interesting...that is not at all my memory of the discussions we had at that time. There was a lot of pushback against anything that makes the RST files less readable - still is, if certain people join the conversation. Tables were one of the early flash points. Mauro, you added that text; do you remember things differently? Do you feel we should retain that recommendation? Thanks, jon
On Fri, 03 Sep 2021 09:11:26 -0600, Jonathan Corbet wrote: > Akira Yokosawa <akiyks@gmail.com> writes: > > [Adding Mauro] > >> On Thu, 2 Sep 2021 16:08:53 -0600, Jonathan Corbet wrote: >> >>> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added >>> a new document on orphan files, which is great. But the use of >>> "list-table" results in documents that are absolutely unreadable in their >>> plain-text form. Switch this file to the regular RST table format instead; >>> the rendered (HTML) output is identical. >> >> In the "list tables" section of doc-guide/sphinx.rst, the first paragraph >> starts with the sentence: >> >> We recommend the use of list table formats. >> >> Yes, the disadvantage of list tables is mentioned later in the paragraph: >> >> Compared to the ASCII-art they might not be as comfortable for readers >> of the text files. >> >> , but I still see list-table is meant as the preferred format. > > Interesting...that is not at all my memory of the discussions we had at > that time. There was a lot of pushback against anything that makes the > RST files less readable - still is, if certain people join the > conversation. Tables were one of the early flash points. > > Mauro, you added that text; do you remember things differently? Do you > feel we should retain that recommendation? No, the text was first added by Markus Heiser [added to CC] in commit 0249a7644857 ("doc-rst: flat-table directive - initial implementation") and have not updated ever since. He might remember the circumstances, but 2016 was a long time ago, I guess. Or did the discussions take place after the list table support had been added? Thanks, Akira (a newcomer to kerneldoc) > > Thanks, > > jon >
Am 04.09.21 um 03:23 schrieb Akira Yokosawa: > On Fri, 03 Sep 2021 09:11:26 -0600, Jonathan Corbet wrote: >> Akira Yokosawa <akiyks@gmail.com> writes: >> >> [Adding Mauro] >> >>> On Thu, 2 Sep 2021 16:08:53 -0600, Jonathan Corbet wrote: >>> >>>> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added >>>> a new document on orphan files, which is great. But the use of >>>> "list-table" results in documents that are absolutely unreadable in their >>>> plain-text form. Switch this file to the regular RST table format instead; >>>> the rendered (HTML) output is identical. >>> >>> In the "list tables" section of doc-guide/sphinx.rst, the first paragraph >>> starts with the sentence: >>> >>> We recommend the use of list table formats. >>> >>> Yes, the disadvantage of list tables is mentioned later in the paragraph: >>> >>> Compared to the ASCII-art they might not be as comfortable for readers >>> of the text files. >>> >>> , but I still see list-table is meant as the preferred format. >> >> Interesting...that is not at all my memory of the discussions we had at >> that time. There was a lot of pushback against anything that makes the >> RST files less readable - still is, if certain people join the >> conversation. Tables were one of the early flash points. >> >> Mauro, you added that text; do you remember things differently? Do you >> feel we should retain that recommendation? > > No, the text was first added by Markus Heiser [added to CC] in commit > 0249a7644857 ("doc-rst: flat-table directive - initial implementation") > and have not updated ever since. > > He might remember the circumstances, but 2016 was a long time ago, > I guess. We prefer list tables ... """Their advantage is that they are easy to create or modify and that the diff of a modification is much more meaningful, because it is limited to the modified content.""" By example: We have some very large tables with tons of rows and cols. If you need to extend one column just by one character you have to edit the whole table and the diff is not readable. It is not limited to big tables, e.g. if you patch a simple typo, you might need touch content not related to your fix. At the end it is a trade of, what weights more, readability of the plain text or readability of the patch / most often I would vote for the latter. -- Markus -- > > Or did the discussions take place after the list table support had been > added? > > Thanks, Akira (a newcomer to kerneldoc) > >> >> Thanks, >> >> jon >>
Markus Heiser <markus.heiser@darmarit.de> writes: > We prefer list tables ... > > """Their advantage is that they are easy to create or modify and that the > diff of a modification is much more meaningful, because it is limited to > the modified content.""" > > By example: We have some very large tables with tons of rows and cols. > If you need to extend one column just by one character you have to edit > the whole table and the diff is not readable. > > It is not limited to big tables, e.g. if you patch a simple typo, > you might need touch content not related to your fix. > > At the end it is a trade of, what weights more, readability of the > plain text or readability of the patch / most often I would vote > for the latter. If the documentation is of any use of all there will be a lot more people reading it than will be reading patches making tweaks to it. Optimizing for patch readability seems like the wrong focus to me. The ext4 folks can decide what they like best in this specific case. But I think that the advice in favor of list tables is wrong in the general case; they are completely unreadable in their source form, and that goes against one of the key reasons we adopted RST in the first place. Somebody will surely try to add a list table to the wrong document someday and I'll get to live through another one of those nifty explosions - and I'll have neither reasons nor motivation to defend that policy. Thanks, jon
Am 06.09.21 um 16:17 schrieb Jonathan Corbet: > Markus Heiser <markus.heiser@darmarit.de> writes: > >> We prefer list tables ... >> >> """Their advantage is that they are easy to create or modify and that the >> diff of a modification is much more meaningful, because it is limited to >> the modified content.""" >> >> By example: We have some very large tables with tons of rows and cols. >> If you need to extend one column just by one character you have to edit >> the whole table and the diff is not readable. >> >> It is not limited to big tables, e.g. if you patch a simple typo, >> you might need touch content not related to your fix. >> >> At the end it is a trade of, what weights more, readability of the >> plain text or readability of the patch / most often I would vote >> for the latter. > > If the documentation is of any use of all there will be a lot more > people reading it than will be reading patches making tweaks to it. > Optimizing for patch readability seems like the wrong focus to me. > > The ext4 folks can decide what they like best in this specific case. > But I think that the advice in favor of list tables is wrong in the > general case; they are completely unreadable in their source form, and > that goes against one of the key reasons we adopted RST in the first > place. > > Somebody will surely try to add a list table to the wrong document > someday and I'll get to live through another one of those nifty > explosions - and I'll have neither reasons nor motivation to defend that > policy. I do not see a problem changing the policy to use pre-formated tables. @jon do you like to fix the "list tables" section of doc-guide/sphinx.rst Thanks, Markus
On Thu 02-09-21 16:08:53, Jonathan Corbet wrote: > Commit 3a6541e97c03 (Add documentation about the orphan file feature) added > a new document on orphan files, which is great. But the use of > "list-table" results in documents that are absolutely unreadable in their > plain-text form. Switch this file to the regular RST table format instead; > the rendered (HTML) output is identical. > > Signed-off-by: Jonathan Corbet <corbet@lwn.net> Thanks! Definitely looks more readable :). You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > Documentation/filesystems/ext4/orphan.rst | 32 ++++++++--------------- > 1 file changed, 11 insertions(+), 21 deletions(-) > > diff --git a/Documentation/filesystems/ext4/orphan.rst b/Documentation/filesystems/ext4/orphan.rst > index bb19ecd1b626..d096fe0ba19e 100644 > --- a/Documentation/filesystems/ext4/orphan.rst > +++ b/Documentation/filesystems/ext4/orphan.rst > @@ -21,27 +21,17 @@ in heavy creation of orphan inodes. When orphan file feature > (referenced from the superblock through s\_orphan_file_inum) with several > blocks. Each of these blocks has a structure: > > -.. list-table:: > - :widths: 8 8 24 40 > - :header-rows: 1 > - > - * - Offset > - - Type > - - Name > - - Description > - * - 0x0 > - - Array of \_\_le32 entries > - - Orphan inode entries > - - Each \_\_le32 entry is either empty (0) or it contains inode number of > - an orphan inode. > - * - blocksize - 8 > - - \_\_le32 > - - ob\_magic > - - Magic value stored in orphan block tail (0x0b10ca04) > - * - blocksize - 4 > - - \_\_le32 > - - ob\_checksum > - - Checksum of the orphan block. > +============= ================ =============== =============================== > +Offset Type Name Description > +============= ================ =============== =============================== > +0x0 Array of Orphan inode Each \_\_le32 entry is either > + \_\_le32 entries entries empty (0) or it contains > + inode number of an orphan > + inode. > +blocksize-8 \_\_le32 ob\_magic Magic value stored in orphan > + block tail (0x0b10ca04) > +blocksize-4 \_\_le32 ob\_checksum Checksum of the orphan block. > +============= ================ =============== =============================== > > When a filesystem with orphan file feature is writeably mounted, we set > RO\_COMPAT\_ORPHAN\_PRESENT feature in the superblock to indicate there may > -- > 2.31.1 >
Jan Kara <jack@suse.cz> writes: > On Thu 02-09-21 16:08:53, Jonathan Corbet wrote: >> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added >> a new document on orphan files, which is great. But the use of >> "list-table" results in documents that are absolutely unreadable in their >> plain-text form. Switch this file to the regular RST table format instead; >> the rendered (HTML) output is identical. >> >> Signed-off-by: Jonathan Corbet <corbet@lwn.net> > > Thanks! Definitely looks more readable :). You can add: > > Reviewed-by: Jan Kara <jack@suse.cz> Thanks for having a look! I'll ahead and apply these, then. jon
On Tue, Sep 21, 2021 at 05:18:06PM -0600, Jonathan Corbet wrote: > Jan Kara <jack@suse.cz> writes: > > > On Thu 02-09-21 16:08:53, Jonathan Corbet wrote: > >> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added > >> a new document on orphan files, which is great. But the use of > >> "list-table" results in documents that are absolutely unreadable in their > >> plain-text form. Switch this file to the regular RST table format instead; > >> the rendered (HTML) output is identical. > >> > >> Signed-off-by: Jonathan Corbet <corbet@lwn.net> > > > > Thanks! Definitely looks more readable :). You can add: > > > > Reviewed-by: Jan Kara <jack@suse.cz> > > Thanks for having a look! I'll ahead and apply these, then. Hey Jon, I don't see these patches in linux-next. I'm guessing because you were busy with some silly thing like LPC. :-) Do you want to take them, or I can take them through the ext4 tree. - Ted
"Theodore Ts'o" <tytso@mit.edu> writes: > On Tue, Sep 21, 2021 at 05:18:06PM -0600, Jonathan Corbet wrote: >> Jan Kara <jack@suse.cz> writes: >> >> > On Thu 02-09-21 16:08:53, Jonathan Corbet wrote: >> >> Commit 3a6541e97c03 (Add documentation about the orphan file feature) added >> >> a new document on orphan files, which is great. But the use of >> >> "list-table" results in documents that are absolutely unreadable in their >> >> plain-text form. Switch this file to the regular RST table format instead; >> >> the rendered (HTML) output is identical. >> >> >> >> Signed-off-by: Jonathan Corbet <corbet@lwn.net> >> > >> > Thanks! Definitely looks more readable :). You can add: >> > >> > Reviewed-by: Jan Kara <jack@suse.cz> >> >> Thanks for having a look! I'll ahead and apply these, then. > > Hey Jon, > > I don't see these patches in linux-next. I'm guessing because you > were busy with some silly thing like LPC. :-) > > Do you want to take them, or I can take them through the ext4 tree. Ah...I learned something today. If you try to apply your own patches with "git am -s" (via b4 to lazily grab Jan's Reviewed-by tags), it fails, complaining about duplicate signoff lines. I'd failed to notice that before. I've *really* applied them this time (and tweaked my scripts :). Thanks, jon
On Thu, 02 Sep 2021, Jonathan Corbet <corbet@lwn.net> wrote: > Commit 3a6541e97c03 (Add documentation about the orphan file feature) added > a new document on orphan files, which is great. But the use of > "list-table" results in documents that are absolutely unreadable in their > plain-text form. Switch this file to the regular RST table format instead; > the rendered (HTML) output is identical. > > Signed-off-by: Jonathan Corbet <corbet@lwn.net> > --- > Documentation/filesystems/ext4/orphan.rst | 32 ++++++++--------------- > 1 file changed, 11 insertions(+), 21 deletions(-) > > diff --git a/Documentation/filesystems/ext4/orphan.rst b/Documentation/filesystems/ext4/orphan.rst > index bb19ecd1b626..d096fe0ba19e 100644 > --- a/Documentation/filesystems/ext4/orphan.rst > +++ b/Documentation/filesystems/ext4/orphan.rst > @@ -21,27 +21,17 @@ in heavy creation of orphan inodes. When orphan file feature > (referenced from the superblock through s\_orphan_file_inum) with several > blocks. Each of these blocks has a structure: > > -.. list-table:: > - :widths: 8 8 24 40 > - :header-rows: 1 > - > - * - Offset > - - Type > - - Name > - - Description > - * - 0x0 > - - Array of \_\_le32 entries > - - Orphan inode entries > - - Each \_\_le32 entry is either empty (0) or it contains inode number of > - an orphan inode. > - * - blocksize - 8 > - - \_\_le32 > - - ob\_magic > - - Magic value stored in orphan block tail (0x0b10ca04) > - * - blocksize - 4 > - - \_\_le32 > - - ob\_checksum > - - Checksum of the orphan block. > +============= ================ =============== =============================== > +Offset Type Name Description > +============= ================ =============== =============================== > +0x0 Array of Orphan inode Each \_\_le32 entry is either > + \_\_le32 entries entries empty (0) or it contains > + inode number of an orphan > + inode. > +blocksize-8 \_\_le32 ob\_magic Magic value stored in orphan > + block tail (0x0b10ca04) > +blocksize-4 \_\_le32 ob\_checksum Checksum of the orphan block. > +============= ================ =============== =============================== > > When a filesystem with orphan file feature is writeably mounted, we set > RO\_COMPAT\_ORPHAN\_PRESENT feature in the superblock to indicate there may As a third alternative, the csv-table directive [1] is sometimes a good choice. Picking | as the delim makes it look more like a table in the source, and you don't have to worry about aligning everything (the spaces before and after the delim are ignored by default). But it does require some boilerplate and you can't wrap the lines. The same table as an example: .. csv-table:: Block Structure :delim: | :header-rows: 1 :widths: auto Offset | Type | Name | Description 0x0 | Array of __le32 entries | Orphan inode entries | Each __le32 entry is either empty (0) or it contains inode number of an orphan inode. blocksize-8 | __le32 | ob_magic | Magic value stored in orphan block tail (0x0b10ca04) blocksize-4 | __le32 | ob_checksum | Checksum of the orphan block. Obviously not the best choice for this particular table, but just so you are aware of an alternative. BR, Jani. [1] https://docutils.sourceforge.io/docs/ref/rst/directives.html#csv-table
diff --git a/Documentation/filesystems/ext4/orphan.rst b/Documentation/filesystems/ext4/orphan.rst index bb19ecd1b626..d096fe0ba19e 100644 --- a/Documentation/filesystems/ext4/orphan.rst +++ b/Documentation/filesystems/ext4/orphan.rst @@ -21,27 +21,17 @@ in heavy creation of orphan inodes. When orphan file feature (referenced from the superblock through s\_orphan_file_inum) with several blocks. Each of these blocks has a structure: -.. list-table:: - :widths: 8 8 24 40 - :header-rows: 1 - - * - Offset - - Type - - Name - - Description - * - 0x0 - - Array of \_\_le32 entries - - Orphan inode entries - - Each \_\_le32 entry is either empty (0) or it contains inode number of - an orphan inode. - * - blocksize - 8 - - \_\_le32 - - ob\_magic - - Magic value stored in orphan block tail (0x0b10ca04) - * - blocksize - 4 - - \_\_le32 - - ob\_checksum - - Checksum of the orphan block. +============= ================ =============== =============================== +Offset Type Name Description +============= ================ =============== =============================== +0x0 Array of Orphan inode Each \_\_le32 entry is either + \_\_le32 entries entries empty (0) or it contains + inode number of an orphan + inode. +blocksize-8 \_\_le32 ob\_magic Magic value stored in orphan + block tail (0x0b10ca04) +blocksize-4 \_\_le32 ob\_checksum Checksum of the orphan block. +============= ================ =============== =============================== When a filesystem with orphan file feature is writeably mounted, we set RO\_COMPAT\_ORPHAN\_PRESENT feature in the superblock to indicate there may
Commit 3a6541e97c03 (Add documentation about the orphan file feature) added a new document on orphan files, which is great. But the use of "list-table" results in documents that are absolutely unreadable in their plain-text form. Switch this file to the regular RST table format instead; the rendered (HTML) output is identical. Signed-off-by: Jonathan Corbet <corbet@lwn.net> --- Documentation/filesystems/ext4/orphan.rst | 32 ++++++++--------------- 1 file changed, 11 insertions(+), 21 deletions(-)