Re: [HACKERS] merge psql ef/ev sf/sv handling functions
you can't do this sort of thing: -psql_error("The server (version %s) does not support editing function source.\n", +psql_error("The server (version %s) does not support editing %s.\n", formatPGVersionNumber(pset.sversion, false, - sverbuf, sizeof(sverbuf))); + sverbuf, sizeof(sverbuf)), + is_func ? "function source" : "view definitions"); It's too much of a pain in the rear for translators. Argh, indeed, I totally forgot about translations. Usually there is a _() hint for gettext. See https://www.postgresql.org/docs/devel/static/nls-programmer.html#nls-guidelines Usually we just use two independent messages, and that's what I did. Yep, makes sense. Thanks for the fix. -- Fabien. -- 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] merge psql ef/ev sf/sv handling functions
Fabien COELHO writes: > Here is an attempt at merging some functions which removes 160 lines of > code. Pushed with minor adjustments. I thought a couple of variable names could be better chosen, but mostly, you can't do this sort of thing: -psql_error("The server (version %s) does not support editing function source.\n", +psql_error("The server (version %s) does not support editing %s.\n", formatPGVersionNumber(pset.sversion, false, - sverbuf, sizeof(sverbuf))); + sverbuf, sizeof(sverbuf)), + is_func ? "function source" : "view definitions"); It's too much of a pain in the rear for translators. See https://www.postgresql.org/docs/devel/static/nls-programmer.html#nls-guidelines Usually we just use two independent messages, and that's what I did. 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] merge psql ef/ev sf/sv handling functions
While reviewing Corey's \if patch, I complained that there was some amount of copy-paste in "psql/command.c". Here is an attempt at merging some functions which removes 160 lines of code. Thank you for the patch. Is this an item for PG11? Yep. -- Fabien. -- 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] merge psql ef/ev sf/sv handling functions
On Wed, Jul 19, 2017 at 2:41 PM, Fabien COELHO wrote: > >>> While reviewing Corey's \if patch, I complained that there was some >>> amount >>> of copy-paste in "psql/command.c". >>> >>> Here is an attempt at merging some functions which removes 160 lines of >>> code. >> >> >> Thank you for the patch. Is this an item for PG11? > > > Yes, as it is submitted to CF 2017-09. > Thank! It is already registered to next CF. I missed it, sorry. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] merge psql ef/ev sf/sv handling functions
While reviewing Corey's \if patch, I complained that there was some amount of copy-paste in "psql/command.c". Here is an attempt at merging some functions which removes 160 lines of code. Thank you for the patch. Is this an item for PG11? Yes, as it is submitted to CF 2017-09. -- Fabien. -- 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] merge psql ef/ev sf/sv handling functions
On Sat, Apr 1, 2017 at 3:04 AM, Fabien COELHO wrote: > > Hello, > > While reviewing Corey's \if patch, I complained that there was some amount > of copy-paste in "psql/command.c". > > Here is an attempt at merging some functions which removes 160 lines of > code. > Thank you for the patch. Is this an item for PG11? Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- 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] merge psql ef/ev sf/sv handling functions
Hello Victor, While reviewing Corey's \if patch, I complained that there was some amount of copy-paste in "psql/command.c". Here is an attempt at merging some functions which removes 160 lines of code. I was looking through your patch. It seems good, the of the functions was very similar. Indeed. I guess that it was initially a copy paste. I have a question for you. What was the reason to replace "printfPQExpBuffer" by "resetPQExpBuffer" and "appendPQExpBufferStr"? Because the "printf" version implies interpreting the format layer which does not add significant value compared to just appending the string. -- Fabien. -- 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] merge psql ef/ev sf/sv handling functions
On 2017-03-31 21:04, Fabien COELHO wrote: Hello, While reviewing Corey's \if patch, I complained that there was some amount of copy-paste in "psql/command.c". Here is an attempt at merging some functions which removes 160 lines of code. Hello, I was looking through your patch. It seems good, the of the functions was very similar. I have a question for you. What was the reason to replace "printfPQExpBuffer" by "resetPQExpBuffer" and "appendPQExpBufferStr"? Thank you for attention! -- -- Victor Drobny Postgres Professional: http://www.postgrespro.com The Russian 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
[HACKERS] merge psql ef/ev sf/sv handling functions
Hello, While reviewing Corey's \if patch, I complained that there was some amount of copy-paste in "psql/command.c". Here is an attempt at merging some functions which removes 160 lines of code. -- Fabien.diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 94a3cfc..edf875d 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -75,10 +75,8 @@ static backslashResult exec_command_d(PsqlScanState scan_state, bool active_bran const char *cmd); static backslashResult exec_command_edit(PsqlScanState scan_state, bool active_branch, PQExpBuffer query_buf, PQExpBuffer previous_buf); -static backslashResult exec_command_ef(PsqlScanState scan_state, bool active_branch, -PQExpBuffer query_buf); -static backslashResult exec_command_ev(PsqlScanState scan_state, bool active_branch, -PQExpBuffer query_buf); +static backslashResult exec_command_ef_ev(PsqlScanState scan_state, bool active_branch, + PQExpBuffer query_buf, bool is_func); static backslashResult exec_command_echo(PsqlScanState scan_state, bool active_branch, const char *cmd); static backslashResult exec_command_elif(PsqlScanState scan_state, ConditionalStack cstack, @@ -118,10 +116,8 @@ static backslashResult exec_command_s(PsqlScanState scan_state, bool active_bran static backslashResult exec_command_set(PsqlScanState scan_state, bool active_branch); static backslashResult exec_command_setenv(PsqlScanState scan_state, bool active_branch, const char *cmd); -static backslashResult exec_command_sf(PsqlScanState scan_state, bool active_branch, -const char *cmd); -static backslashResult exec_command_sv(PsqlScanState scan_state, bool active_branch, -const char *cmd); +static backslashResult exec_command_sf_sv(PsqlScanState scan_state, bool active_branch, + const char *cmd, bool is_func); static backslashResult exec_command_t(PsqlScanState scan_state, bool active_branch); static backslashResult exec_command_T(PsqlScanState scan_state, bool active_branch); static backslashResult exec_command_timing(PsqlScanState scan_state, bool active_branch); @@ -322,9 +318,9 @@ exec_command(const char *cmd, status = exec_command_edit(scan_state, active_branch, query_buf, previous_buf); else if (strcmp(cmd, "ef") == 0) - status = exec_command_ef(scan_state, active_branch, query_buf); + status = exec_command_ef_ev(scan_state, active_branch, query_buf, true); else if (strcmp(cmd, "ev") == 0) - status = exec_command_ev(scan_state, active_branch, query_buf); + status = exec_command_ef_ev(scan_state, active_branch, query_buf, false); else if (strcmp(cmd, "echo") == 0 || strcmp(cmd, "qecho") == 0) status = exec_command_echo(scan_state, active_branch, cmd); else if (strcmp(cmd, "elif") == 0) @@ -380,9 +376,9 @@ exec_command(const char *cmd, else if (strcmp(cmd, "setenv") == 0) status = exec_command_setenv(scan_state, active_branch, cmd); else if (strcmp(cmd, "sf") == 0 || strcmp(cmd, "sf+") == 0) - status = exec_command_sf(scan_state, active_branch, cmd); + status = exec_command_sf_sv(scan_state, active_branch, cmd, true); else if (strcmp(cmd, "sv") == 0 || strcmp(cmd, "sv+") == 0) - status = exec_command_sv(scan_state, active_branch, cmd); + status = exec_command_sf_sv(scan_state, active_branch, cmd, false); else if (strcmp(cmd, "t") == 0) status = exec_command_t(scan_state, active_branch); else if (strcmp(cmd, "T") == 0) @@ -976,28 +972,29 @@ exec_command_edit(PsqlScanState scan_state, bool active_branch, } /* - * \ef -- edit the named function, or present a blank CREATE FUNCTION - * template if no argument is given + * \ef/\ev -- edit the named function/view, or + * present a blank CREATE FUNCTION/VIEW template if no argument is given */ static backslashResult -exec_command_ef(PsqlScanState scan_state, bool active_branch, -PQExpBuffer query_buf) +exec_command_ef_ev(PsqlScanState scan_state, bool active_branch, + PQExpBuffer query_buf, bool is_func) { backslashResult status = PSQL_CMD_SKIP_LINE; if (active_branch) { - char *func = psql_scan_slash_option(scan_state, - OT_WHOLE_LINE, NULL, true); + char *stuff = psql_scan_slash_option(scan_state, + OT_WHOLE_LINE, NULL, true); int lineno = -1; - if (pset.sversion < 80400) + if (pset.sversion < (is_func ? 80400 : 70400)) { char sverbuf[32]; - psql_error("The server (version %s) does not support editing function source.\n", + psql_error("The server (version %s) does not support editing %s.\n", formatPGVersionNumber(pset.sversion, false, - sverbuf, sizeof(sverbuf))); + sverbuf, sizeof(sverbuf)), + is_func ? "function source" : "view definitions"); status = PSQL_CMD_ERROR; } else if (!query_buf) @@ -1007,36 +1004,43 @@ exec_command_ef(PsqlScanState scan_state, bool active_branch, } else { - Oid foid = InvalidOid; + Oid stuff_oid = InvalidOid;