Re: review: psql: edit function, show function commands patch
От | Robert Haas |
---|---|
Тема | Re: review: psql: edit function, show function commands patch |
Дата | |
Msg-id | AANLkTikB_YxYfuueqB_BTE===wVos38836b81oRp32Xf@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: review: psql: edit function, show function commands patch (Robert Haas <robertmhaas@gmail.com>) |
Ответы |
Re: review: psql: edit function, show function commands patch
(Tom Lane <tgl@sss.pgh.pa.us>)
Re: review: psql: edit function, show function commands patch (Pavel Stehule <pavel.stehule@gmail.com>) Re: review: psql: edit function, show function commands patch (Pavel Stehule <pavel.stehule@gmail.com>) |
Список | pgsql-hackers |
On Sun, Aug 1, 2010 at 11:48 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> b) more robust algorithm for header rows identification > > Have not gotten to this one yet. I notIce that on WIN32 the default editor is notepad.exe and the default editor navigation option is "/". Does "notepad.exe /lineno filename" actually work on Windows? A quick Google search suggests that the answer is "no", which seems somewhat unfortunate: it means we'd be shipping "broken on Win32 out of the box". http://www.robvanderwoude.com/commandlineswitches.php#Notepad This is actually my biggest concern about this patch - that it may be just too much of a hassle to actually make it work for people. I just tried setting $EDITOR to MacOS's TextEdit program, and it turns out that TextEdit doesn't understand +. I'm afraid that we're going to end up with a situation where it only works for people using emacs or vi, and everyone else ends up with a broken install (and, possibly, no clear idea how to fix it). Perhaps it would be better to accept \sf and reject \sf+ and \ef <func> <lineno>. Assuming we get past that threshold issue, I'm also wondering whether the "navigation option" terminology is best; e.g. set PSQL_EDITOR_NAVIGATION_OPTION to configure it. It doesn't seem terrible, but it doesn't seem great, either. Anyone have a better idea? The docs are a little rough; they could benefit from some editing by a fluent English speaker. If anyone has time to work on this, it would be much appreciated. Instead of "line number is unacceptable", I think you should write "invalid line number". "dollar" should not be spelled "dolar". "function" should not be spelled "finction". This change looks suspiciously like magic. If it's actually safe, it needs a comment explaining why: - sys = pg_malloc(strlen(editorName) + strlen(fname) + 10 + 1); + sys = pg_malloc(strlen(editorName) + strlen(fname) + 20 + 1); Attempting to compile with this patch applied emits a warning on my machine; whether or not the warning is valid, you need to make it go away: command.c: In function ‘HandleSlashCmds’: command.c:1055: warning: ‘bsep’ may be used uninitialized in this function command.c:1055: note: ‘bsep’ was declared here Why does the \sf output have a trailing blank line? This seems weird, especially because \ef puts no such trailing blank line in the editor. Instead of: + /* skip useles whitespaces */ + while (c >= func) + if (isblank(*c)) + c--; + else + break; ...wouldn't it be just as good to write: while (c >= func && isblank(*c)) c--; (and similarly elsewhere) In extract_separator, if you invert the sense of the first if-test, then you can avoid having to indent the entire function contents. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company
В списке pgsql-hackers по дате отправления:
Следующее
От: Tom LaneДата:
Сообщение: Re: review: psql: edit function, show function commands patch