Re: [PATCH] mkdir/split: send --verbose output to stdout + [PATCH]: same for rmdir/install

[Jim Meyering]

Ondřej Vašík <address@hidden> wrote:
> Hello,
> I think there are still some things to be consolidated.
> The same with verbose output should be done in the case
> of install(when making dir) and in the case of rmdir.
> In the case of shred should be mentioned in info that
> verbose output is to standard error.
> Those things done in attached patch, should break
> no tests (as rmdir has no verbose test and install
> verbose test redirects stdout and stderr to the one
> file)

Good catch.
Thank you for working on that.

It'd be great if you would take the time to prepare
a "perfect" patch, so that I can simply apply it with
"git am YOUR_PATCH".  In case you're interested, here are
some of the things you can do.

While your patch applies fine with "patch", git (as configured for me
locally) objects to the trailing blanks you've added.  And coreutils'
own "make syntax-check" rule would complain about them, too.  Here's what
happens when I try to commit your changes onto a local "topic" branch:

  $ git commit -m vasic-orig -a
  *
  * You have some suspicious patch lines:
  *
  * In ChangeLog-2008
  * trailing whitespace (line 6)
  ChangeLog-2008:6:       * src/install.c (announce_mkdir): write to stdout, 
not stderr
  * In NEWS
  * trailing whitespace (line 38)
  NEWS:38:  mkdir, split, install and rmdir now write --verbose output to 
stdout,
  * In doc/coreutils.texi
  * trailing whitespace (line 8193)
  doc/coreutils.texi:8193:Display to standard error all status updates as 
sterilization proceeds.
  [Exit 1]

Of course, I could deal with that very quickly, but it's not a productive
use of my time.  What I really need is a set of contributor guidelines
I can point to saying this sort of thing.  So I suppose this is
a first cut at that.

In a few cases, you have not followed the coding conventions:

-       error (0, 0, _("removing directory, %s"), dir);
+       printf(_("%s: removing directory, %s\n"), program_name, dir);

Note the space between the function name and the open parenthesis.
That should be "printf (...".
This is the style produced by "GNU indent" in its default mode.
However, please do not reformat any file wholesale, as that would
inevitably (and unfortunately) induce many whitespace-only changes.

BTW, that particular change made me see that "dir" should be quoted,
like the preceding one.  I.e., s/dir/quote (dir)/

Also, I'd rather not use printf, since doing so would
involve some small regressions: adding a "program_name" parameter
and changing the diagnostic (not good for translators).
Also, adding the "\n" is unusual.  Too easy to miss.

Instead, it should use the same function that was added for
mkdir's --verbose mode.  Since that will mean using that
function from three separate programs, it will go into its
own file and have a corresponding .h file, both new files in src/.
However the existing name, verbose_output is too specific
for what the function does.  Names are important.  Please
come up with a better one.  Suggestions from the gallery are
welcome :-)

So you'll have to make the three .c files include the new .h,
and you'll have to adjust src/Makefile.am to link the new .c
file to each of those three programs.

---------------------
Please don't change ChangeLog-2008 or any other ChangeLog file in
coreutils.  As of a month or so ago, the ChangeLogs are solely in the
"git" log.  They are also generated automatically into a top-level
ChangeLog file at "make dist" time.

Instead, please commit the proposed change into your
local git repository with a comment of this form:
(without the ========= lines, of course)

=======================================================
one-line summary of your change

* src/file1.c (): ...
* doc/whatever (): ...
* NEWS: ...
=======================================================

Note that other than the first line, lines 3..end
should follow the GCS ChangeLog guidelines.
It's best to keep their length < 72, since the
log-to-changelog generator prepends a TAB to each line
and doesn't (yet?) wrap long lines.

Then, create the file "patch" like this:

  git format-patch --stdout --signoff HEAD~1 > patch

and mail that file to the list.