Re: patch to diff manpage - clarify "-x"

[Tomas Pospisek]

On Wed, 23 Oct 2002, Paul Eggert wrote:

> > From: Stepan Kasal <address@hidden>
> > Date: Thu, 24 Oct 2002 08:27:33 +0200
>
> > -Exclude files that match PAT.
> > +When comparing directories, ignore files matching PAT.
> >
> > -Exclude files that match any pattern in FILE.
> > +Likewise, patterns are read from FILE.
> > +Likewise, patterns are taken from FILE.
>
> Sorry, I don't like that wording, since it loses the idea that a file
> name is excluded if it matches _any_ pattern taken from FILE.
>
> On further thought, I'm not sure that it's necessary to mention
> directories at all.  in this context.  That block of help looks like
> the following:
>
>   -r  --recursive  Recursively compare any subdirectories found.
>   -N  --new-file  Treat absent files as empty.
>   --unidirectional-new-file  Treat absent first files as empty.
>   -s  --report-identical-files  Report when two files are the same.
>   -x PAT  --exclude=PAT  Exclude files that match PAT.
>   -X FILE  --exclude-from=FILE  Exclude files that match any pattern in FILE.
>   -S FILE  --starting-file=FILE  Start with FILE when comparing directories.
>   --from-file=FILE1  Compare FILE1 to all operands.  FILE1 can be a directory.
>   --to-file=FILE2  Compare all operands to FILE2.  FILE2 can be a directory.
>
> and all the options here relate to recursive comparison, one way or
> another.  So the context is directories already, to some extent.
>
> Also, the diff --help output is already way too long.  I had been
> thinking of shortening it, not lengthening it.

I agree with you there.

But wheter you will or not shorten diff --help, the man page should
contain a description of all the options. And that page should,
contrary to --help, not favour shortnes over a explicit and thatfor maybe
more verbose description. If that's correct than there should be no
problem patching the manpage - the patch I provided should be fine I
imagine.

In case you just want a fix for --help without shortening it:

address@hidden:src$ diff -u diff.c.orig diff.c
--- diff.c.orig 2002-10-24 11:41:03.000000000 +0200
+++ diff.c      2002-10-24 15:10:48.000000000 +0200
@@ -891,8 +896,8 @@
   N_("-N  --new-file  Treat absent files as empty."),
   N_("--unidirectional-new-file  Treat absent first files as empty."),
   N_("-s  --report-identical-files  Report when two files are the same."),
-  N_("-x PAT  --exclude=PAT  Exclude files that match PAT."),
-  N_("-X FILE  --exclude-from=FILE  Exclude files that match any pattern in 
FILE."),
+  N_("-x PAT  --exclude=PAT  Exclude files that match PAT in directories."),
+  N_("-X FILE  --exclude-from=FILE  Likewise files matching any pattern in 
FILE."),
   N_("-S FILE  --starting-file=FILE  Start with FILE when comparing 
directories."),
   N_("--from-file=FILE1  Compare FILE1 to all operands.  FILE1 can be a 
directory."),
   N_("--to-file=FILE2  Compare all operands to FILE2.  FILE2 can be a 
directory."),

The one fix I favour is:

address@hidden:src$ diff -u diff.c.orig diff.c
--- diff.c.orig 2002-10-24 11:41:03.000000000 +0200
+++ diff.c      2002-10-24 15:10:48.000000000 +0200
@@ -98,6 +98,9 @@
 static bool report_identical_files;


+/* We're excluding things using -x or -X */
+static bool excluding = 0;
+
 /* Return a string containing the command options with which diff was
invoked.
    Spaces appear between what were separate ARGV-elements.
    There is a space at the beginning but none at the end.
@@ -479,10 +482,12 @@
          break;

        case 'x':
+      excluding = 1;
          add_exclude (excluded, optarg, exclude_options ());
          break;

        case 'X':
+      excluding = 1;
          if (add_exclude_file (add_exclude, excluded, optarg,
                                exclude_options (), '\n'))
            pfatal_with_name (optarg);
@@ -891,8 +896,8 @@
   N_("-N  --new-file  Treat absent files as empty."),
   N_("--unidirectional-new-file  Treat absent first files as empty."),
   N_("-s  --report-identical-files  Report when two files are the same."),
-  N_("-x PAT  --exclude=PAT  Exclude files that match PAT."),
-  N_("-X FILE  --exclude-from=FILE  Exclude files that match any pattern in 
FILE."),
+  N_("-x PAT  --exclude=PAT  Exclude files that match PAT in directories."),
+  N_("-X FILE  --exclude-from=FILE  Likewise files matching any pattern in 
FILE."),
   N_("-S FILE  --starting-file=FILE  Start with FILE when comparing 
directories."),
   N_("--from-file=FILE1  Compare FILE1 to all operands.  FILE1 can be a 
directory."),
   N_("--to-file=FILE2  Compare all operands to FILE2.  FILE2 can be a 
directory."),
@@ -1109,6 +1114,14 @@
        = dir_file_pathname (parent->file[1].name, name1);
     }

+  /* Complain if we're excluding things and any one of the files is a
+   * directory */
+  printf("excluding: %d - file0: %s (%d), file1: %s (%d)\n",
+         excluding, name0, DIR_P(0), name1, DIR_P(1) );
+  if (excluding && ! (DIR_P(0) || DIR_P(1))) {
+    try_help ("-x or -X can only be used when comparing directories",
"");
+  }
+
   /* Stat the files.  */

   for (f = 0; f < 2; f++)


I think that the fix might be in principle correct. The problem with that
fix is that it's not working for links to directories. There might be
other problems I'm not aware of since I'm neither a diff expert nor have I
studied the code in depth.

So I'd favour that approach, but I don't know the code enough to give you
a nice and clean version that dereferences properly links or whatever else
is needed.

Stephan Kasal (nazdar Stepane!) wrote:

> I guess that the confusion came from this:
>
> diff -s file file               # works
> diff -x '*_x' file_x file2_x    # doesn't
> diff -x '*_x' file_x file_y     # doesn't

The confusion came from the fact that I wanted to test my pattern before
letting it rattle through my entire filesystem. So I started off with:

$ diff -s bla bla.txt bla.tx

which didn't work, so I guessed it might be some
diff-reg-expression-dialect problem (such dialects do exist, right?) I
kept on trying and finally I gave up after reading the manpage thinking it
was a bug in diff.

*t

--
--------------------------------------------------------
     Tomas Pospisek
     sourcepole    -   Linux & Open Source Solutions
     http://sourcepole.com
     Elestastrasse 18,  7310 Bad Ragaz,  Switzerland
     Tel:+41 (81) 330 77 13,  Fax:+41 (81) 330 77 12
--------------------------------------------------------