Horton

Thread

Commits

  1. Merge duplicative code for \sf/\sv, \ef/\ev in psql/command.c.

  1. merge psql ef/ev sf/sv handling functions

    Fabien COELHO <coelho@cri.ensmp.fr> — 2017-03-31T18:04:04Z

    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.

  2. Re: merge psql ef/ev sf/sv handling functions

    Victor Drobny <v.drobny@postgrespro.ru> — 2017-07-18T12:10:24Z

    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

  3. Re: merge psql ef/ev sf/sv handling functions

    Fabien COELHO <coelho@cri.ensmp.fr> — 2017-07-18T12:40:37Z

    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.

  4. Re: merge psql ef/ev sf/sv handling functions

    Masahiko Sawada <sawada.mshk@gmail.com> — 2017-07-19T01:10:30Z

    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

  5. Re: merge psql ef/ev sf/sv handling functions

    Fabien COELHO <fabien.coelho@mines-paristech.fr> — 2017-07-19T05:39:22Z

    >> 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.

  6. Re: merge psql ef/ev sf/sv handling functions

    Fabien COELHO <coelho@cri.ensmp.fr> — 2017-07-19T05:41:27Z

    >> 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.

  7. Re: merge psql ef/ev sf/sv handling functions

    Masahiko Sawada <sawada.mshk@gmail.com> — 2017-07-19T06:05:53Z

    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

  8. Re: merge psql ef/ev sf/sv handling functions

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-09-06T21:37:32Z

    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

  9. Re: merge psql ef/ev sf/sv handling functions

    Fabien COELHO <coelho@cri.ensmp.fr> — 2017-09-07T05:19:59Z

    > 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.