Em Tue, 6 Oct 2020 12:19:53 -0700
Eric Biggers <ebiggers@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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
