Re: [PATCH] INSTALL, Makefile, cmd.mk, lint-man.mk: Lint about '\" t' comment for tbl(1)

[Alejandro Colomar <alx.manpages@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3334 bytes --]

Hi Mike!

On 11/9/22 16:58, Mike Frysinger wrote:
> On 09 Nov 2022 16:18, Alejandro Colomar wrote:
>> --- a/lib/lint-man.mk
>> +++ b/lib/lint-man.mk
> 
> i guess not a new issue, but it feels like writing lint logic in Makefiles
> is not the best use of time.  this logic is really hairy.

Hmm, well, the line is not so clear.  But I'd say this is the second case where 
I add lint logic hardcoded in the makefile.  The first case was for checking 
that rendered output fits in a terminal (80 col):

<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/tree/lib/lint-man.mk#n82>

All other lints are just running the linter, which in some cases is just a 
single command, and in others is a pipeline.

But this one is the most clear example of lint logic in the Makefiles, yes.


Option B would be to write those 3 conditionals in a script under <./scripts/>, 
and call it from the Makefile.  Since I don't expect this (hardcoding lint logic 
in the Makefile) to happen very often, I'm not tempted enough to do that.  For 
now, a few lines of embedded code seem reasonable to me.

But please feel free to propose a different approach.

> 
>> +$(_LINT_man_tbl): $(_LINTDIR)/%.lint-man.tbl.touch: $(MANDIR)/% | $$(@D)/.
>> +	$(info LINT (tbl)	$@)
>> +	if $(GREP) '^\.TS$$' <$< >/dev/null && ! $(HEAD) -n1 <$< | $(GREP) '\\" t$$' >/dev/null; then \
> 
> POSIX grep has a -q option so you don't have to redirect to /dev/null.
> 	if $(GREP) -q '^\.TS$$' <$< && ...

Hmm, I'm usually not a fan of it (I was in the past, but I've learnt to prefer 
pipes and redirects).  But in this case it helps keep the lines shorter, so yes, 
I like it for this code.

> 
> also, is there a reason you're using a redirect instead of just passing the
> file to grep ?  the redirect works, but it seems to contribute to general
> "this code is hard for humans to read".  i don't think you really need to
> be concerned with files starting with dashes ...
> 	if $(GREP) -q '^\.TS$$' $< && ...
> 
> or more completely:
> 	if $(GREP) -q '^\.TS$$' $< && ! $(HEAD) -n1 $< | $(GREP) -q '\\" t$$'; then

I have grown a tendency to use pipes and redirection instead of filenames and 
options.  Especially when writing my own filters.

When I write long scripts where each command goes on a line of its own, I prefer 
that very much.  It helps readability IMO.  As an example, <./scripts/bash_aliases>.

However, for 'if's, it's much more clear when the condition is a one-liner, and 
that changes some things.  Having the line under 80 col helps in readability a 
lot more than redirections.  And having fewer special characters also helps 
parse the complex line.  So, yes, I agree that in this case it's better to make 
it shorter as you propose.  I'll send an updated v2 soon.

> 
>> +	fi;
> 
> don't need this trailing semi-colon

In bash scripts, I tend to always write semicolons.  I like their preciseness. 
In Makefiles, however, I learnt recently that GNU make makes a difference when 
you use them: it can't optimize certain things.  So it's actually better to 
remove them.

I just put them out of inertia from the rest of the embedded script, and agree 
that it's better without them.

> -mike

Thanks!

Cheers,

Alex

-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]