[postgis-devel] Code formatting digging in

Fri Feb 23 02:11:01 PST 2018

I've made a set of PRs to have a look at what astyle and clang-format
passes will look like.

 - astyle: https://github.com/postgis/postgis/pull/218/
 - clang-format: https://github.com/postgis/postgis/pull/219
 - a pass of clang-format on top of a pass of astyle:
https://github.com/Komzpa/postgis/pull/3/files
 - clang-format with PointerAlignment: Right:
https://github.com/postgis/postgis/pull/220/files
 - a pass of clang-format with PointerAlignment: Right on top of a pass of
astyle:
https://github.com/Komzpa/postgis/pull/4/files

TLDR: clang-format is much eyecandier.

1. astyle currently doesn't pass CI, clang-format does.
2. astyle doesn't do anything on line length:
https://github.com/Komzpa/postgis/pull/3/files#diff-cfb8f8761c5379a23860242267edde72L93
 - clang-format wraps long function calls, long comments and long "text
strings"
3. astyle doesn't attempt at fixing typography, so there are spaces before
`;` and `)`
4. clang-format aligns blocks of comments between different lines:
https://github.com/Komzpa/postgis/pull/3/files#diff-398fded035024e0cd5d922b19ada5c68L884

5. clang-format can get rid of weird newlines
https://github.com/Komzpa/postgis/pull/3/files#diff-1fe4151e8b2138fc826e601c61a56eb7L550

https://github.com/Komzpa/postgis/pull/3/files#diff-1fe4151e8b2138fc826e601c61a56eb7L607

(it looks like someone did s/;/;\n/g to parts of code at some point)
6. clang-format deals with #define's internals
https://github.com/Komzpa/postgis/pull/3/files#diff-16bdbc1af78b0ac623fbd5f0d4b0ad79L138

7. clang-format deals with function return type
https://github.com/Komzpa/postgis/pull/3/files#diff-1cd5ee9588f4b7c9ec3b06432a82979dL39
8. clang-format deals with pointer alignment
https://github.com/Komzpa/postgis/pull/3/files#diff-398fded035024e0cd5d922b19ada5c68L42

We have all possible kinds in codebase ("char* i", "LWGEOM *geom",
"GEOSGeometry * g").
I've initially made a pass with DerivePointerAlignment and it decided that
most popular one in codebase is Left. For your convenience I built both
versions, as it was questioned by mloskot.

About syncing clang-format versions:

We can vendor a clang-format binary in a nearby repo, just like
https://github.com/angular/clang-format

I've seen that debbie has commit access to postgis repo. How about asking
her to regularly (on commit, or on release for starters) run clang-format
and commit, possibly duplicating previous commit header so that people who
don't know `git diff -w`/`git blame -w` can still find the time, date and
commit when change happened?

пт, 23 февр. 2018 г. в 1:26, Tomas Vondra <tomas.vondra at 2ndquadrant.com>:

> On 02/22/2018 10:53 PM, Sandro Santilli wrote:
> > On Thu, Feb 22, 2018 at 09:20:31PM +0100, Tomas Vondra wrote:
> >
> >> Considering how tightly is PostGIS coupled with PostgreSQL, I think it
> >> would make perfect sense to use the same code formatting.
> >
> > You can currently build PostGIS by just installing
> > postgresql-server-dev package (for Debian and derivates)
> > I'm not sure it does include the formatter ?
>
> AFAIK that only includes header files, so no - pgindent is not there.
> But it's not like it's a massive tool - you can get it here:
>
>     https://github.com/postgres/postgres/tree/master/src/tools/pgindent
>
> and include it in PostGIS. Or to be more precise, it's a separate tool
> available here:
>
>     https://git.postgresql.org/git/pg_bsd_indent.git
>
> and the thing included in the main git repository is a wrapper,
> configuration, etc.
>
> > Also, could it not still change between releases of PostgreSQL ?
> >
>
> Not really, it tends to be very stable. One of the primary reasons why
> we have pgindent is to make all the branches more alike, to make
> back-patching simpler. Changing the formatting rules between releases
> would directly contradict that.
>
> regards
>
> --
> Tomas Vondra                  http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> _______________________________________________
> postgis-devel mailing list
> postgis-devel at lists.osgeo.org
> https://lists.osgeo.org/mailman/listinfo/postgis-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/postgis-devel/attachments/20180223/3d107dcb/attachment-0001.html>