Re: bugs in dirname module - coreutils portion

[Eric Blake]

Paul wrote:
> I took a very quick look and have one other comment:
> 
> -                        components of the specified directories\n\
> +                     components of the specified directories\n\
> 
> This isn't right: those should be leading spaces, without any tabs.

Agreed, since it is output to the terminal where you can't trust the
user to have sane tab stops.

> Perhaps you put the source code through a tab normalizer?  If so,
> please undo all changes of that sort.

I normally run emacs with
 (add-hook 'before-save-hook 'whitespace-cleanup)
That was the culprit.  I guess I should file a bug with emacs that
whitespace.el should not normalize spaces inside string literals.

Meanwhile, is there an easier way to run emacs whitespace-cleanup
to catch trailing whitespace and space-before-tab issues without
also being forced to turn on the 8-spaces-vs-tab cleanup?  What
settings do you use for whitespace happiness during editing?

> 
> It might be better, for a larger change like this, to do the
> tab-normalization changes separately anyway, so that we can see the
> more-important stuff easily.

I'll regenerate with diff -b, to mask the whitespace changes
(I don't have commit rights yet, so someone else will end up
applying my final patch anyway).

Jim wrote:
> Also, a style issue:
> 
> +  while (ISSLASH (*base))
> +    base++;
> +  for (p = base; *p; p++)
> +    if (ISSLASH (*p))
> +      saw_slash = true;
> +    else if (saw_slash)
> +      {
> +     base = p;
> +     saw_slash = false;
> +      }
> 
> Please do not omit braces unless the block they would
> enclose consists of a single line -- adding a blank
> line between the loops is nice, too.

I was debating about that before submitting my patch, and
settled on the above based on the fact that gcc didn't warn,
and based on my interpretation of the GNU coding standards
(http://www.gnu.org/prep/standards/standards.html#Formatting)
as being that the braces should be omitted for ALL one-statement
bodies, simple or compound, except when it might lead to
dangling-else confusion (and thus -Wall noise).  But I can agree
with your interpretation of omitting the braces only for a one-liner
and including it for one-statement but multi-line compound
bodies.  I will fix that hunk to your recommended style when I
repost.

That also means that my gnulib patch has the same style issues;
I will have to make sure that, once commits actually take place, both
versions have the same formatting style in the files my patch touched.

--
Eric Blake