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

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

 



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: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux