review: psql: edit function, show function commands patch

Hi,

here's a review of the \sf and \ef [num] patch from
http://archives.postgresql.org/message-id/162867791003290927y3ca44051p80e697bc6b19de29@mail.gmail.com

== Formatting ==

The patch has some small tabs/spaces and whitespace  issues and it 
applies with some offsets, I ran pgindent and rebased against HEAD, 
attaching the resulting patch for your convenience.

== Functionality ==

The patch adds the following features:
  * \e file.txt num  ->  starts a editor for the current query buffer 
and puts the cursor on the [num] line
  * \ef func num -> starts a editor for a function and puts the cursor 
on the [num] line
  * \sf func -> shows a full CREATE FUNCTION statement for the function
  * \sf+ func -> the same, but with line numbers
  * \sf[+] func num -> the same, but only from line num onward

It only touches psql, so no performance or backend stability worries.

In my humble opinion, only the \sf[+] is interesting, because it gives 
you a copy/pasteable version of the function definition without opening 
up an editor, and I can find that useful (OTOH: you can set PSQL_EDITOR 
to cat and get the same effect with \ef... ok, just joking). Line 
numbers are an extra touch, personally it does not thrill me too much, 
but I've nothing against it.

The number variants of \e and \ef work by simply executing $EDITOR +num 
file. I tried with some editors that came to my mind, and not all of 
them support it (most do, though):

  * emacs and emacsclient work
  * vi works
  * nano works
  * pico works
  * mcedit works
  * kwrite does not work
  * kedit does not work

not sure what other people (or for instance Windows people) use. Apart 
from no universal support from editors, it does not save that many 
keystrokes - at most a couple. In the end you can usually easily jump to 
the line you want once you are inside your dream editor.

My recommendation would be to only integrate the \sf[+] part of the 
patch, which will have the additional benefit of making it much smaller 
and cleaner (will avoid the grotty splitting of the number from the 
function name, for instance). But I'm just another user out there, maybe 
others will find uses for the other cases.

I would personally not add the leading and trailing newlines to \sf 
output, but that's a question of taste.

Docs could use some small grammar fixes, but other than that they're fine.

== Code ==

In \sf code there just a strncmp, so this works:
\sfblablabla funcname

The error for an empty \sf is not great, it should probably look more like
\sf: missing required argument
following the examples of \pset, \copy or \prompt.

Why is lnptr always being passed as a pointer? Looks like a unnecessary 
complication and one more variable to care about. Can't we just pass lineno?

== End ==

Cheers,
Jan