[RFC PATCH 0/3] doc-rst:c-domain: fix some issues in the c-domain

[RFC PATCH 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

From: Markus Heiser <markus.heiser@darmarIT.de>

Hi Jon,

this is a small series, fixing a issues about sphinx version incompatibility and
adds improved handling of function-like macros [1]. The last patch is optional,
I don't know if it is better to create 'FOO (C macro)' index entries instead of
'FOO (C function)' entries (what sphinx does) [2].

[1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg05673.html
[2] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg05678.html

Markus Heiser (3):
  doc-rst:c-domain: fix sphinx version incompatibility
  doc-rst:c-domain: function-like macros arguments
  doc-rst:c-domain: function-like macros index entry

 Documentation/sphinx/cdomain.py | 79 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Markus Heiser]

From: Markus Heiser <markus.heiser@darmarIT.de>

The self.indexnode's tuple has changed in sphinx version 1.4, from a
former 4 element tuple to a 5 element tuple.

https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c

Signed-off-by: Markus Heiser <markus.heiser@darmarIT.de>
---
 Documentation/sphinx/cdomain.py | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/sphinx/cdomain.py b/Documentation/sphinx/cdomain.py
index 9eb714a..66816ae 100644
--- a/Documentation/sphinx/cdomain.py
+++ b/Documentation/sphinx/cdomain.py
@@ -29,11 +29,15 @@ u"""
 
 from docutils.parsers.rst import directives
 
+import sphinx
 from sphinx.domains.c import CObject as Base_CObject
 from sphinx.domains.c import CDomain as Base_CDomain
 
 __version__  = '1.0'
 
+# Get Sphinx version
+major, minor, patch = map(int, sphinx.__version__.split("."))
+
 def setup(app):
 
     app.override_domain(CDomain)
@@ -85,8 +89,14 @@ class CObject(Base_CObject):
 
         indextext = self.get_index_text(name)
         if indextext:
-            self.indexnode['entries'].append(('single', indextext,
-                                              targetname, '', None))
+            if major >= 1 and minor < 4:
+                # indexnode's tuple changed in 1.4
+                # https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c
+                self.indexnode['entries'].append(
+                    ('single', indextext, targetname, ''))
+            else:
+                self.indexnode['entries'].append(
+                    ('single', indextext, targetname, '', None))
 
 class CDomain(Base_CDomain):
 
-- 
2.7.4

Re: [PATCH 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Jonathan Corbet]

On Wed, 31 Aug 2016 17:29:30 +0200
Markus Heiser <markus.heiser@darmarit.de> wrote:

> +            if major >= 1 and minor < 4:
> +                # indexnode's tuple changed in 1.4
> +                # https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c
> +                self.indexnode['entries'].append(
> +                    ('single', indextext, targetname, ''))
> +            else:
> +                self.indexnode['entries'].append(
> +                    ('single', indextext, targetname, '', None))

So this doesn't seem right.  We'll get the four-entry tuple behavior with
1.3 and the five-entry behavior with 1.4...but what happens when 2.0
comes out?

Did you want maybe:

	if major == 1 and minor < 4:

?

(That will fail on 0.x, but we've already stated that we don't support
below 1.2).

jon

Re: [PATCH 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Markus Heiser]

Am 06.09.2016 um 14:19 schrieb Jonathan Corbet <corbet@lwn.net>:

> On Wed, 31 Aug 2016 17:29:30 +0200
> Markus Heiser <markus.heiser@darmarit.de> wrote:
> 
>> +            if major >= 1 and minor < 4:
>> +                # indexnode's tuple changed in 1.4
>> +                # https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c
>> +                self.indexnode['entries'].append(
>> +                    ('single', indextext, targetname, ''))
>> +            else:
>> +                self.indexnode['entries'].append(
>> +                    ('single', indextext, targetname, '', None))
> 
> So this doesn't seem right.  We'll get the four-entry tuple behavior with
> 1.3 and the five-entry behavior with 1.4...but what happens when 2.0
> comes out?
> 
> Did you want maybe:
> 
> 	if major == 1 and minor < 4:
> 
> ?

Ups, yes you are right.

Should I send a new patch .. or could you fix it?

-- Markus --

> 
> (That will fail on 0.x, but we've already stated that we don't support
> below 1.2).
> 
> jon

Re: [PATCH 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Jonathan Corbet]

On Tue, 6 Sep 2016 14:24:11 +0200
Markus Heiser <markus.heiser@darmarit.de> wrote:

> Should I send a new patch .. or could you fix it?

Please just regenerate the series and I'll apply it.

Thanks,

jon

Re: [PATCH 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Jani Nikula]

On Tue, 06 Sep 2016, Jonathan Corbet <corbet@lwn.net> wrote:
> On Wed, 31 Aug 2016 17:29:30 +0200
> Markus Heiser <markus.heiser@darmarit.de> wrote:
>
>> +            if major >= 1 and minor < 4:
>> +                # indexnode's tuple changed in 1.4
>> +                # https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c
>> +                self.indexnode['entries'].append(
>> +                    ('single', indextext, targetname, ''))
>> +            else:
>> +                self.indexnode['entries'].append(
>> +                    ('single', indextext, targetname, '', None))
>
> So this doesn't seem right.  We'll get the four-entry tuple behavior with
> 1.3 and the five-entry behavior with 1.4...but what happens when 2.0
> comes out?
>
> Did you want maybe:
>
> 	if major == 1 and minor < 4:
>
> ?
>
> (That will fail on 0.x, but we've already stated that we don't support
> below 1.2).

Is there a way to check the number of entries expected in the tuples
instead of trying to match the version?

BR,
Jani.




-- 
Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Markus Heiser]

Am 06.09.2016 um 15:34 schrieb Jani Nikula <jani.nikula@intel.com>:

> On Tue, 06 Sep 2016, Jonathan Corbet <corbet@lwn.net> wrote:
>> On Wed, 31 Aug 2016 17:29:30 +0200
>> Markus Heiser <markus.heiser@darmarit.de> wrote:
>> 
>>> +            if major >= 1 and minor < 4:
>>> +                # indexnode's tuple changed in 1.4
>>> +                # https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c
>>> +                self.indexnode['entries'].append(
>>> +                    ('single', indextext, targetname, ''))
>>> +            else:
>>> +                self.indexnode['entries'].append(
>>> +                    ('single', indextext, targetname, '', None))
>> 
>> So this doesn't seem right.  We'll get the four-entry tuple behavior with
>> 1.3 and the five-entry behavior with 1.4...but what happens when 2.0
>> comes out?
>> 
>> Did you want maybe:
>> 
>> 	if major == 1 and minor < 4:
>> 
>> ?
>> 
>> (That will fail on 0.x, but we've already stated that we don't support
>> below 1.2).
> 
> Is there a way to check the number of entries expected in the tuples
> instead of trying to match the version?

Sadly not, the dissection of the tuple is spread around the source :(

Sphinx has some more of these tuples with fixed length (remember
conf.py, the latex_documents settings) where IMHO hash/value pairs
(dicts) are more suitable.

-- Markus --
> BR,
> Jani.
> -- 
> Jani Nikula, Intel Open Source Technology Center

Re: [PATCH 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Mauro Carvalho Chehab]

Em Tue, 6 Sep 2016 17:10:53 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> Am 06.09.2016 um 15:34 schrieb Jani Nikula <jani.nikula@intel.com>:
> 
> > On Tue, 06 Sep 2016, Jonathan Corbet <corbet@lwn.net> wrote:  
> >> On Wed, 31 Aug 2016 17:29:30 +0200
> >> Markus Heiser <markus.heiser@darmarit.de> wrote:
> >>   
> >>> +            if major >= 1 and minor < 4:
> >>> +                # indexnode's tuple changed in 1.4
> >>> +                # https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c
> >>> +                self.indexnode['entries'].append(
> >>> +                    ('single', indextext, targetname, ''))
> >>> +            else:
> >>> +                self.indexnode['entries'].append(
> >>> +                    ('single', indextext, targetname, '', None))  
> >> 
> >> So this doesn't seem right.  We'll get the four-entry tuple behavior with
> >> 1.3 and the five-entry behavior with 1.4...but what happens when 2.0
> >> comes out?
> >> 
> >> Did you want maybe:
> >> 
> >> 	if major == 1 and minor < 4:
> >> 
> >> ?
> >> 
> >> (That will fail on 0.x, but we've already stated that we don't support
> >> below 1.2).  
> > 
> > Is there a way to check the number of entries expected in the tuples
> > instead of trying to match the version?  
> 
> Sadly not, the dissection of the tuple is spread around the source :(
> 
> Sphinx has some more of these tuples with fixed length (remember
> conf.py, the latex_documents settings) where IMHO hash/value pairs
> (dicts) are more suitable.

Well, the LaTeX stuff at conf.py seems to have a new field on version
1.4.x. At least, our config has:

# (source start file, name, description, authors, manual section).

but 1.4.x docs mentions another tuple: toctree_only.

Regards,
Mauro

Re: [PATCH 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Markus Heiser]

Am 06.09.2016 um 17:55 schrieb Mauro Carvalho Chehab <mchehab@infradead.org>:
...
>> Sphinx has some more of these tuples with fixed length (remember
>> conf.py, the latex_documents settings) where IMHO hash/value pairs
>> (dicts) are more suitable.
> 
> Well, the LaTeX stuff at conf.py seems to have a new field on version
> 1.4.x. At least, our config has:
> 
> # (source start file, name, description, authors, manual section).
> 
> but 1.4.x docs mentions another tuple: toctree_only.

Hmm, as far as I can see, toctree_only is supported since Release 0.3 
(May 6, 2008):

  https://github.com/sphinx-doc/sphinx/blob/master/CHANGES.old#L1025

But is was implemented optional (from the beginning):

 https://github.com/sphinx-doc/sphinx/blame/master/sphinx/builders/latex.py#L104

-- Markus --

> 
> Regards,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] doc-rst:c-domain: function-like macros arguments

[Markus Heiser]

From: Markus Heiser <markus.heiser@darmarIT.de>

Handle signatures of function-like macros well. Don't try to deduce
arguments types of function-like macros.

Signed-off-by: Markus Heiser <markus.heiser@darmarIT.de>
---
 Documentation/sphinx/cdomain.py | 55 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/Documentation/sphinx/cdomain.py b/Documentation/sphinx/cdomain.py
index 66816ae..0816090 100644
--- a/Documentation/sphinx/cdomain.py
+++ b/Documentation/sphinx/cdomain.py
@@ -1,4 +1,5 @@
 # -*- coding: utf-8; mode: python -*-
+# pylint: disable=W0141,C0113,C0103,C0325
 u"""
     cdomain
     ~~~~~~~
@@ -25,11 +26,18 @@ u"""
 
           * :c:func:`VIDIOC_LOG_STATUS` or
           * :any:`VIDIOC_LOG_STATUS` (``:any:`` needs sphinx 1.3)
+
+     * Handle signatures of function-like macros well. Don't try to deduce
+       arguments types of function-like macros.
+
 """
 
+from docutils import nodes
 from docutils.parsers.rst import directives
 
 import sphinx
+from sphinx import addnodes
+from sphinx.domains.c import c_funcptr_sig_re, c_sig_re
 from sphinx.domains.c import CObject as Base_CObject
 from sphinx.domains.c import CDomain as Base_CDomain
 
@@ -38,6 +46,7 @@ __version__  = '1.0'
 # Get Sphinx version
 major, minor, patch = map(int, sphinx.__version__.split("."))
 
+
 def setup(app):
 
     app.override_domain(CDomain)
@@ -57,9 +66,53 @@ class CObject(Base_CObject):
         "name" : directives.unchanged
     }
 
+    def handle_func_like_macro(self, sig, signode):
+        u"""Handles signatures of function-like macros.
+
+        If the objtype is 'function' and the the signature ``sig`` is a
+        function-like macro, the name of the macro is returned. Otherwise
+        ``False`` is returned.  """
+
+        if not self.objtype == 'function':
+            return False
+
+        m = c_funcptr_sig_re.match(sig)
+        if m is None:
+            m = c_sig_re.match(sig)
+        if m is None:
+            raise ValueError('no match')
+
+        rettype, fullname, arglist, _const = m.groups()
+        if rettype or not arglist.strip():
+            return False
+
+        arglist = arglist.replace('`', '').replace('\\ ', '').strip()  # remove markup
+        arglist = [a.strip() for a in arglist.split(",")]
+
+        # has the first argument a type?
+        if len(arglist[0].split(" ")) > 1:
+            return False
+
+        # This is a function-like macro, it's arguments are typeless!
+        signode  += addnodes.desc_name(fullname, fullname)
+        paramlist = addnodes.desc_parameterlist()
+        signode  += paramlist
+
+        for argname in arglist:
+            param = addnodes.desc_parameter('', '', noemph=True)
+            # separate by non-breaking space in the output
+            param += nodes.emphasis(argname, argname)
+            paramlist += param
+
+        return fullname
+
     def handle_signature(self, sig, signode):
         """Transform a C signature into RST nodes."""
-        fullname = super(CObject, self).handle_signature(sig, signode)
+
+        fullname = self.handle_func_like_macro(sig, signode)
+        if not fullname:
+            fullname = super(CObject, self).handle_signature(sig, signode)
+
         if "name" in self.options:
             if self.objtype == 'function':
                 fullname = self.options["name"]
-- 
2.7.4

Re: [PATCH 2/3] doc-rst:c-domain: function-like macros arguments

[Jonathan Corbet]

So I'm going into total nit-picking territory here, but since I'm looking
at it and I think the series needs a respin anyway...

On Wed, 31 Aug 2016 17:29:31 +0200
Markus Heiser <markus.heiser@darmarit.de> wrote:

> +        m = c_funcptr_sig_re.match(sig)
> +        if m is None:
> +            m = c_sig_re.match(sig)
> +        if m is None:
> +            raise ValueError('no match')

How about we put that second test inside the first if block and avoid the
redundant None test if the first match works?  The energy saved may
prevent a hurricane someday :)

> +
> +        rettype, fullname, arglist, _const = m.groups()
> +        if rettype or not arglist.strip():
> +            return False
> +
> +        arglist = arglist.replace('`', '').replace('\\ ', '').strip()  # remove markup
> +        arglist = [a.strip() for a in arglist.split(",")]

Similarly, stripping the args three times seems a bit much.  The middle
one is totally redundant and could go at a minimum.

Thanks,

jon

Re: [PATCH 2/3] doc-rst:c-domain: function-like macros arguments

[Markus Heiser]

Am 06.09.2016 um 14:27 schrieb Jonathan Corbet <corbet@lwn.net>:

> So I'm going into total nit-picking territory here, but since I'm looking
> at it and I think the series needs a respin anyway...
> 
> On Wed, 31 Aug 2016 17:29:31 +0200
> Markus Heiser <markus.heiser@darmarit.de> wrote:
> 
>> +        m = c_funcptr_sig_re.match(sig)
>> +        if m is None:
>> +            m = c_sig_re.match(sig)
>> +        if m is None:
>> +            raise ValueError('no match')
> 
> How about we put that second test inside the first if block and avoid the
> redundant None test if the first match works?  The energy saved may
> prevent a hurricane someday :)

And prohibit the MS-Windows update installer will save the climate ;)
It is a habit of mine to avoid indentations, but you are right,
it is not appropriate here.

>> +
>> +        rettype, fullname, arglist, _const = m.groups()
>> +        if rettype or not arglist.strip():
>> +            return False
>> +
>> +        arglist = arglist.replace('`', '').replace('\\ ', '').strip()  # remove markup
>> +        arglist = [a.strip() for a in arglist.split(",")]
> 
> Similarly, stripping the args three times seems a bit much.  The middle
> one is totally redundant and could go at a minimum.

Thanks for pointing this. You are right, I will fix it.

-- Markus --

> 
> Thanks,
> 
> jon

[PATCH 3/3] doc-rst:c-domain: function-like macros index entry

[Markus Heiser]

From: Markus Heiser <markus.heiser@darmarIT.de>

For function-like macros, sphinx creates 'FOO (C function)' entries.
With this patch 'FOO (C macro)' are created for function-like macros,
which is the same for object-like macros.

Signed-off-by: Markus Heiser <markus.heiser@darmarIT.de>
---
 Documentation/sphinx/cdomain.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/sphinx/cdomain.py b/Documentation/sphinx/cdomain.py
index 0816090..2a1bd09 100644
--- a/Documentation/sphinx/cdomain.py
+++ b/Documentation/sphinx/cdomain.py
@@ -37,6 +37,7 @@ from docutils.parsers.rst import directives
 
 import sphinx
 from sphinx import addnodes
+from sphinx.locale import _
 from sphinx.domains.c import c_funcptr_sig_re, c_sig_re
 from sphinx.domains.c import CObject as Base_CObject
 from sphinx.domains.c import CDomain as Base_CDomain
@@ -66,6 +67,8 @@ class CObject(Base_CObject):
         "name" : directives.unchanged
     }
 
+    is_function_like_macro = False
+
     def handle_func_like_macro(self, sig, signode):
         u"""Handles signatures of function-like macros.
 
@@ -104,6 +107,7 @@ class CObject(Base_CObject):
             param += nodes.emphasis(argname, argname)
             paramlist += param
 
+        self.is_function_like_macro = True
         return fullname
 
     def handle_signature(self, sig, signode):
@@ -151,6 +155,12 @@ class CObject(Base_CObject):
                 self.indexnode['entries'].append(
                     ('single', indextext, targetname, '', None))
 
+    def get_index_text(self, name):
+        if self.is_function_like_macro:
+            return _('%s (C macro)') % name
+        else:
+            return super(CObject, self).get_index_text(name)
+
 class CDomain(Base_CDomain):
 
     """C language domain."""
-- 
2.7.4

Re: [PATCH 3/3] doc-rst:c-domain: function-like macros index entry

[Jonathan Corbet]

On Wed, 31 Aug 2016 17:29:32 +0200
Markus Heiser <markus.heiser@darmarit.de> wrote:

> For function-like macros, sphinx creates 'FOO (C function)' entries.
> With this patch 'FOO (C macro)' are created for function-like macros,
> which is the same for object-like macros.

As others have pointed out, we generally want to hide the difference
between functions and macros, so this is probably one change we don't
want.

Thanks,

jon