use of pager on Windows psql

Started by Andrew Dunstanover 17 years ago7 messages

andrew@dunslane.net

over 17 years ago

psql's print.c contains this piece of code:

/*
* PageOutput
*
* Tests if pager is needed and returns appropriate FILE pointer.
*/
FILE *
PageOutput(int lines, unsigned short int pager)
{
/* check whether we need / can / are supposed to use pager */
if (pager
#ifndef WIN32
&&
isatty(fileno(stdin)) &&
isatty(fileno(stdout))
#endif
)
{

Why are we not doing the isatty tests on Windows? We can and do use
isatty on Windows elsewhere, so I'm a bit mystified about this.

In fact, it looks to me like it would be much more sensible to #include
"settings.h" and then simply test pset.notty for all platforms.

cheers

andrew

Bruce Momjian

bruce@momjian.us

over 17 years ago

In reply to: Andrew Dunstan (#1)

Re: use of pager on Windows psql

Andrew Dunstan wrote:

psql's print.c contains this piece of code:

/*
* PageOutput
*
* Tests if pager is needed and returns appropriate FILE pointer.
*/
FILE *
PageOutput(int lines, unsigned short int pager)
{
/* check whether we need / can / are supposed to use pager */
if (pager
#ifndef WIN32
&&
isatty(fileno(stdin)) &&
isatty(fileno(stdout))
#endif
)
{

Why are we not doing the isatty tests on Windows? We can and do use
isatty on Windows elsewhere, so I'm a bit mystified about this.

Not sure why ware are not. Should we enabled that code on Win32 and see
how it works? Can you test it? Was it some MinGW limitation? I do see
isatty() being used on lots of platforms.

This is kind of odd. Ah, I bet it came from libpq's PQprint(), which I
think we had working on Win32 long before we had psql working and
perhaps I copied it from there. I don't see the Win32 checks around
isatty() anywhere else.

In fact, it looks to me like it would be much more sensible to #include
"settings.h" and then simply test pset.notty for all platforms.

Yes, we could do that but does the isatty() value ever change while psql
is running? When you do '\g filename' does stdout then have isatty as
false?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Andrew Dunstan

andrew@dunslane.net

over 17 years ago

In reply to: Bruce Momjian (#2)

Re: use of pager on Windows psql

Bruce Momjian wrote:

Andrew Dunstan wrote:

psql's print.c contains this piece of code:

/*
* PageOutput
*
* Tests if pager is needed and returns appropriate FILE pointer.
*/
FILE *
PageOutput(int lines, unsigned short int pager)
{
/* check whether we need / can / are supposed to use pager */
if (pager
#ifndef WIN32
&&
isatty(fileno(stdin)) &&
isatty(fileno(stdout))
#endif
)
{

Why are we not doing the isatty tests on Windows? We can and do use
isatty on Windows elsewhere, so I'm a bit mystified about this.

Not sure why ware are not. Should we enabled that code on Win32 and see
how it works? Can you test it? Was it some MinGW limitation? I do see
isatty() being used on lots of platforms.

This is kind of odd. Ah, I bet it came from libpq's PQprint(), which I
think we had working on Win32 long before we had psql working and
perhaps I copied it from there. I don't see the Win32 checks around
isatty() anywhere else.

In fact, it looks to me like it would be much more sensible to #include
"settings.h" and then simply test pset.notty for all platforms.

Yes, we could do that but does the isatty() value ever change while psql
is running? When you do '\g filename' does stdout then have isatty as
false?

Good point. I think the best thing would just be to remove the #ifndef
WIN32 / #endif lines

cheers

andrew

Bruce Momjian

bruce@momjian.us

over 17 years ago

In reply to: Andrew Dunstan (#3)

1 attachment(s)

Re: [HACKERS] use of pager on Windows psql

Andrew Dunstan wrote:

Not sure why ware are not. Should we enabled that code on Win32 and see
how it works? Can you test it? Was it some MinGW limitation? I do see
isatty() being used on lots of platforms.

This is kind of odd. Ah, I bet it came from libpq's PQprint(), which I
think we had working on Win32 long before we had psql working and
perhaps I copied it from there. I don't see the Win32 checks around
isatty() anywhere else.

In fact, it looks to me like it would be much more sensible to #include
"settings.h" and then simply test pset.notty for all platforms.

Yes, we could do that but does the isatty() value ever change while psql
is running? When you do '\g filename' does stdout then have isatty as
false?

Good point. I think the best thing would just be to remove the #ifndef
WIN32 / #endif lines

OK, patch applied to remove the Win32 test in both places.

I was wrong about \g filename changing stdout, I think. It keeps stdout
but uses a different output stream. I am just unsure, given all the
features of psql, wether it could change stdin/stdout while running in
some way.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Attachments:

/rtmp/difftext/x-diffDownload

Index: src/bin/psql/print.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/print.c,v
retrieving revision 1.105
diff -c -c -r1.105 print.c
*** src/bin/psql/print.c	17 May 2008 21:40:44 -0000	1.105
--- src/bin/psql/print.c	17 May 2008 23:32:36 -0000
***************
*** 1912,1924 ****
  PageOutput(int lines, unsigned short int pager)
  {
  	/* check whether we need / can / are supposed to use pager */
! 	if (pager
! #ifndef WIN32
! 		&&
! 		isatty(fileno(stdin)) &&
! 		isatty(fileno(stdout))
! #endif
! 		)
  	{
  		const char *pagerprog;
  		FILE	   *pagerpipe;
--- 1912,1918 ----
  PageOutput(int lines, unsigned short int pager)
  {
  	/* check whether we need / can / are supposed to use pager */
! 	if (pager && isatty(fileno(stdin)) && isatty(fileno(stdout)))
  	{
  		const char *pagerprog;
  		FILE	   *pagerpipe;
Index: src/interfaces/libpq/fe-print.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-print.c,v
retrieving revision 1.75
diff -c -c -r1.75 fe-print.c
*** src/interfaces/libpq/fe-print.c	1 Jan 2008 19:46:00 -0000	1.75
--- src/interfaces/libpq/fe-print.c	17 May 2008 23:32:36 -0000
***************
*** 147,159 ****
  
  		if (fout == NULL)
  			fout = stdout;
! 		if (po->pager && fout == stdout
! #ifndef WIN32
! 			&&
! 			isatty(fileno(stdin)) &&
! 			isatty(fileno(stdout))
! #endif
! 			)
  		{
  			/*
  			 * If we think there'll be more than one screen of output, try to
--- 147,154 ----
  
  		if (fout == NULL)
  			fout = stdout;
! 		if (po->pager && fout == stdout && isatty(fileno(stdin)) &&
! 			isatty(fileno(stdout)))
  		{
  			/*
  			 * If we think there'll be more than one screen of output, try to

Andrew Dunstan

andrew@dunslane.net

over 17 years ago

In reply to: Bruce Momjian (#4)

Re: [HACKERS] use of pager on Windows psql

Bruce Momjian wrote:

Andrew Dunstan wrote:

Not sure why ware are not. Should we enabled that code on Win32 and see
how it works? Can you test it? Was it some MinGW limitation? I do see
isatty() being used on lots of platforms.

This is kind of odd. Ah, I bet it came from libpq's PQprint(), which I
think we had working on Win32 long before we had psql working and
perhaps I copied it from there. I don't see the Win32 checks around
isatty() anywhere else.

In fact, it looks to me like it would be much more sensible to #include
"settings.h" and then simply test pset.notty for all platforms.

Yes, we could do that but does the isatty() value ever change while psql
is running? When you do '\g filename' does stdout then have isatty as
false?

Good point. I think the best thing would just be to remove the #ifndef
WIN32 / #endif lines

OK, patch applied to remove the Win32 test in both places.

This broke the buildfarm and finally explains the following kluge which
has been puzzling me for four years:

/*
* for some reason MinGW (and MSVC) outputs an extra newline, so
this
* suppresses it
*/
#ifndef WIN32
fputc('\n', fout);
#endif

I have removed the kluge (and yes, I tested it).

cheers

andrew

Bruce Momjian

bruce@momjian.us

over 17 years ago

In reply to: Andrew Dunstan (#5)

Re: [HACKERS] use of pager on Windows psql

Andrew Dunstan wrote:

In fact, it looks to me like it would be much more sensible to #include
"settings.h" and then simply test pset.notty for all platforms.

Yes, we could do that but does the isatty() value ever change while psql
is running? When you do '\g filename' does stdout then have isatty as
false?

Good point. I think the best thing would just be to remove the #ifndef
WIN32 / #endif lines

OK, patch applied to remove the Win32 test in both places.

This broke the buildfarm and finally explains the following kluge which
has been puzzling me for four years:

/*
* for some reason MinGW (and MSVC) outputs an extra newline, so
this
* suppresses it
*/
#ifndef WIN32
fputc('\n', fout);
#endif

I have removed the kluge (and yes, I tested it).

Oh, that kluge. Why did the isatty() addition fix this? Was the pager
being used on Win32 for the regression tests and somehow eating a line
or something?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Andrew Dunstan

andrew@dunslane.net

over 17 years ago

In reply to: Bruce Momjian (#6)

Re: [HACKERS] use of pager on Windows psql

Bruce Momjian wrote:

This broke the buildfarm and finally explains the following kluge which
has been puzzling me for four years:

/*
* for some reason MinGW (and MSVC) outputs an extra newline, so
this
* suppresses it
*/
#ifndef WIN32
fputc('\n', fout);
#endif

I have removed the kluge (and yes, I tested it).

Oh, that kluge. Why did the isatty() addition fix this? Was the pager
being used on Win32 for the regression tests and somehow eating a line
or something?

It apparently produced an extra line which we had compensated for with
the kluge (without really understanding why we had to).

Anyway, all is good now, as the buildfarm shows.

cheers

andrew