Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ac908c75-4046-df64-efeb-d3e3dea4da74@gmail.com>
Date:   Fri, 19 Aug 2022 22:50:59 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v2 1/3] docs/conf.py: Treat mathjax as fallback math
 renderer
Content-Language: en-US
To:     Jonathan Corbet <corbet@lwn.net>
Cc:     linux-doc@vger.kernel.org,
        Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
        linux-kernel@vger.kernel.org,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Akira Yokosawa <akiyks@gmail.com>
References: <c41cab17-afd6-bc99-56a1-e4e73b8c1ef6@gmail.com>
 <a45f1aec-a2cb-0ef2-7100-378a5130c492@gmail.com>
 <87wnb5bh67.fsf@meer.lwn.net>
From:   Akira Yokosawa <akiyks@gmail.com>
In-Reply-To: <87wnb5bh67.fsf@meer.lwn.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 18 Aug 2022 11:22:24 -0600, Jonathan Corbet wrote:
> Akira Yokosawa <akiyks@gmail.com> writes:
> 
>> Currently, math expressions using the "math::" directive or
>> the ":math:" role of Sphinx need the imgmath extension for proper
>> rendering in html and epub builds.
>> imgmath requires dvipng (and latex).
>> Otherwise, "make htmldocs" will complain of missing commands.
>>
>> As a matter of fact, the mathjax extension is loaded by default since
>> Sphinx v1.8 and it is good enough for html docs without any dependency
>> on texlive packages.
>>
>> Stop loading the imgmath extension for html docs unless requirements
>> for imgmath are met.
>>
>> For epub docs, keep the same behavior of always loading imgmath.
>>
>> Signed-off-by: Akira Yokosawa <akiyks@gmail.com>
>> Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org>
>> ---
>> Changes sinve v1:
>>  - Acked-by from Mauro
>>
>> --
>>  Documentation/conf.py | 34 +++++++++++++++++++++++++++++++++-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/conf.py b/Documentation/conf.py
>> index 934727e23e0e..3ec1f845c839 100644
>> --- a/Documentation/conf.py
>> +++ b/Documentation/conf.py
>> @@ -15,6 +15,23 @@
>>  import sys
>>  import os
>>  import sphinx
>> +from subprocess import check_output
>> +
>> +# helper
>> +# ------
>> +
>> +def have_command(cmd, ver_opt, str_in_ver):
>> +    """Run ```cmd`` with ``ver_opt`` and see if ``str_in_ver`` is found
>> +    or not.
>> +    """
>> +
>> +    try:
>> +        ver_str = check_output([cmd, ver_opt]).decode('utf-8', 'ignore')
>> +        have_cmd = str_in_ver in ver_str
>> +    except:
>> +        have_cmd = False
>> +
>> +    return have_cmd
> 
> So this is adding infrastructure that isn't really mentioned in the
> changelog.
Sorry for missing it.

> 
> A more fundamental comment, though, is that I have learned (the hard
> way, repeatedly) that good things do not come from bare "except"
> statements.  They always hide bugs.  If there is an exception you're
> worried about here, please list it explicitly.
I see.  Thank you for the insightful tip.

I'm more inclined to use the simpler approach of the "which()" function
defined in sphinx/kfigure.py, which is free of try-except constructs.

Will respin this along with the fix of 2/3 in a couple of days.

        Thanks, Akira

> 
> Otherwise seems good.
> 
> Thanks,
> 
> jon