On Mon, Mar 21, 2016 at 10:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@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.
I'd rather not omit sleep but removing "Watch every" is fine (preferred actually), so:
