This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 1/6 v3] Reformat libio files
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: OndÅej BÃlka <neleai at seznam dot cz>
- Cc: Richard Henderson <rth at twiddle dot net>, <libc-alpha at sourceware dot org>
- Date: Wed, 5 Jun 2013 12:30:54 +0000
- Subject: Re: [PATCH 1/6 v3] Reformat libio files
- References: <1370099488-13916-1-git-send-email-neleai at seznam dot cz> <1370099488-13916-2-git-send-email-neleai at seznam dot cz> <51ACAD3E dot 7000409 at twiddle dot net> <20130603161259 dot GA13206 at domone dot kolej dot mff dot cuni dot cz> <Pine dot LNX dot 4 dot 64 dot 1306031642110 dot 18596 at digraph dot polyomino dot org dot uk> <20130605094542 dot GB8824 at domone dot kolej dot mff dot cuni dot cz>
On Wed, 5 Jun 2013, Ondrej Bilka wrote:
> I disabled cleanups and now do formating only.
>
> These cleanups do not do formating and you cannot avoid doing formating
> before or after cleanup and in both cases its somewhat messy.
In certain cases, *local* formatting changes may be needed for a cleanup,
e.g. reindenting subsequent lines of a call - in which case the cleanup
patch should include just those local changes needed, regardless of the
human/tool combination used to produce the patch. Not global formatting
changes unrelated to the cleanup in question.
> Just do not complain that formating of pre ISO C function definitions is
> wrong and I should rewrite them to ISO C format.
That's not my complaint. Rather:
* Formatting changes should only be made where there is a consensus that
the new formatting is the correct standard for glibc.
* For your pre-ISO-C function definitions, you are making a change where
the consensus is that the change is *incorrect*.
A formatting patch should never change something correctly formatting to
be incorrect, or change something where both before and after are correct
absent a consensus for a change.
If parts of formatting changes or cleanups are uncontroversial and parts
are more controversial, those should be split into separate patch
submissions.
> > That appears to be putting the backslash further to the right than Emacs
> > C-C C-\ does (in most cases I think of Emacs formatting as a good guide if
> > the GNU Coding Standards don't specify some detail).
>
> What does this do? a C-C C-\ is undefined at emacs that I installed. Is
> possible to make it standalone?
C-C C-\, in C mode, puts backslashes at ends of lines in the selected
region.
> A program cannot guess what alternative is selected so it is better to
> stick with one.
If in doubt, you can simply avoid changing existing macros using
backslash-newline unless actually changing the contents of the macro
definition, and just keep the backslashes in the same columns as before
when editing the contents of an existing such macro.
> > > - ; /* Ignore error from unseekable devices. */
> > > + ; /* Ignore error from unseekable devices. */
> >
> > What rule do you think there is for indentation of comments after code
> > that is followed in glibc (or generally in the GNU system)? I wasn't
> > aware of one, and without consensus on one, existing code shouldn't be
> > changed like this; formatting changes should be when code clearly deviates
> > from the established desired style.
> >
> Again uncrustify does this. I do not know how turn it specially off. As
> it is minor I would accept this noise.
You should aim to produce patches that do a single cleanup (all changes
disabled by default, one cleanup enabled) in preference to doing a whole
load of changes, some lacking consensus, and then trying to disable them
selectively.
I don't know whether uncrustify is a suitable tool for this at all. But
any combination of tool and human used needs to respect consensus.
--
Joseph S. Myers
joseph@codesourcery.com