A few pgindent oddities

Tom Lane wrote:

Since we were breaking our usual rule by re-indenting the 8.1 branch,
I took the time to eyeball the whole "cvs diff" for changes that weren't
just comment block fixes. I found a few things that need attention.

This change is disturbing first because it seems completely unnecessary,
and second because I'm not convinced that every C preprocessor will deal
correctly with a comment continued off a #endif line:

I don't understand why this first problem happened. Alvaro and I talked
about it but I could not determine the cause. I did not want to modify
the indent code at this stage just to fix it. I will look for a fix
later.

Index: contrib/pgbench/pgbench.c
***************
*** 1110,1116 ****
fprintf(stderr, "Use limit/ulimt to increase the limit before using pgbench.\n");
exit(1);
}
! #endif   /* #if !(defined(__CYGWIN__) || defined(__MINGW32__)) */
break;
case 'C':
is_connect = 1;
--- 1110,1117 ----
fprintf(stderr, "Use limit/ulimt to increase the limit before using pgbench.\n");
exit(1);
}
! #endif   /* #if !(defined(__CYGWIN__) ||
!                                  * defined(__MINGW32__)) */
break;
case 'C':
is_connect = 1;

This change seems odd and unnecessary as well:

This is caused by this part of pgindent:

# workaround for indent bug with 'else' handling
# indent comment so BSD indent will move it
sed 's;\([} ]\)else[ ]*\(/\*.*\)$;\1else\
\2;g' |

The problem is that BSD indent crashes on:

else /* test */
{

Not sure why, but I guess I can fix it some day. :-)

Index: src/backend/access/transam/slru.c
***************
*** 252,258 ****
/* indeed, the I/O must have failed */
if (shared->page_status[slotno] == SLRU_PAGE_READ_IN_PROGRESS)
shared->page_status[slotno] = SLRU_PAGE_EMPTY;
!             else                /* write_in_progress */
{
shared->page_status[slotno] = SLRU_PAGE_VALID;
shared->page_dirty[slotno] = true;
--- 253,260 ----
/* indeed, the I/O must have failed */
if (shared->page_status[slotno] == SLRU_PAGE_READ_IN_PROGRESS)
shared->page_status[slotno] = SLRU_PAGE_EMPTY;
!             else
!                 /* write_in_progress */
{
shared->page_status[slotno] = SLRU_PAGE_VALID;
shared->page_dirty[slotno] = true;

And what happened here?

I saw this one to and was stumped at the cause. We have other 'typedef
enum' lines in the code which were not mangled, just this one. Again,
needs research.

Index: src/interfaces/libpq/libpq-fe.h
***************
*** 35,41 ****

/* Application-visible enum types */

! typedef enum
{
/*
* Although it is okay to add to this list, values which become unused
--- 35,41 ----

/* Application-visible enum types */

! typedef enum
{
/*
* Although it is okay to add to this list, values which become unused

regards, tom lane

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:

And what happened here?

I saw this one to and was stumped at the cause. We have other 'typedef
enum' lines in the code which were not mangled, just this one. Again,
needs research.

Index: src/interfaces/libpq/libpq-fe.h
***************
*** 35,41 ****

/* Application-visible enum types */

! typedef enum
{
/*
* Although it is okay to add to this list, values which become unused
--- 35,41 ----

/* Application-visible enum types */

! typedef enum
{
/*
* Although it is okay to add to this list, values which become unused

Ah the cause of this one is this at the top of src/interfaces/libpq/libpq-fe.h:

#ifdef __cplusplus
extern "C"
{
#endif

Not sure I can blame pgindent. Of course the fact that the other
'typedef enum' lines in the file are not indented isn't consistent.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:

Tom Lane wrote:

Since we were breaking our usual rule by re-indenting the 8.1 branch,
I took the time to eyeball the whole "cvs diff" for changes that weren't
just comment block fixes. I found a few things that need attention.

This change is disturbing first because it seems completely unnecessary,
and second because I'm not convinced that every C preprocessor will deal
correctly with a comment continued off a #endif line:

I don't understand why this first problem happened. Alvaro and I talked
about it but I could not determine the cause. I did not want to modify
the indent code at this stage just to fix it. I will look for a fix
later.

I removed the comment. Let's see if we hit it again.

Index: contrib/pgbench/pgbench.c
***************
*** 1110,1116 ****
fprintf(stderr, "Use limit/ulimt to increase the limit before using pgbench.\n");
exit(1);
}
! #endif   /* #if !(defined(__CYGWIN__) || defined(__MINGW32__)) */
break;
case 'C':
is_connect = 1;
--- 1110,1117 ----
fprintf(stderr, "Use limit/ulimt to increase the limit before using pgbench.\n");
exit(1);
}
! #endif   /* #if !(defined(__CYGWIN__) ||
!                                  * defined(__MINGW32__)) */
break;
case 'C':
is_connect = 1;

This change seems odd and unnecessary as well:

I saw this one to and was stumped at the cause. We have other 'typedef
enum' lines in the code which were not mangled, just this one. Again,
needs research.

I fixed this one by hacking pgindent script to left-justify all typedefs
in that file only.

Index: src/interfaces/libpq/libpq-fe.h
***************
*** 35,41 ****

/* Application-visible enum types */

! typedef enum
{
/*
* Although it is okay to add to this list, values which become unused
--- 35,41 ----

/* Application-visible enum types */

! typedef enum
{
/*
* Although it is okay to add to this list, values which become unused

regards, tom lane

-- 
Bruce Momjian                        |  http://candle.pha.pa.us
pgman@candle.pha.pa.us               |  (610) 359-1001
+  If your life is a hard drive,     |  13 Roberts Road
+  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073