small cleanup in unicode_norm.c

Started by John Naylorover 5 years ago4 messageshackers
Jump to latest
#1John Naylor
john.naylor@enterprisedb.com

We've had get_canonical_class() for a while as a backend-only function.
There is some ad-hoc code elsewhere that implements the same logic in a
couple places, so it makes sense for all sites to use this function
instead, as in the attached.

--
John Naylor
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachments:

use-canonical-class-func.patchapplication/octet-stream; name=use-canonical-class-func.patchDownload+19-25
#2Michael Paquier
michael@paquier.xyz
In reply to: John Naylor (#1)
Re: small cleanup in unicode_norm.c

On Mon, Dec 07, 2020 at 03:24:56PM -0400, John Naylor wrote:

We've had get_canonical_class() for a while as a backend-only function.
There is some ad-hoc code elsewhere that implements the same logic in a
couple places, so it makes sense for all sites to use this function
instead, as in the attached.

Thanks John for caring about that. This is a nice simplification, and
it looks fine to me.

-static uint8
-get_canonical_class(pg_wchar ch)
-{
Two nits here. I would use "code" for the name of the argument for
consistency with get_code_entry(), and add a description at the top of
this helper routine (say a simple "get the combining class of given
code"). Anything else you can think of?
--
Michael

#3John Naylor
john.naylor@enterprisedb.com
In reply to: Michael Paquier (#2)
Re: small cleanup in unicode_norm.c

On Tue, Dec 8, 2020 at 5:45 AM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Dec 07, 2020 at 03:24:56PM -0400, John Naylor wrote:

We've had get_canonical_class() for a while as a backend-only function.
There is some ad-hoc code elsewhere that implements the same logic in a
couple places, so it makes sense for all sites to use this function
instead, as in the attached.

Thanks John for caring about that. This is a nice simplification, and
it looks fine to me.

-static uint8
-get_canonical_class(pg_wchar ch)
-{
Two nits here. I would use "code" for the name of the argument for
consistency with get_code_entry(), and add a description at the top of
this helper routine (say a simple "get the combining class of given
code"). Anything else you can think of?

Thanks for taking a look. Sounds good, I've made those adjustments and
wrote a commit message. I took another look and didn't see anything else to
address.

--
John Naylor
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachments:

v2-0001-Simplify-ad-hoc-code-for-getting-a-Unicode-codepo.patchapplication/octet-stream; name=v2-0001-Simplify-ad-hoc-code-for-getting-a-Unicode-codepo.patchDownload+22-26
#4Michael Paquier
michael@paquier.xyz
In reply to: John Naylor (#3)
Re: small cleanup in unicode_norm.c

On Tue, Dec 08, 2020 at 02:25:43PM -0400, John Naylor wrote:

Thanks for taking a look. Sounds good, I've made those adjustments and
wrote a commit message. I took another look and didn't see anything else to
address.

Looks good to me, so applied.
--
Michael