[GRASS-dev] GRASS 7 development started

[Paul Kelly]

On Mon, 28 Apr 2008, Glynn Clements wrote:

>
> Markus Neteler wrote:
>
>> Today the development of GRASS 6.4 has been moved out into
>> an own development branch. SVN trunk is now dedicated to GRASS 7
>> development which includes major refactoring of the code and
>> improvements to data formats etc.
>
> Can we finalise the coding style, so that we can apply it?
>
> This should be done before we start making substantial changes. Once
> the code has been re-formatted, it will become much harder to compare
> "before" and "after" versions, as any such diff will be dominated by
> the formatting changes.
>
> My personal preference is:
>
> indent -bad -bap -bbb -bl -bli0 -bls -cli0 -cs -fc1 -hnl -i4 -l79 \
>       -nbbo -nbc -nbfda -nbfde -ncdb -ncdw -nce -nfca -npcs -nprs \
>       -npsl -nsc -nsob -saf -sai -saw -sbi0 -ss -ts8 -ut
>
> Descriptions of the above options are given below, although it may be
> more clear to just apply the above command to a bunch of files and
> look at them.

I did that, and was pleasantly surprised to see that some C source files I 
had written hadn't changed much at all. Have just a couple of 
comments/queries that I will mention below.

> Others will presumably have different preferences; in which case
> please express them sooner rather than later.
>
> The precise style doesn't really matter[1]; what DOES matter is that
> we have one style for all of GRASS, and that everyone abides by it[2].
>
> [1] Except for -ts8, which isn't a stylistic choice but a file format
> issue. Using something other than -ts8 is irrational; the only other
> sane alternative is to just forbid tabs outright (-nut, --no-tabs).

Allowing no tabs at all wouldn't be all that bad IMHO either. FFmpeg does 
it and it makes it very simple to see if patches submitted there comply 
with the whitespace formatting conventions - if there are any tabs at all, 
it's rejected. My main query is whether code that contains mixed tabs and 
spaces is "illegal" - I suspect it should be - and if all occurences of 8 
consecutive space characters are required to be converted to a tab?

I find sometimes after a lot of editing of a piece of code and adjusting 
the indenting, that the occurence of spaces and tabs becomes quite mixed 
(generally as a result of me not knowing how to set indentation settings 
in my editor). In this case I think not allowing tabs and just spaces 
makes it easier to fix things up again manually.

> This is particularly important for Python code, where indentation is
> syntatic, not just stylistic. I would suggest using "python -t -t ..."
> to catch this.
>
> [2] The main advantage of 7.x is the ability to make incompatible API
> changes. Any such changes will typically require changing files across
> large numbers of modules. Expecting developers to modify their text
> editor's formatting settings from one file to the next (because
> different files have different formatting) is unacceptable, IMHO.
>
> Descriptions of the above options:
>
[...]
>
>       -cs, --space-after-cast
>           Put a space after a cast operator.

When looking at the reformatted code this one jumped out at me as not 
being the usual way I would write it. I think in my head the idea is a 
cast normally does not actually modify the meaning of data the way another 
operator might, and thus putting it right up against the value being cast 
(without a space) somehow conveys that better. That's all.

Another strange thing I noticed in the reformatted code (I'm not sure 
which option causes it), is that the following code snippet
---------------------
                 if (totalfonts >= maxfonts)
                 {
                     maxfonts += 20;
                     fontcap = (struct GFONT_CAP *) G_realloc(fontcap,
                                          maxfonts * sizeof(struct GFONT_CAP));
                 }
---------------------
became this:
---------------------
                 if (totalfonts >= maxfonts)
                 {
                     maxfonts += 20;
                     fontcap = (struct GFONT_CAP *) G_realloc(fontcap,
                                                              maxfonts *
                                                              sizeof(struct
                                                                     GFONT_CAP));
                 }
---------------------

In the original example, when writing the code, I had moved the start of 
the continuation line back to avoid something ugly happening like in the 
indented code. I think the original version is easier to read though.

And that's about it for my preferences.

Paul