Re: [PATCH] find: doc: Fix -prune SCM example and really make it efficient

[Bernhard Voelker]

First of all: thanks for the patch.
A concrete code change is always a good basis for discussion. :-)

On 10/23/22 01:54, raf wrote:
> Subject: [PATCH] find: doc: Fix -prune SCM example and really make it 
efficient

IMO we shouldn't call this a "fix", because there was nothing wrong with the 
previous sample code.
See below.

> * find/find.1 - Fix explanation of -prune SCM example and make it efficient
> * doc/find.texi - Make -prune SCM example efficient
> ---
>   NEWS          |  5 +++++
>   doc/find.texi |  6 +++---
>   find/find.1   | 19 ++++++++++++++-----
>   3 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index dffca5aa..b3f2074c 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -13,6 +13,11 @@ GNU findutils NEWS - User visible changes.      -*- outline 
> -*- (allout)
>     findutils now builds again on systems with musl-libc.
>     This requires gettext-0.19.8.
>   
> +** Documentation Changes
> +
> +  The find.1 manual and the Texinfo manual -prune SCM example has
> +  been corrected (manual) and made much more efficient (both) [#62259]

again, the previous version was not incorrect.

> +
>   
>   * Noteworthy changes in release 4.9.0 (2022-02-22) [stable]
>   
> diff --git a/doc/find.texi b/doc/find.texi
> index 1f295837..8c3972d4 100644
> --- a/doc/find.texi
> +++ b/doc/find.texi
> @@ -5139,9 +5139,9 @@ already found.
>   
>   @smallexample
>   find repo/ \
> --exec test -d @{@}/.svn \; -or \
> --exec test -d @{@}/.git \; -or \
> --exec test -d @{@}/CVS \; -print -prune
> +    -type d \
> +    -exec test -d @{@}/.svn -o -d @{@}/.git -o -d @{@}/CVS \; \
> +    -print -prune
>   @end smallexample

The purpose was more to get an idea about how pruning works rather than getting 
the most
efficient way for the example use case.  Of course, we could and should also 
give a direction
about an even more efficient way.
In that example, this works because the 'test' utility - most probably coming 
from coreutils
or a compatible implementation - allows to use the -o operator to do more 
checks in one
process invocation.

What about guiding the user in the documentation by saying that the '-exec ... 
-or -exec ...'
is the basic way in find to run several checks against the current entry, and 
to give a hint
that in this particular case the `test` utility allows to reduce the number of 
execv()s by
using the -o operator of that tool?

BTW: `make syntax-check` complains about the new syntax:

  $ make syntax-check
  ...
  prohibit_test_minus_ao
  doc/find.texi:5143:    -exec test -d @{@}/.svn -o -d @{@}/.git -o -d @{@}/CVS 
\; \
  maint.mk: use "test C1 && test C2", not "test C1 -a C2"; use "test C1 || test C2", not 
"test C1 -o C2"
  make: *** [maint.mk:1099: sc_prohibit_test_minus_ao] Error 1

It seems we have to exempt the texi file from that check.

>   In this example, @command{test} is used to tell if we are currently
> diff --git a/find/find.1 b/find/find.1
> index 429aa2f0..f5fa4eee 100644
> --- a/find/find.1
> +++ b/find/find.1
> @@ -2494,15 +2494,14 @@ projects' roots:
>   .in +4m
>   .B $ find repo/ \e
>   .in +4m
> -.B \e( \-exec test \-d \(aq{}/.svn\(aq \e; \e
> -.B \-or \-exec test \-d \(aq{}/.git\(aq \e; \e
> -.B \-or \-exec test \-d \(aq{}/CVS\(aq \e; \e
> -.B \e) \-print \-prune
> +.B \-type d \e
> +.B \-exec test \-d \(aq{}/.svn\(aq \-o \-d \(aq{}/.git\(aq \-o \-d 
> \(aq{}/CVS\(aq \e; \e
> +.B \-print \-prune
>   .in -4m
>   .in -4m
>   \&
>   .fi
> -Sample output:
> +Sample directories:

ugg, yes, the output does not contain the SCM directories.  That was wrong.  
Good catch!

>   .nf
>   \&
>   .in +4m
> @@ -2513,6 +2512,16 @@ Sample output:
>   .B repo/project4/.git
>   .in
>   \&
> +Sample output:
> +.nf
> +\&
> +.in +4m
> +.B repo/project1
> +.B repo/gnu/project2
> +.B repo/gnu/project3
> +.B repo/project4
> +.in
> +\&
>   .fi
>   In this example,
>   .B \-prune

Would you like to propose a v2?

P.S. You've sent more patches already, and they're becoming more non-trivial.
This requires that we have a Copyright assignment of you and your employer in
place.  I think I mentioned this already last time, but this got out of my 
focus.
Would you like to proceed with the FSF copyright paperwork, please?
It's just that we can only accept trivial patches without an official Copyright
assignment to the FSF - this one would still be okay because of its size,
but it seems you want to contribute more ... which is great.
Of course, I can and will assist you in that regard - we're always in need of
more official contributors.

Thanks & have a nice day,
Berny