Re: [HACKERS] trailing whitespace in psql table output
Robert Haas robertmh...@gmail.com writes: On Mon, Sep 27, 2010 at 2:09 PM, David Fetter da...@fetter.org wrote: I must be missing something pretty crucial here as far as the complexity of changing all the regression tests. Wouldn't trimming all trailing whitespace do the trick? Sure. But everyone using pg_regress will have to update their regression test expected outputs. I'm inclined to think that that's not a fatal objection; it's not like we haven't felt free to change psql's output format before. As long as we don't back-patch this change, it should be no worse than other things we've done to third-party code without a backwards glance. It would be good to get rid of this whitespace because (I believe) it is one of very few reasons for needing to have any trailing whitespace in git-controlled files. If we could get to a point where trailing whitespace in patches could be rejected automatically, it'd eliminate one small pet peeve. 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] trailing whitespace in psql table output
On Tue, Sep 28, 2010 at 12:18 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Sep 27, 2010 at 2:09 PM, David Fetter da...@fetter.org wrote: I must be missing something pretty crucial here as far as the complexity of changing all the regression tests. Wouldn't trimming all trailing whitespace do the trick? Sure. But everyone using pg_regress will have to update their regression test expected outputs. I'm inclined to think that that's not a fatal objection; it's not like we haven't felt free to change psql's output format before. As long as we don't back-patch this change, it should be no worse than other things we've done to third-party code without a backwards glance. It would be good to get rid of this whitespace because (I believe) it is one of very few reasons for needing to have any trailing whitespace in git-controlled files. If we could get to a point where trailing whitespace in patches could be rejected automatically, it'd eliminate one small pet peeve. I have to agree that would be nice. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] trailing whitespace in psql table output
David Fetter da...@fetter.org writes: On Mon, Sep 27, 2010 at 03:11:07PM -0400, Robert Haas wrote: Sure. But everyone using pg_regress will have to update their regression test expected outputs. Again, I must be missing something super important. What is it that prevents people from doing find . -type f |xargs perl -pi.bak -e 's/\s+$//g' I think the concern isn't about the change being hard to make, it's about the fallout from having to have different expected-result files for 9.1 versus previous releases. A lot of third-party modules try to maintain source code compatibility across all PG releases they support, and this would break that. That's not necessarily a sufficient reason for us not to do it --- but it's definitely not a negligible issue, either. It should be noted that this won't be zero-cost for the core project either. For example, any back-patched fix that adjusts regression test outputs will have to deal with the incompatibility. And if we do put in some git-level check on whitespace, it'll have to be made to only apply to master not back branches. 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] trailing whitespace in psql table output
On mån, 2010-09-27 at 11:09 -0700, David Fetter wrote: I must be missing something pretty crucial here as far as the complexity of changing all the regression tests. Wouldn't trimming all trailing whitespace do the trick? No, there is trailing whitespace that is significant. -- 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] trailing whitespace in psql table output
On tis, 2010-09-28 at 12:18 -0400, Tom Lane wrote: I'm inclined to think that that's not a fatal objection; it's not like we haven't felt free to change psql's output format before. As long as we don't back-patch this change, it should be no worse than other things we've done to third-party code without a backwards glance. In the past, pg_regress used diff -b or -w, so making whitespace changes in psql was not a problem. It would be good to get rid of this whitespace because (I believe) it is one of very few reasons for needing to have any trailing whitespace in git-controlled files. If we could get to a point where trailing whitespace in patches could be rejected automatically, it'd eliminate one small pet peeve. You won't be able to programmatically forbid all trailing whitespace (at least without additional arrangements) because of: psql -c 'select 1 as a, null as b' | cat -A a | b$ ---+---$ 1 | $=== Plus, there might be tests that check trailing space behavior or some such, but I haven't looked for those. -- 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] trailing whitespace in psql table output
Peter Eisentraut pete...@gmx.net writes: On tis, 2010-09-28 at 12:18 -0400, Tom Lane wrote: It would be good to get rid of this whitespace because (I believe) it is one of very few reasons for needing to have any trailing whitespace in git-controlled files. If we could get to a point where trailing whitespace in patches could be rejected automatically, it'd eliminate one small pet peeve. You won't be able to programmatically forbid all trailing whitespace (at least without additional arrangements) because of: Yeah, if we can't get all the way there easily, the above argument isn't worth much. 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] trailing whitespace in psql table output
On fre, 2010-09-24 at 22:38 +0100, Roger Leigh wrote: On Tue, Sep 21, 2010 at 09:28:07PM +0300, Peter Eisentraut wrote: Everyone using git diff in color mode will already or soon be aware that psql, for what I can only think is an implementation oversight, produces trailing whitespace in the table headers, like this: two | f1 $ -+$ | asdfghjkl;$ | d34aaasdf$ (2 rows)$ Does this break the output with \pset border 2? Um, no. In the meantime, I have arrived at the conclusion that doing this isn't worth it because it will break all regression test output. We can fix the stuff in our tree, but pg_regress is also used externally, and those guys would have a nightmare with this change. Perhaps if there is another more significant revision of the table style in the future, we should keep this issue in mind. -- 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] trailing whitespace in psql table output
On Sep 27, 2010, at 5:05 AM, Peter Eisentraut wrote: Um, no. In the meantime, I have arrived at the conclusion that doing this isn't worth it because it will break all regression test output. We can fix the stuff in our tree, but pg_regress is also used externally, and those guys would have a nightmare with this change. Perhaps if there is another more significant revision of the table style in the future, we should keep this issue in mind. Or change the way pg_regress works. David -- 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] trailing whitespace in psql table output
Excerpts from David E. Wheeler's message of lun sep 27 12:25:31 -0400 2010: On Sep 27, 2010, at 5:05 AM, Peter Eisentraut wrote: Um, no. In the meantime, I have arrived at the conclusion that doing this isn't worth it because it will break all regression test output. We can fix the stuff in our tree, but pg_regress is also used externally, and those guys would have a nightmare with this change. Perhaps if there is another more significant revision of the table style in the future, we should keep this issue in mind. Or change the way pg_regress works. Perhaps using unaligned mode? The problem with that is that it becomes very difficult to review changes to expected output. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] trailing whitespace in psql table output
On Mon, Sep 27, 2010 at 1:34 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from David E. Wheeler's message of lun sep 27 12:25:31 -0400 2010: On Sep 27, 2010, at 5:05 AM, Peter Eisentraut wrote: Um, no. In the meantime, I have arrived at the conclusion that doing this isn't worth it because it will break all regression test output. We can fix the stuff in our tree, but pg_regress is also used externally, and those guys would have a nightmare with this change. Perhaps if there is another more significant revision of the table style in the future, we should keep this issue in mind. Or change the way pg_regress works. Perhaps using unaligned mode? The problem with that is that it becomes very difficult to review changes to expected output. Uh, yuck! If we don't care about changing the expected output, we can just trim the whitespace as Peter suggested originally. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] trailing whitespace in psql table output
On Mon, Sep 27, 2010 at 01:53:45PM -0400, Robert Haas wrote: On Mon, Sep 27, 2010 at 1:34 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from David E. Wheeler's message of lun sep 27 12:25:31 -0400 2010: On Sep 27, 2010, at 5:05 AM, Peter Eisentraut wrote: Um, no. In the meantime, I have arrived at the conclusion that doing this isn't worth it because it will break all regression test output. We can fix the stuff in our tree, but pg_regress is also used externally, and those guys would have a nightmare with this change. Perhaps if there is another more significant revision of the table style in the future, we should keep this issue in mind. Or change the way pg_regress works. Perhaps using unaligned mode? The problem with that is that it becomes very difficult to review changes to expected output. Uh, yuck! If we don't care about changing the expected output, we can just trim the whitespace as Peter suggested originally. I must be missing something pretty crucial here as far as the complexity of changing all the regression tests. Wouldn't trimming all trailing whitespace do the trick? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] trailing whitespace in psql table output
On Mon, Sep 27, 2010 at 2:09 PM, David Fetter da...@fetter.org wrote: On Mon, Sep 27, 2010 at 01:53:45PM -0400, Robert Haas wrote: On Mon, Sep 27, 2010 at 1:34 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from David E. Wheeler's message of lun sep 27 12:25:31 -0400 2010: On Sep 27, 2010, at 5:05 AM, Peter Eisentraut wrote: Um, no. In the meantime, I have arrived at the conclusion that doing this isn't worth it because it will break all regression test output. We can fix the stuff in our tree, but pg_regress is also used externally, and those guys would have a nightmare with this change. Perhaps if there is another more significant revision of the table style in the future, we should keep this issue in mind. Or change the way pg_regress works. Perhaps using unaligned mode? The problem with that is that it becomes very difficult to review changes to expected output. Uh, yuck! If we don't care about changing the expected output, we can just trim the whitespace as Peter suggested originally. I must be missing something pretty crucial here as far as the complexity of changing all the regression tests. Wouldn't trimming all trailing whitespace do the trick? Sure. But everyone using pg_regress will have to update their regression test expected outputs. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] trailing whitespace in psql table output
On Mon, Sep 27, 2010 at 03:11:07PM -0400, Robert Haas wrote: On Mon, Sep 27, 2010 at 2:09 PM, David Fetter da...@fetter.org wrote: On Mon, Sep 27, 2010 at 01:53:45PM -0400, Robert Haas wrote: On Mon, Sep 27, 2010 at 1:34 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from David E. Wheeler's message of lun sep 27 12:25:31 -0400 2010: On Sep 27, 2010, at 5:05 AM, Peter Eisentraut wrote: Um, no. In the meantime, I have arrived at the conclusion that doing this isn't worth it because it will break all regression test output. We can fix the stuff in our tree, but pg_regress is also used externally, and those guys would have a nightmare with this change. Perhaps if there is another more significant revision of the table style in the future, we should keep this issue in mind. Or change the way pg_regress works. Perhaps using unaligned mode? The problem with that is that it becomes very difficult to review changes to expected output. Uh, yuck! If we don't care about changing the expected output, we can just trim the whitespace as Peter suggested originally. I must be missing something pretty crucial here as far as the complexity of changing all the regression tests. Wouldn't trimming all trailing whitespace do the trick? Sure. But everyone using pg_regress will have to update their regression test expected outputs. Again, I must be missing something super important. What is it that prevents people from doing find . -type f |xargs perl -pi.bak -e 's/\s+$//g' or moral equivalent on their pg_regression tree? Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] trailing whitespace in psql table output
On Mon, Sep 27, 2010 at 4:12 PM, David Fetter da...@fetter.org wrote: On Mon, Sep 27, 2010 at 03:11:07PM -0400, Robert Haas wrote: On Mon, Sep 27, 2010 at 2:09 PM, David Fetter da...@fetter.org wrote: On Mon, Sep 27, 2010 at 01:53:45PM -0400, Robert Haas wrote: On Mon, Sep 27, 2010 at 1:34 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from David E. Wheeler's message of lun sep 27 12:25:31 -0400 2010: On Sep 27, 2010, at 5:05 AM, Peter Eisentraut wrote: Um, no. In the meantime, I have arrived at the conclusion that doing this isn't worth it because it will break all regression test output. We can fix the stuff in our tree, but pg_regress is also used externally, and those guys would have a nightmare with this change. Perhaps if there is another more significant revision of the table style in the future, we should keep this issue in mind. Or change the way pg_regress works. Perhaps using unaligned mode? The problem with that is that it becomes very difficult to review changes to expected output. Uh, yuck! If we don't care about changing the expected output, we can just trim the whitespace as Peter suggested originally. I must be missing something pretty crucial here as far as the complexity of changing all the regression tests. Wouldn't trimming all trailing whitespace do the trick? Sure. But everyone using pg_regress will have to update their regression test expected outputs. Again, I must be missing something super important. What is it that prevents people from doing find . -type f |xargs perl -pi.bak -e 's/\s+$//g' or moral equivalent on their pg_regression tree? I assume it's not that simple, but I haven't tried it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] trailing whitespace in psql table output
On Tue, Sep 21, 2010 at 09:28:07PM +0300, Peter Eisentraut wrote: Everyone using git diff in color mode will already or soon be aware that psql, for what I can only think is an implementation oversight, produces trailing whitespace in the table headers, like this: two | f1 $ -+$ | asdfghjkl;$ | d34aaasdf$ (2 rows)$ Does this break the output with \pset border 2? IIRC when I was doing the \pset linestyle work, I did look at doing this, but found that the padding was required in some cases. I couldn't tell from looking over the patch whether or not you were already taking this into account though? Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `-GPG Public Key: 0x25BFB848 Please GPG sign your mail. signature.asc Description: Digital signature
[HACKERS] trailing whitespace in psql table output
Everyone using git diff in color mode will already or soon be aware that
psql, for what I can only think is an implementation oversight, produces
trailing whitespace in the table headers, like this:
two | f1 $
-+$
| asdfghjkl;$
| d34aaasdf$
(2 rows)$
($ is the line end; cf. cat -A). Note that this only applies to
headers, not content cells.
Attached is a patch to fix that.
diff --git a/src/bin/psql/print.c b/src/bin/psql/print.c
index e55404b..da23b7b 100644
--- a/src/bin/psql/print.c
+++ b/src/bin/psql/print.c
@@ -817,20 +817,24 @@ print_aligned_text(const printTableContent *cont, FILE *fout)
nbspace = width_wrap[i] - this_line-width;
/* centered */
- fprintf(fout, %-*s%s%-*s,
-nbspace / 2, , this_line-ptr, (nbspace + 1) / 2, );
+ fprintf(fout, %-*s%s,
+nbspace / 2, , this_line-ptr);
if (!(this_line + 1)-ptr)
{
more_col_wrapping--;
- header_done[i] = 1;
+ header_done[i] = true;
}
+
+ if (i cont-ncolumns - 1 || !header_done[i])
+ fprintf(fout, %-*s,
+ (nbspace + 1) / 2, );
}
else
fprintf(fout, %*s, width_wrap[i], );
if (opt_border != 0 || format-wrap_right_border == true)
- fputs(!header_done[i] ? format-header_nl_right : ,
+ fputs(!header_done[i] ? format-header_nl_right : (i cont-ncolumns -1 ? : ),
fout);
if (opt_border != 0 i col_count - 1)
--
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] trailing whitespace in psql table output
On Wed, Sep 22, 2010 at 3:28 AM, Peter Eisentraut pete...@gmx.net wrote: Everyone using git diff in color mode will already or soon be aware that psql, for what I can only think is an implementation oversight, produces trailing whitespace in the table headers, I think removing trailing whitespace in headers itself is reasonable, but the change breaks almost all of regression tests. Will we adjust expected results for the change? -- Itagaki Takahiro -- 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] trailing whitespace in psql table output
On Wed, Sep 22, 2010 at 09:48:12AM +0900, Itagaki Takahiro wrote: On Wed, Sep 22, 2010 at 3:28 AM, Peter Eisentraut pete...@gmx.net wrote: Everyone using git diff in color mode will already or soon be aware that psql, for what I can only think is an implementation oversight, produces trailing whitespace in the table headers, I think removing trailing whitespace in headers itself is reasonable, but the change breaks almost all of regression tests. Will we adjust expected results for the change? On its face, this doesn't seem like a hard change to make. Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
