Re: Acceptable/Best formatting of callbacks (for pluggable storage)

On 2019-Jan-11, Andres Freund wrote:

> Just as an example of why I think this isn't great:

Hmm, to me, the first example is much better because of *vertical* space
-- I can have the whole API in one screenful. In the other example, the
same number of functions take many more lines. The fact that the
arguments are indented differently doesn't bother me.

> typedef Size (*EstimateDSMForeignScan_function) (ForeignScanState *node,
> ParallelContext *pcxt);
> typedef void (*InitializeDSMForeignScan_function) (ForeignScanState *node,
> ParallelContext *pcxt,
> void *coordinate);
> typedef void (*ReInitializeDSMForeignScan_function) (ForeignScanState *node,
> ParallelContext *pcxt,
> void *coordinate);
> typedef void (*InitializeWorkerForeignScan_function) (ForeignScanState *node,
> shm_toc *toc,
> void *coordinate);
> typedef void (*ShutdownForeignScan_function) (ForeignScanState *node);
> typedef bool (*IsForeignScanParallelSafe_function) (PlannerInfo *root,
> RelOptInfo *rel,
> RangeTblEntry *rte);
> typedef List *(*ReparameterizeForeignPathByChild_function) (PlannerInfo *root,
> List *fdw_private,
> RelOptInfo *child_rel);
>
> that's a lot of indentation variability in a small amount of space - I
> find it noticably slower to mentally parse due to that. Compare that
> with:
>
> typedef Size (*EstimateDSMForeignScan_function)
> (ForeignScanState *node,
> ParallelContext *pcxt);
>
> typedef void (*InitializeDSMForeignScan_function)
> (ParallelContext *pcxt,
> void *coordinate);
>
> typedef void (*ReInitializeDSMForeignScan_function)
> (ForeignScanState *node,
> ParallelContext *pcxt,
> void *coordinate);
>
> typedef void (*InitializeWorkerForeignScan_function)
> (ForeignScanState *node,
> shm_toc *toc,
> void *coordinate);
>
> typedef void (*ShutdownForeignScan_function)
> (ForeignScanState *node);
>
> typedef bool (*IsForeignScanParallelSafe_function)
> (PlannerInfo *root,
> RelOptInfo *rel,
> RangeTblEntry *rte);
>
> typedef List *(*ReparameterizeForeignPathByChild_function)
> (PlannerInfo *root,
> List *fdw_private,
> RelOptInfo *child_rel);
>
> I find the second formatting considerably easier to read, albeit not for
> the first few seconds.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services