From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Linux Doc Mailing List <linux-doc@vger.kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
"Theodore Y. Ts'o" <tytso@mit.edu>,
Jaegeuk Kim <jaegeuk@kernel.org>,
linux-fscrypt@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 35/52] docs: fs: fscrypt.rst: get rid of :c:type: tags
Date: Wed, 14 Oct 2020 09:12:40 +0200 [thread overview]
Message-ID: <20201014091240.78ba3425@coco.lan> (raw)
In-Reply-To: <20201006191953.GA3598358@gmail.com>
Em Tue, 6 Oct 2020 12:19:53 -0700
Eric Biggers <ebiggers@kernel.org> escreveu:
> On Tue, Oct 06, 2020 at 04:03:32PM +0200, Mauro Carvalho Chehab wrote:
> > The :c:type: tag has problems with Sphinx 3.x, as structs
> > there should be declared with c:struct.
> >
> > So, remove them, relying at automarkup.py extension to
> > convert them into cross-references.
>
> I tried 'make htmldocs' before and after your patchset ("sphinx3-fixes-v5").
> Before, all the struct fscrypt_* are rendered in code font. After, they are
> rendered in the regular text font. Is that really working as intended?
It is up to automarkup.py to change from "struct foo" into:
:c:type:`struct foo` (Sphinx 2.x)
or:
:c:struct:`foo` (Sphinx 3.x)
At v5, the automarkup.py extension was disabled, as it was broken
with Sphinx > 2.x. At v6, I added a patch from Nícolas addressing
it.
It should be said that, currently, if there's no documentation for
"foo", automarkup will just keep using the regular text font,
keeping the text untouched.
>
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> > Documentation/filesystems/fscrypt.rst | 51 ++++++++++++---------------
> > 1 file changed, 23 insertions(+), 28 deletions(-)
> >
>
> Why are the changes to fscrypt.rst split between two patches,
>
> docs: get rid of :c:type explicit declarations for structs
>
> and
>
> docs: fs: fscrypt.rst: get rid of :c:type: tags
>
> ? They're the same type of changes. The first just removes half the :c:type:
> tags, and the second removes the rest. Shouldn't it be one patch?
>
The reason is just because it was easier this way.
On the first patch, I used sed to replace structs on a
semi-automated way, checking the results.
at the second one, I addressed the remaining symbols manually.
Anyway, at the new version, I just placed everything related
to fscript.rst at the same patch, to make easier to review.
> > diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
> > index 4f858b38a412..46a9d1bd2ab5 100644
> > --- a/Documentation/filesystems/fscrypt.rst
> > +++ b/Documentation/filesystems/fscrypt.rst
> > @@ -437,8 +437,7 @@ FS_IOC_SET_ENCRYPTION_POLICY
> > The FS_IOC_SET_ENCRYPTION_POLICY ioctl sets an encryption policy on an
> > empty directory or verifies that a directory or regular file already
> > has the specified encryption policy. It takes in a pointer to a
> > -struct fscrypt_policy_v1 or a :c:type:`struct
> > -fscrypt_policy_v2`, defined as follows::
> > +struct fscrypt_policy_v1 or a struct fscrypt_policy_v2, defined as follows::
> [...]
> > If the file is not yet encrypted, then FS_IOC_SET_ENCRYPTION_POLICY
> > verifies that the file is an empty directory. If so, the specified
> > @@ -637,9 +634,8 @@ The FS_IOC_GET_ENCRYPTION_POLICY ioctl can also retrieve the
> > encryption policy, if any, for a directory or regular file. However,
> > unlike `FS_IOC_GET_ENCRYPTION_POLICY_EX`_,
> > FS_IOC_GET_ENCRYPTION_POLICY only supports the original policy
> > -version. It takes in a pointer directly to a :c:type:`struct
> > -fscrypt_policy_v1` rather than a :c:type:`struct
> > -fscrypt_get_policy_ex_arg`.
> > +version. It takes in a pointer directly to struct fscrypt_policy_v1
> > +rather than struct fscrypt_get_policy_ex_arg.
>
> In some cases you deleted the "a" in "a struct" but in other cases you didn't.
> Intentional? It seems the file should consistently use one style or the other.
Yes, it was intentional. On almost all other docs documents I reviewed or
converted, they say "struct" instead of "a struct".
At the second version, I did the replacement on a consistent way.
>
> Also please use textwidth=70 for consistency with the rest of the file.
Done. At the new patch I posted, none of the lines touched by the
patch uses more than 70 columns.
You may notice that I opted to keep "struct foo" at the same line.
This is not a mandatory requirement for automarkup.py to work, but
I would recommend keeping them at the same line, as, if someone tries to
do something like:
$ git grep "struct foo" Documentation/
It would be able to find them.
Thanks,
Mauro
prev parent reply other threads:[~2020-10-14 7:12 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1601992016.git.mchehab+huawei@kernel.org>
2020-10-06 14:03 ` [PATCH v5 22/52] docs: get rid of :c:type explicit declarations for structs Mauro Carvalho Chehab
2020-10-06 16:41 ` Greg Kroah-Hartman
2020-10-06 14:03 ` [PATCH v5 35/52] docs: fs: fscrypt.rst: get rid of :c:type: tags Mauro Carvalho Chehab
2020-10-06 19:19 ` Eric Biggers
2020-10-14 7:12 ` Mauro Carvalho Chehab [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201014091240.78ba3425@coco.lan \
--to=mchehab+huawei@kernel.org \
--cc=corbet@lwn.net \
--cc=ebiggers@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fscrypt@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).