On Wed, Oct 26, 2022 at 03:20:09PM +0200, Bernhard Voelker <m...@bernhard-voelker.de> wrote:
> On 10/26/22 11:33, James Youngman wrote: > > The style "test X -o Y" is obsolescent in POSIX (citation: > > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html > > <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html>). > > > The POSIX standard recommends the use instead of test X || test > > Y. Which is, in effect, what we are doing in the existing code. > > > > Supposing efficiency is an overriding concern we could use something like > > this: Efficiency is only a concern because the example is described as being efficient, but the reason for that is its use of -prune. Three sh processes per directory would be fine, but it's currently doing three sh processes for every file type, not just directories. That seems to me to just be too wasteful. That's why I want to insert "-type d" before the -exec. I'm not that concerned with combining the three -execs into one. It just seemed like a good idea at the time. > > -exec sh -c 'test -d "$1"/.svn || test -d "$1"/.git || test -d "$1"/CVS' > > fnord {} \; > > > > The fnord there of course is assigned to $0. The above would need > > careful testing for space handling in particular. The quoted environment variables do protect against shell meta-characters like spaces and quotes in names. I think this would have been nicer: -exec sh -c 'test -d {}/.svn || test -d {}/.git || test -d {}/CVS' \; BUT: I just tested this with a directory name containing a space, and {} doesn't automatically backslash-quote shell meta-characters when it's used as part of a larger command line argument that might result in multiple shell tokens. That's a surprise to me. I'd consider it a bug, but changing it might not be backwards-compatible (but it might be). Quoting the {} isn't enough. Shells won't treat it the same as quoted environment variables. This also works and is shorter: -exec sh -c 'test -d "$0"/.svn || test -d "$0"/.git || test -d "$0"/CVS' {} \; Would that be OK? Or is it too wierd to use $0 that way? > Thanks for the reminder - I had forgotten about that. > Personally I never use `test X -o Y` ... because of the ambiguity mentioned > there. > > Still, I think the find manual should first use the triple -exec example as > today, > and then mention that this special case can be tuned by using the above > '-exec sh -c ...'. > > I believe documenting both ways is good because the tests for certain child > names is > just a special case which can be done by the test utility - while other > real-life > examples would need to perform other checks. > > Have a nice day, > Berny OK. I'll submit another version of the patch that does that. But which version of the single -exec command would be best: -exec sh -c 'test -d "$1"/.svn || test -d "$1"/.git || test -d "$1"/CVS' fnord {} \; -exec sh -c '[ -d "$1"/.svn ] || [ -d "$1"/.git ] || [ -d "$1"/CVS ]' fnord {} \; -exec sh -c 'test -d "$0"/.svn || test -d "$0"/.git || test -d "$0"/CVS' {} \; -exec sh -c '[ -d "$0"/.svn ] || [ -d "$0"/.git ] || [ -d "$0"/CVS ]' {} \; I don't like including "fnord", because I feel many readers won't immediately understand why it's there, and so it should be accompanied by an explanation of why it's there, but including such an explanation is not desirable because it has nothing to do with the point of the example which is to demonstrate -prune as an efficiency measure, not to demonstrate sh -c. But I'll use whichever version you prefer. cheers, raf