Re: [PATCH] scripts/kernel-doc: add internal hyperlink to DOC: sections

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 29.01.2021 11:01, Jani Nikula wrote:
> On Thu, 28 Jan 2021, Jonathan Corbet <corbet@xxxxxxx> wrote:
>> On Mon, 18 Jan 2021 12:08:13 +0100
>> Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx> wrote:
>>
>>> While DOC: section titles are not converted into RST headings
>>> sections and are only decorated with strong emphasis markup,
>>> nothing stops us from generating internal hyperlinks for them,
>>> to mimic implicit hyperlinks to RST headings.
>>>
>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
>>> Cc: Jonathan Corbet <corbet@xxxxxxx>
>>> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
>>> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>>
>> So I've applied this, but ...
>>
>>>  scripts/kernel-doc | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
>>> index 6325bec3f66f..272f70c9fb13 100755
>>> --- a/scripts/kernel-doc
>>> +++ b/scripts/kernel-doc
>>> @@ -833,6 +833,7 @@ sub output_blockhead_rst(%) {
>>>  	next if (defined($nosymbol_table{$section}));
>>>  
>>>  	if ($output_selection != OUTPUT_INCLUDE) {
>>> +	    print ".. _$section:\n\n";
>>>  	    print "**$section**\n\n";
>>>  	}
>>
>> ...the placement within this if means that the section marker doesn't
>> appear whenever a doc block is explicitly included with the :doc: modifier
>> - which I think is much of the time.  I *think* the marker should be
>> output unconditionally.
>>
>> Jani, you appear to be the culprit behind that particular "if", which
>> seems a little questionable to me in general at the moment, but you must
>> have had a good reason.  Do you see any reason why a section marker should
>> be suppressed in the same way?
> 
> ISTR there were a lot of explicit doc block inclusion where the .rst
> file already had a section heading, so this would have lead to double
> heading. (Specifically, actual heading from rst, followed by a bolded
> line from the doc comment title.)
> 
> Turning the doc comment title into an actual heading would require
> knowing the surrounding heading level i.e. which underline character to
> use. This could be done by passing the character to kernel-doc with the
> directive, and omitting the heading in the .rst, but this in turn makes
> the .rst less readable as plain text because you lose the structure.
> 
> It's not perfect, but I'm not sure what the right fix to that should be.
> 
> As to the patch at hand, it does seem like moving the hyperlink target
> outside of the if block here is the right thing to do. We'll want the
> targets to work regardless of how the doc comment is included. I'd seen
> Michal's patch before, but didn't think of this at the time.

The problem is that in majority of cases where we have explicit heading
in .rst file, the same name is used as DOC: title. If we unconditionally
add target for every DOC: title, then Sphinx will complain with:

"WARNING: Duplicate target name, cannot be used as a unique reference"

and render documentation with non-working cross references.

AFAIK, this patch as-is does not introduce any new warning.

Thanks,
Michal



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux