Robert Haas <robertmh...@gmail.com> 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
