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

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

On Sat, Apr 1, 2017 at 3:04 AM, Fabien COELHO <coelho@cri.ensmp.fr> 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

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

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

On Wed, Jul 19, 2017 at 2:41 PM, Fabien COELHO <coelho@cri.ensmp.fr> 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

Fabien COELHO <coelho@cri.ensmp.fr> 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

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