Re: [HACKERS] psql describe.c cleanup
On Thu, Jun 16, 2011 at 10:05 AM, Merlin Moncure mmonc...@gmail.com wrote: On Wed, Jun 15, 2011 at 9:01 AM, Merlin Moncure mmonc...@gmail.com wrote: On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt schmi...@gmail.com wrote: On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure mmonc...@gmail.com wrote: What I do wonder though is if the ; appending should really be happening in printQuery() instead of in each query -- the idea being that formatting for external consumption should be happening in one place. Maybe that's over-thinking it though. That's a fair point, and hacking up printQuery() would indeed solve my original gripe about copy-pasting queries out of psql -E. But more generally, I think that standardizing the style of the code is a Good Thing, particularly where style issues impact usability (like here). sure -- if anyone would like to comment on this one way or the other feel free -- otherwise I'll pass the patch up the chain as-is...it's not exactly the 'debate of the century' :-). done. marked ready for committer. Committed. -- 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] psql describe.c cleanup
On Wed, Jun 15, 2011 at 9:01 AM, Merlin Moncure mmonc...@gmail.com wrote: On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt schmi...@gmail.com wrote: On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure mmonc...@gmail.com wrote: What I do wonder though is if the ; appending should really be happening in printQuery() instead of in each query -- the idea being that formatting for external consumption should be happening in one place. Maybe that's over-thinking it though. That's a fair point, and hacking up printQuery() would indeed solve my original gripe about copy-pasting queries out of psql -E. But more generally, I think that standardizing the style of the code is a Good Thing, particularly where style issues impact usability (like here). sure -- if anyone would like to comment on this one way or the other feel free -- otherwise I'll pass the patch up the chain as-is...it's not exactly the 'debate of the century' :-). done. marked ready for committer. merlin -- 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] psql describe.c cleanup
On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt schmi...@gmail.com wrote: On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure mmonc...@gmail.com wrote: What I do wonder though is if the ; appending should really be happening in printQuery() instead of in each query -- the idea being that formatting for external consumption should be happening in one place. Maybe that's over-thinking it though. That's a fair point, and hacking up printQuery() would indeed solve my original gripe about copy-pasting queries out of psql -E. But more generally, I think that standardizing the style of the code is a Good Thing, particularly where style issues impact usability (like here). sure -- if anyone would like to comment on this one way or the other feel free -- otherwise I'll pass the patch up the chain as-is...it's not exactly the 'debate of the century' :-). merlin -- 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] psql describe.c cleanup
On Sat, May 7, 2011 at 2:40 PM, Josh Kupershmidt schmi...@gmail.com wrote: Hi all, I use psql's -E mode every now and then, copy-and-pasting and further tweaking the SQL displayed. Most queries are displayed terminated by a semicolon, but quite a few aren't, making copy-and-paste just a bit more tedious. Attached is a patch to fix every SQL query I saw in describe.c. There were also a few queries with trailing newlines, and I fixed those too. I did a quick review and test of your patch. It didn't quite apply cleanly due to recent non-related describe.c changes -- updated patch attached. First, I'll give you a thumbs up on the original inspiration for the patch. The output should be standardized, and I see no reason not to append a semicolon on usability basis. Beyond that, the changes are mostly cosmetic and I can't see how it will break things outside of terminating a query early by accident (I didn't see any). What I do wonder though is if the ; appending should really be happening in printQuery() instead of in each query -- the idea being that formatting for external consumption should be happening in one place. Maybe that's over-thinking it though. merlin psql_describe.v3.patch Description: Binary data -- 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] psql describe.c cleanup
Excerpts from Josh Kupershmidt's message of sáb may 07 16:40:35 -0300 2011: And while I'm griping about describe.c, is it just me or is the source code indentation in that file totally screwy? I'm using emacs and I've loaded the snippet for pgsql-c-mode from ./src/tools/editors/emacs.samples into my ~/.emacs, but the indentation of multiline strings still looks inconsistent. See attached screenshot, e.g. the start of line 80 has four tabs and one space, while line 79 has six tabs and two spaces? pgindent moves strings back to the left when it thinks they fit within 80 columns. Yes, that seems pretty screwy. -- Á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] psql describe.c cleanup
On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure mmonc...@gmail.com wrote: I did a quick review and test of your patch. It didn't quite apply cleanly due to recent non-related describe.c changes -- updated patch attached. Thanks for looking at this. Your updated patch looks good to me. First, I'll give you a thumbs up on the original inspiration for the patch. The output should be standardized, and I see no reason not to append a semicolon on usability basis. Beyond that, the changes are mostly cosmetic and I can't see how it will break things outside of terminating a query early by accident (I didn't see any). Yeah, I really didn't want to break any queries, so I did my best to test every query I changed. What I do wonder though is if the ; appending should really be happening in printQuery() instead of in each query -- the idea being that formatting for external consumption should be happening in one place. Maybe that's over-thinking it though. That's a fair point, and hacking up printQuery() would indeed solve my original gripe about copy-pasting queries out of psql -E. But more generally, I think that standardizing the style of the code is a Good Thing, particularly where style issues impact usability (like here). I would actually like to see a movement towards having all these queries use whitespace/formatting consistently. For instance, when I do a \d sometable I see some queries with lines bleeding out maybe 200 columns, some wrapped at 80 columns, etc. This sort of style issue is not something that a simple kludge in printQuery() could solve (and even if we put in a sophisticated formatting tool inside printQuery(), we'd still be left with an ugly describe.c). For the record, I find that having SQL queries consistently formatted makes them much more readable, allowing a human to roughly parse a query on sight once you're used to the formatting. So I wouldn't be opposed to putting the kludge in printQuery(), but I would like to see us standardize the queries regardless. (And in that case, I wouldn't mind if we dropped all the semicolons in describe.c, just that we're consistent.) Josh -- 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] psql describe.c cleanup
On Tue, Jun 14, 2011 at 3:25 PM, Alvaro Herrera alvhe...@commandprompt.com wrote: pgindent moves strings back to the left when it thinks they fit within 80 columns. Yes, that seems pretty screwy. I am losing track of the ways in which pgindent has managed to mangle our source code :-/ Josh -- 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] psql describe.c cleanup
On lör, 2011-05-07 at 15:40 -0400, Josh Kupershmidt wrote: And while I'm griping about describe.c, is it just me or is the source code indentation in that file totally screwy? I'm using emacs and I've loaded the snippet for pgsql-c-mode from ./src/tools/editors/emacs.samples into my ~/.emacs, but the indentation of multiline strings still looks inconsistent. See attached screenshot, e.g. the start of line 80 has four tabs and one space, while line 79 has six tabs and two spaces? Yes, it's screwy, but not only there. See http://archives.postgresql.org/message-id/1295913661.13617.5.ca...@vanquo.pezone.net for the reason. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
