Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread Michael Paquier
On Tue, Mar 22, 2016 at 6:25 AM, Tom Lane  wrote:
> If I don't hear objections PDQ, I'm going to update the docs and commit
> it like that.

Thanks!
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread Alvaro Herrera
Tom Lane wrote:
> "David G. Johnston"  writes:
> > On Monday, March 21, 2016, Tom Lane  wrote:
> >> What about just discarding the old format entirely, and printing one of
> >> these two things:
> >> 
> >> Timestamp (every Ns)
> >> 
> >> User Given Title  Timestamp (every Ns)
> 
> > This works for me.
> 
> If I don't hear objections PDQ, I'm going to update the docs and commit
> it like that.

It works for me too.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread Tom Lane
"David G. Johnston"  writes:
> On Monday, March 21, 2016, Tom Lane  wrote:
>> What about just discarding the old format entirely, and printing one of
>> these two things:
>> 
>> Timestamp (every Ns)
>> 
>> User Given Title  Timestamp (every Ns)

> This works for me.

If I don't hear objections PDQ, I'm going to update the docs and commit
it like that.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread Tom Lane
"David G. Johnston"  writes:
> I'd rather not omit sleep but removing "Watch every" is fine (preferred
> actually), so:
> Title Is Here Mon Mar 21 15:05:06 2016 (5s)

Meh ... seems a bit awkward to me.  Couldn't you include " (5s)" in the
title, if you want that info?  If it's variable, you could still
accommodate that:

regression=# \set delay 5
regression=# \pset title 'My Title (':delay' s)'
Title is "My Title (5 s)".
regression=# select repeat('xyzzy',12) \watch :delay
   My Title (5 s)   Mon Mar 21 13:39:25 2016

repeat
--
 xyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzyxyzzy
(1 row)

But I don't care enough to veto it.
Anyone else have an opinion?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread David G. Johnston
On Mon, Mar 21, 2016 at 10:14 AM, Tom Lane  wrote:

> Robert Haas  writes:
> > Well, the title isn't normally centered, but yeah, that is odd.  Yeah,
> > that is odd.  Come to think of it, I think I might have expected the
> > title to appear *above* "Watch every %s", not below it.  That might
> > decrease the oddness.
>
> AFAICS, it appears *beside* it with this patch.  It's only below if the
> terminal is narrow enough that it wraps to there.
>
> > As for letting the committer decide, I don't care about this
> > personally at all, so I'm only looking at it to be nice to the people
> > who do.  Whatever is the consensus is OK with me.  I just don't want
> > to get yelled at later for committing something here, so it would be
> > nice to see a few votes for whatever we're gonna do here.
>
> I'm still of the opinion that what would make the most sense is to replace
> the "Watch every Ns" text with the user-given title, if there is one.
> I ran that up the flagpole already and didn't get a lot of salutes, but
> it seems to respond to your concern that the user title ought to be first.
>
> Regardless of that, I concur with your complaints about coding style, in
> particular with the need to repeat the magic constant 50 in several
> places.  Also, I think the patch makes do_watch return the wrong result
> code for the (typical) case where we exit because of query cancel not
> PSQLexecWatch failure.
>
> So on the whole, I'd do it as attached.
>
​
I'd rather not omit sleep but removing "Watch every" is fine (preferred
actually), so:

if (user_title)​
​snprintf(title, title_len, "%s\t%s (%ld​s)", user_title,
asctime(localtime(&timer)), sleep)

"""
Title Is Here Mon Mar 21 15:05:06 2016 (5s)

col1
-
1
"""

David J.


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread Tom Lane
Robert Haas  writes:
> Well, the title isn't normally centered, but yeah, that is odd.  Yeah,
> that is odd.  Come to think of it, I think I might have expected the
> title to appear *above* "Watch every %s", not below it.  That might
> decrease the oddness.

AFAICS, it appears *beside* it with this patch.  It's only below if the
terminal is narrow enough that it wraps to there.

> As for letting the committer decide, I don't care about this
> personally at all, so I'm only looking at it to be nice to the people
> who do.  Whatever is the consensus is OK with me.  I just don't want
> to get yelled at later for committing something here, so it would be
> nice to see a few votes for whatever we're gonna do here.

I'm still of the opinion that what would make the most sense is to replace
the "Watch every Ns" text with the user-given title, if there is one.
I ran that up the flagpole already and didn't get a lot of salutes, but
it seems to respond to your concern that the user title ought to be first.

Regardless of that, I concur with your complaints about coding style, in
particular with the need to repeat the magic constant 50 in several
places.  Also, I think the patch makes do_watch return the wrong result
code for the (typical) case where we exit because of query cancel not
PSQLexecWatch failure.

So on the whole, I'd do it as attached.

regards, tom lane

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index eef6e4b..a309109 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*** static bool
*** 3020,3026 
  do_watch(PQExpBuffer query_buf, long sleep)
  {
  	printQueryOpt myopt = pset.popt;
! 	char		title[50];
  
  	if (!query_buf || query_buf->len <= 0)
  	{
--- 3020,3029 
  do_watch(PQExpBuffer query_buf, long sleep)
  {
  	printQueryOpt myopt = pset.popt;
! 	const char	 *user_title;
! 	char		 *title;
! 	int			  title_len;
! 	int			  res = 0;
  
  	if (!query_buf || query_buf->len <= 0)
  	{
*** do_watch(PQExpBuffer query_buf, long sle
*** 3034,3042 
  	 */
  	myopt.topt.pager = 0;
  
  	for (;;)
  	{
- 		int			res;
  		time_t		timer;
  		long		i;
  
--- 3037,3055 
  	 */
  	myopt.topt.pager = 0;
  
+ 	/*
+ 	 * If there's a title in the user configuration, make sure we have room
+ 	 * for it in the title buffer.
+ 	 */
+ 	user_title = myopt.title;
+ 	if (user_title)
+ 		title_len = strlen(user_title) + 50;
+ 	else
+ 		title_len = 50;
+ 	title = pg_malloc(title_len);
+ 
  	for (;;)
  	{
  		time_t		timer;
  		long		i;
  
*** do_watch(PQExpBuffer query_buf, long sle
*** 3045,3052 
  		 * of completion of the command?
  		 */
  		timer = time(NULL);
! 		snprintf(title, sizeof(title), _("Watch every %lds\t%s"),
!  sleep, asctime(localtime(&timer)));
  		myopt.title = title;
  
  		/* Run the query and print out the results */
--- 3058,3071 
  		 * of completion of the command?
  		 */
  		timer = time(NULL);
! 		if (user_title)
! 			snprintf(title, title_len,
! 	 "%s\t%s",
! 	 user_title, asctime(localtime(&timer)));
! 		else
! 			snprintf(title, title_len,
! 	 _("Watch every %lds\t%s"),
! 	 sleep, asctime(localtime(&timer)));
  		myopt.title = title;
  
  		/* Run the query and print out the results */
*** do_watch(PQExpBuffer query_buf, long sle
*** 3056,3065 
  		 * PSQLexecWatch handles the case where we can no longer repeat the
  		 * query, and returns 0 or -1.
  		 */
! 		if (res == 0)
  			break;
- 		if (res == -1)
- 			return false;
  
  		/*
  		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
--- 3075,3082 
  		 * PSQLexecWatch handles the case where we can no longer repeat the
  		 * query, and returns 0 or -1.
  		 */
! 		if (res == 0 || res == -1)
  			break;
  
  		/*
  		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
*** do_watch(PQExpBuffer query_buf, long sle
*** 3084,3090 
  		sigint_interrupt_enabled = false;
  	}
  
! 	return true;
  }
  
  /*
--- 3101,3108 
  		sigint_interrupt_enabled = false;
  	}
  
! 	pg_free(title);
! 	return (res >= 0);
  }
  
  /*

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread Robert Haas
On Mon, Mar 21, 2016 at 11:17 AM, David G. Johnston
 wrote:
>> And does everybody agree that this is a desirable change?
>
> Adding the title is desirable.  While I'm inclined to bike-shed this
> anything that gets it in I can live with and so I'm content letting the
> author/committer decide where exactly things (including whitespace) appear.
>
> It is a bit odd that the "Watch every %s" gets centered if the result is
> wide but that the title remains left-aligned.

Well, the title isn't normally centered, but yeah, that is odd.  Yeah,
that is odd.  Come to think of it, I think I might have expected the
title to appear *above* "Watch every %s", not below it.  That might
decrease the oddness.

As for letting the committer decide, I don't care about this
personally at all, so I'm only looking at it to be nice to the people
who do.  Whatever is the consensus is OK with me.  I just don't want
to get yelled at later for committing something here, so it would be
nice to see a few votes for whatever we're gonna do here.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread David G. Johnston
On Mon, Mar 21, 2016 at 8:03 AM, Robert Haas  wrote:

> On Sun, Mar 20, 2016 at 9:31 AM, Michael Paquier
>  wrote:
> > And the patch attached gives the following output:
> > With title:
> > =# \watch 1
> > Watch every 1sSun Mar 20 22:28:38 2016
> > popo
> >  a
> > ---
> >  1
> > (1 row)
>

​This doesn't show the blank line above "popo" that the prior example
had...​

>
> > And without title:
> > Watch every 1sSun Mar 20 22:29:31 2016
> >
> >  a
> > ---
> >  1
> > (1 row)
>
>
​Unchanged from present behavior - but its not obvious that the watch line
is center-aligned​

And does everybody agree that this is a desirable change?
>

​Adding the title is desirable.  While I'm inclined to bike-shed this
anything that gets it in I can live with and so I'm content letting the
author/committer decide where exactly things (including whitespace) appear.

​It is a bit odd that the "Watch every %s" gets centered if the result
is wide but that the title remains left-aligned.

The minimally invasive change would be the following:

>optional title<
>watch<
>(blank line)
>headers
>head-body divider
>body
>optional footer

Though I like the idea of:

>optional title
>(blank line - if Title present)
>headers
>head-body divider
>body
>optional footer
>watch

​David J.​


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-21 Thread Robert Haas
On Sun, Mar 20, 2016 at 9:31 AM, Michael Paquier
 wrote:
> And the patch attached gives the following output:
> With title:
> =# \watch 1
> Watch every 1sSun Mar 20 22:28:38 2016
> popo
>  a
> ---
>  1
> (1 row)
>
> And without title:
> Watch every 1sSun Mar 20 22:29:31 2016
>
>  a
> ---
>  1
> (1 row)

And does everybody agree that this is a desirable change?

As for the patch itself, you could replace all this:

+   /*
+* Take into account any title present in the user setup as a part of
+* what is printed for each iteration by using it as a header.
+*/
+   if (myopt.title)
+   {
+   title_len = strlen(myopt.title);
+   title = pg_malloc(title_len + 50);
+   head_title = pg_strdup(myopt.title);
+   }
+   else
+   {
+   title_len = 0;
+   title = pg_malloc(50);
+   head_title = pg_strdup("");
+   }

...with:

head_title = pg_strdup(myopt.title != NULL ? myopt.title : "");
title_len = strlen(head_title);
title = pg_malloc(title_len + 50);

Better yet, include the + 50 in title_len, and then you don't need to
reference the number 50 again further down.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-20 Thread Michael Paquier
On Sat, Mar 19, 2016 at 11:42 PM, Robert Haas  wrote:
> On Thu, Mar 10, 2016 at 1:40 AM, David G. Johnston
>  wrote:
>> Adding -hackers for consideration in the Commitfest.
>
> I don't much like how this patch uses the arbitrary constant 50 in no
> fewer than 5 locations.
>
> Also, it seems like we could arrange for head_title to be "" rather
> than NULL when myopt.title is NULL.  Then instead of this:
>
> +if (head_title)
> +snprintf(title, strlen(myopt.title) + 50,
> + _("Watch every %lds\t%s\n%s"),
> + sleep, asctime(localtime(&timer)), head_title);
> +else
> +snprintf(title, 50, _("Watch every %lds\t%s"),
> + sleep, asctime(localtime(&timer)));
>
> ...we could just the first branch of that if all the time.

OK, why not.

>  if (res == -1)
> +{
> +pg_free(title);
> +pg_free(head_title);
>  return false;
> +}
>
> Instead of repeating the cleanup code, how about making this break;
> then, change the return statement at the bottom of the function to
> return (res != -1).

OK.

And the patch attached gives the following output:
With title:
=# \watch 1
Watch every 1sSun Mar 20 22:28:38 2016
popo
 a
---
 1
(1 row)

And without title:
Watch every 1sSun Mar 20 22:29:31 2016

 a
---
 1
(1 row)
-- 
Michael


psql_watch_title-v3.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-19 Thread Tom Lane
David Steele  writes:
> On 3/17/16 5:07 PM, David G. Johnston wrote:
>> Figured out it had to be added to 2016-09...done

> Hmm ... this patch is currently marked "needs review" in CF 2016-03.  Am
> I missing something, should this have been closed?

The message I saw was post-1-March.  If it was in fact submitted in
time for 2016-03, then we owe it a review.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-19 Thread Tom Lane
David Steele  writes:
> On 3/17/16 7:00 PM, Tom Lane wrote:
>> The message I saw was post-1-March.  If it was in fact submitted in
>> time for 2016-03, then we owe it a review.

> I meant to add the CF record and forgot:
> https://commitfest.postgresql.org/9/480
> It was added 2016-01-13 by Michael Paquier.

OK, so it looks like David's 10-Mar patch was actually just a repost of
Michael's 28-Jan patch, which was already in the queue to be reviewed in
2016-03 (and hasn't yet been).  So the addition to 2016-09 was simply
erroneous and should be deleted.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-19 Thread Robert Haas
On Thu, Mar 10, 2016 at 1:40 AM, David G. Johnston
 wrote:
> Adding -hackers for consideration in the Commitfest.

I don't much like how this patch uses the arbitrary constant 50 in no
fewer than 5 locations.

Also, it seems like we could arrange for head_title to be "" rather
than NULL when myopt.title is NULL.  Then instead of this:

+if (head_title)
+snprintf(title, strlen(myopt.title) + 50,
+ _("Watch every %lds\t%s\n%s"),
+ sleep, asctime(localtime(&timer)), head_title);
+else
+snprintf(title, 50, _("Watch every %lds\t%s"),
+ sleep, asctime(localtime(&timer)));

...we could just the first branch of that if all the time.

 if (res == -1)
+{
+pg_free(title);
+pg_free(head_title);
 return false;
+}

Instead of repeating the cleanup code, how about making this break;
then, change the return statement at the bottom of the function to
return (res != -1).

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-19 Thread David Steele
On 3/17/16 7:00 PM, Tom Lane wrote:
> David Steele  writes:
>> On 3/17/16 5:07 PM, David G. Johnston wrote:
>>> Figured out it had to be added to 2016-09...done
> 
>> Hmm ... this patch is currently marked "needs review" in CF 2016-03.  Am
>> I missing something, should this have been closed?
> 
> The message I saw was post-1-March.  If it was in fact submitted in
> time for 2016-03, then we owe it a review.

I meant to add the CF record and forgot:

https://commitfest.postgresql.org/9/480

It was added 2016-01-13 by Michael Paquier.

-- 
-David
da...@pgmasters.net


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-19 Thread Michael Paquier
On Fri, Mar 18, 2016 at 8:16 AM, Tom Lane  wrote:
> David Steele  writes:
>> On 3/17/16 7:00 PM, Tom Lane wrote:
>>> The message I saw was post-1-March.  If it was in fact submitted in
>>> time for 2016-03, then we owe it a review.
>
>> I meant to add the CF record and forgot:
>> https://commitfest.postgresql.org/9/480
>> It was added 2016-01-13 by Michael Paquier.
>
> OK, so it looks like David's 10-Mar patch was actually just a repost of
> Michael's 28-Jan patch, which was already in the queue to be reviewed in
> 2016-03 (and hasn't yet been).  So the addition to 2016-09 was simply
> erroneous and should be deleted.

My mistake I guess. I should have mentioned as well on this thread
that I registered it.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-19 Thread David Steele
On 3/17/16 5:07 PM, David G. Johnston wrote:

> Figured out it had to be added to 2016-09...done

Hmm ... this patch is currently marked "needs review" in CF 2016-03.  Am
I missing something, should this have been closed?

-- 
-David
da...@pgmasters.net


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-19 Thread David G. Johnston
Figured out it had to be added to 2016-09...done

On Wed, Mar 9, 2016 at 11:40 PM, David G. Johnston <
david.g.johns...@gmail.com> wrote:

> Adding -hackers for consideration in the Commitfest.
>
> Thanks!
>
> David J.
>
> >>>Original request by me
>
>
> http://www.postgresql.org/message-id/CAKFQuwZqjz-je3Z=8jdodym3jm-n2ul4cuqy5vh8n75e5v1...@mail.gmail.com
>
> When executing a query using \watch in psql the first execution of the
> query includes "Title is [...]" when \pset title is in use.  Subsequent
> executions do not.  Once that first display goes off-screen the information
> in the title is no longer readily accessible.  If using \watch for a
> long-running monitoring query it can be helpful to incorporate some context
> information into the title.
>
> -- Forwarded message --
> From: Michael Paquier 
> Date: Thu, Jan 28, 2016 at 6:01 AM
> Subject: Re: [GENERAL] Request - repeat value of \pset title during \watch
> interations
> To: "David G. Johnston" 
> Cc: Tom Lane , "pgsql-gene...@postgresql.org" <
> pgsql-gene...@postgresql.org>
>
>
> On Thu, Jan 28, 2016 at 1:54 PM, David G. Johnston
>  wrote:
> > On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <
> michael.paqu...@gmail.com>
> > wrote:
> >>
> >> On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
> >>  wrote:
> >> > So how about:
> >> >
> >> > + snprintf(title, strlen(myopt.title) + 50,
> >> > + _("Watch every %lds\t%s\t%s"),
> >> > +  sleep, head_title, asctime(localtime(&timer)));
> >>
> >> I would just keep the timestamp and the title separated so what do you
> >> think about that instead?
> >> Watch every Xs   $timestamp
> >> $head_title
> >
> >
> > That works.  I like having the title immediately above the table.
> >
> > The other option that came to mind would be to place the time information
> > after the table display while leaving the title before it.  On an output
> > that requires more vertical space than is available in the terminal one
> > would no longer have to scroll up to confirm last execution time.  If
> doing
> > this I'd probably get rid of any logic that attempts to center the time
> > information on the table and simply leave it left-aligned.
>
> ​And the example:
> ​
> OK, attached is an updated patch. How does that look?
>
>Watch every 5sFri Jan 29 13:06:31 2016
>
> This is a medium length title
> repeat
>
> 
> --
>  
> 
> (1 row)
> ​
>
>
>


Re: [HACKERS] [GENERAL] Request - repeat value of \pset title during \watch interations

2016-03-09 Thread David G. Johnston
Adding -hackers for consideration in the Commitfest.

Thanks!

David J.

>>>Original request by me

http://www.postgresql.org/message-id/CAKFQuwZqjz-je3Z=8jdodym3jm-n2ul4cuqy5vh8n75e5v1...@mail.gmail.com

When executing a query using \watch in psql the first execution of the
query includes "Title is [...]" when \pset title is in use.  Subsequent
executions do not.  Once that first display goes off-screen the information
in the title is no longer readily accessible.  If using \watch for a
long-running monitoring query it can be helpful to incorporate some context
information into the title.

-- Forwarded message --
From: Michael Paquier 
Date: Thu, Jan 28, 2016 at 6:01 AM
Subject: Re: [GENERAL] Request - repeat value of \pset title during \watch
interations
To: "David G. Johnston" 
Cc: Tom Lane , "pgsql-gene...@postgresql.org" <
pgsql-gene...@postgresql.org>


On Thu, Jan 28, 2016 at 1:54 PM, David G. Johnston
 wrote:
> On Wed, Jan 27, 2016 at 9:13 PM, Michael Paquier <
michael.paqu...@gmail.com>
> wrote:
>>
>> On Thu, Jan 28, 2016 at 9:34 AM, David G. Johnston
>>  wrote:
>> > So how about:
>> >
>> > + snprintf(title, strlen(myopt.title) + 50,
>> > + _("Watch every %lds\t%s\t%s"),
>> > +  sleep, head_title, asctime(localtime(&timer)));
>>
>> I would just keep the timestamp and the title separated so what do you
>> think about that instead?
>> Watch every Xs   $timestamp
>> $head_title
>
>
> That works.  I like having the title immediately above the table.
>
> The other option that came to mind would be to place the time information
> after the table display while leaving the title before it.  On an output
> that requires more vertical space than is available in the terminal one
> would no longer have to scroll up to confirm last execution time.  If
doing
> this I'd probably get rid of any logic that attempts to center the time
> information on the table and simply leave it left-aligned.

​And the example:
​
OK, attached is an updated patch. How does that look?

   Watch every 5sFri Jan 29 13:06:31 2016

This is a medium length title
repeat


--
 

(1 row)
​
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9750a5b..3241d27 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3020,7 +3020,8 @@ static bool
 do_watch(PQExpBuffer query_buf, long sleep)
 {
 	printQueryOpt myopt = pset.popt;
-	char		title[50];
+	char		 *title;
+	bool		 *head_title = NULL;
 
 	if (!query_buf || query_buf->len <= 0)
 	{
@@ -3034,6 +3035,18 @@ do_watch(PQExpBuffer query_buf, long sleep)
 	 */
 	myopt.topt.pager = 0;
 
+	/*
+	 * Take into account any title present in the user setup as a part of
+	 * what is printed for each iteration by using it as a header.
+	 */
+	if (myopt.title)
+	{
+		title = pg_malloc(strlen(myopt.title) + 50);
+		head_title = pg_strdup(myopt.title);
+	}
+	else
+		title = pg_malloc(50);
+
 	for (;;)
 	{
 		int			res;
@@ -3045,8 +3058,13 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		 * of completion of the command?
 		 */
 		timer = time(NULL);
-		snprintf(title, sizeof(title), _("Watch every %lds\t%s"),
- sleep, asctime(localtime(&timer)));
+		if (head_title)
+			snprintf(title, strlen(myopt.title) + 50,
+	 _("Watch every %lds\t%s\n%s"),
+	 sleep, asctime(localtime(&timer)), head_title);
+		else
+			snprintf(title, 50, _("Watch every %lds\t%s"),
+	 sleep, asctime(localtime(&timer)));
 		myopt.title = title;
 
 		/* Run the query and print out the results */
@@ -3059,7 +3077,11 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		if (res == 0)
 			break;
 		if (res == -1)
+		{
+			pg_free(title);
+			pg_free(head_title);
 			return false;
+		}
 
 		/*
 		 * Set up cancellation of 'watch' via SIGINT.  We redo this each time
@@ -3084,6 +3106,8 @@ do_watch(PQExpBuffer query_buf, long sleep)
 		sigint_interrupt_enabled = false;
 	}
 
+	pg_free(title);
+	pg_free(head_title);
 	return true;
 }
 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers