Re: [PATCH] makedocbook: Fix false report of unhandled texinfo command

[Jon Turney <jon.turney@dronecode.org.uk>]

On 31/10/2022 12:28, Mike Frysinger wrote:
> On 31 Oct 2022 09:59, Jon Turney wrote:
> 
> i guess the feedback below isn't exactly about new code even if it's relevant ...
> 
>> +    # fail if there are unhandled texinfo commands
>> +    match = re.search('(?<!@)@[^@\s]+', s)
> 
> regexes should use raw strings (i.e., this is missing the r prefix)
> 
>> +    if match:
>> +        print("texinfo command '%s' remains in output" % match.group(), file=sys.stderr)
> 
> this is a little dangerous in general as match.group() could return a tuple,
> and this would fail.  if you want to use %, you should force a tuple.
>    print("..." % (match.group(),), ...)

I must be missing something here, because I'm not sure in what sense 
'could' is being used. I think match.group() is defined to be the same 
as match.group(0), which always returns a single string. (I guess this 
could be written explicitly).

So the danger pointed out seems to be that if parts of the code were 
changed, without making other necessary changes, it would be wrong. But 
I'm not sure the suggested change just to avoid that future possibility 
makes things better or clearer.

> 
>> +        exit(1)
> 
> scripts should never use exit(), only sys.exit().  although i see the current
> script gets this wrong in a lot of places.
> 
> also you can simplify this -- sys.exit accepts a string that it'll print to
> stderr and then exit non-zero.
>    sys.exit(".....")

Yes, this is one of the first things I wrote in python, and 
unfortunately, it shows.

I've posted a revised patch-set.