Thread: psql describe.c cleanup
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. 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? Josh
Attachment
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.camel@vanquo.pezone.net for the reason.
On Sat, May 7, 2011 at 2:40 PM, Josh Kupershmidt <schmiddy@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
Attachment
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 <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure <mmoncure@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
On Tue, Jun 14, 2011 at 3:25 PM, Alvaro Herrera <alvherre@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
On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: > On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure <mmoncure@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
On Wed, Jun 15, 2011 at 9:01 AM, Merlin Moncure <mmoncure@gmail.com> wrote: > On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >> On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure <mmoncure@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
On Thu, Jun 16, 2011 at 10:05 AM, Merlin Moncure <mmoncure@gmail.com> wrote: > On Wed, Jun 15, 2011 at 9:01 AM, Merlin Moncure <mmoncure@gmail.com> wrote: >> On Tue, Jun 14, 2011 at 9:08 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >>> On Tue, Jun 14, 2011 at 12:15 PM, Merlin Moncure <mmoncure@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