Re: [PATCH v2] kernel-doc: rename the kernel-doc directive 'functions' to 'identifiers'

On Sun, 20 Oct 2019 21:17:17 +0800 Changbin Du changbin.du@gmail.com wrote:

The 'functions' directive is not only for functions, but also works for structs/unions. So the name is misleading. This patch renames it to 'identifiers', which specific the functions/types to be included in documentation. We keep the old name as an alias of the new one before all documentation are updated.

Signed-off-by: Changbin Du changbin.du@gmail.com

So I think this is basically OK, but I have one more request...

[...]

diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py index 1159405cb920..0689f9c37f1e 100644 --- a/Documentation/sphinx/kerneldoc.py +++ b/Documentation/sphinx/kerneldoc.py @@ -59,9 +59,10 @@ class KernelDocDirective(Directive): optional_arguments = 4 option_spec = { 'doc': directives.unchanged_required,

•    'functions': directives.unchanged,
     'export': directives.unchanged,
     'internal': directives.unchanged,
  

•    'identifiers': directives.unchanged,
  

•    'functions': directives.unchanged,  # alias of 'identifiers'
  

  } has_content = False

@@ -71,6 +72,7 @@ class KernelDocDirective(Directive): filename = env.config.kerneldoc_srctree + '/' + self.arguments[0] export_file_patterns = []

•    identifiers = None
  

# Tell sphinx of the dependency env.note_dependency(os.path.abspath(filename)) @@ -86,19 +88,22 @@ class KernelDocDirective(Directive): export_file_patterns = str(self.options.get('internal')).split() elif 'doc' in self.options: cmd += ['-function', str(self.options.get('doc'))]

•    elif 'identifiers' in self.options:
  

•        identifiers = self.options.get('identifiers').split()
     elif 'functions' in self.options:
  

•        functions = self.options.get('functions').split()
  

•        if functions:
  

•            for f in functions:
  

•                cmd += ['-function', f]
  

•        else:
  

•            cmd += ['-no-doc-sections']
  

•        identifiers = self.options.get('functions').split()
  

Rather than do this, can you just change the elif line to read:

elif ('identifiers' in self.options) or ('functions' in self.options):

...then leave the rest of the code intact? It keeps the logic together, and avoids the confusing distinction between identifiers=='' and identifiers==None .

Thanks,

jon

     for pattern in export_file_patterns:
         for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
             env.note_dependency(os.path.abspath(f))
             cmd += ['-export-file', f]

•    if identifiers:
  

•        for i in identifiers:
  

•            cmd += ['-function', i]
  

•    elif identifiers is not None:
  

•        cmd += ['-no-doc-sections']
  

•    cmd += [filename]
  

try: