Enforcing code style

[strk]

Follow up to #433

At the moment (r14869) we are _requiring_ astyle 1.23 but Ubuntu 14.04 (2 years ago) ships 2.03. Time to change ?

I've pushed an "astyle" branch to the git repository at ​https://git.osgeo.org/gogs/postgis/postgis.git

The branch was styled with astyle 2.03 on ubuntu 14.04 and the check for astyle version was removed from the astyle.sh script.

Could you please report here the effects of "make astyle" with your version of astyle on your system ? If we can all run astyle in a consistent way we could build some automation for developers to enforce it.

[pramsey]

Huh, not super

On branch astyle
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   extensions/address_standardizer/standard.c
	modified:   extensions/address_standardizer/std_pg_hash.c
	modified:   liblwgeom/lwgeom.c
	modified:   liblwgeom/lwgeom_geos.c
	modified:   liblwgeom/lwhomogenize.c
	modified:   liblwgeom/lwin_twkb.c
	modified:   liblwgeom/lwin_wkb.c
	modified:   liblwgeom/lwlinearreferencing.c
	modified:   liblwgeom/lwout_wkb.c
	modified:   liblwgeom/lwstroke.c
	modified:   loader/dbfopen.c
	modified:   loader/shp2pgsql-gui.c
	modified:   postgis/gserialized_gist_2d.c
	modified:   postgis/lwgeom_backend_api.c
	modified:   postgis/lwgeom_functions_lrs.c
	modified:   postgis/lwgeom_geos.c
	modified:   postgis/lwgeom_in_gml.c
	modified:   postgis/lwgeom_in_kml.c
	modified:   raster/rt_core/rt_mapalgebra.c
	modified:   raster/rt_core/rt_raster.c
	modified:   raster/rt_core/rt_spatial_relationship.c
	modified:   raster/rt_core/rt_statistics.c
	modified:   raster/rt_pg/rtpg_mapalgebra.c
	modified:   topology/postgis_topology.c

no changes added to commit (use "git add" and/or "git commit -a")
Butterfly:~/Code/postgis-git pramsey(astyle)$ astyle --version
Artistic Style Version 2.05.1
Butterfly:~/Code/postgis-git pramsey(astyle)$ 

[pramsey]

I think I like mine better

diff --git a/liblwgeom/lwgeom.c b/liblwgeom/lwgeom.c
index da9dc2a..7461332 100644
--- a/liblwgeom/lwgeom.c
+++ b/liblwgeom/lwgeom.c
@@ -48,8 +48,8 @@ lwgeom_force_clockwise(LWGEOM *lwgeom)
                lwtriangle_force_clockwise((LWTRIANGLE *)lwgeom);
                return;
 
-               /* Not handle POLYHEDRALSURFACE and TIN
-                  as they are supposed to be well oriented */
+       /* Not handle POLYHEDRALSURFACE and TIN
+          as they are supposed to be well oriented */
        case MULTIPOLYGONTYPE:
        case COLLECTIONTYPE:
                coll = (LWCOLLECTION *)lwgeom;

[pramsey]

Except for the handling of enum, which gets worse

diff --git a/postgis/lwgeom_in_kml.c b/postgis/lwgeom_in_kml.c
index cedeb3a..930aaa6 100644
--- a/postgis/lwgeom_in_kml.c
+++ b/postgis/lwgeom_in_kml.c
@@ -209,15 +209,15 @@ static double parse_kml_double(char *d, bool space_before,
        int st;
        enum states
        {
-           INIT        = 0,
-           NEED_DIG    = 1,
-           DIG         = 2,
-           NEED_DIG_DEC        = 3,
-           DIG_DEC     = 4,
-           EXP         = 5,
-           NEED_DIG_EXP        = 6,
-           DIG_EXP     = 7,
-           END                 = 8
+               INIT            = 0,
+               NEED_DIG        = 1,
+               DIG             = 2,
+               NEED_DIG_DEC    = 3,
+               DIG_DEC         = 4,
+               EXP             = 5,
+               NEED_DIG_EXP    = 6,
+               DIG_EXP         = 7,
+               END             = 8
        };

[strk]

In any case, there's still no "stable" output from astyle, so we'd better look for something different if we want to go there.

Maybe we could try the qgis one, which sounds pretty complex: ​https://github.com/qgis/QGIS/blob/master/scripts/astyle.sh

[pramsey]

I think we should just have an astyle pass over the whole code base, and then go back to ruining it for another 5 years.

[strk]

I got confirmation by Jurgen that QGIS basically ships a specific version of astyle (includes its source code within the qgis one).

How about that solution ?

[pramsey]

seems like we're in more of a slow burn with editorconfig than a bulk change

[robe]

Milestone renamed