Add grapheme_levenshtein function.

[youkidearitai]

Measure levenshtein for grapheme cluster unit.
RFC: https://wiki.php.net/rfc/grapheme_levenshtein

[youkidearitai]

This RFC was accepted. I will go to implementation.

[devnexen]

Nice, could you possibly rebase with last master state to redo the CI ? cheers.

[youkidearitai]

I rebased and force pushed. I'm waiting for CI end.

[devnexen]

should be fine, thanks. Looking good but I ll go back at it in couple of hours.

[devnexen]

LGTM minus the remark

[DanielEScherzer]

Is there a reason that false is being returned on failure, rather than throwing an exception?

[youkidearitai]

For example, Userland can uses intl_get_error_message function after this function if this error. Therefore, this block is returns false.

[youkidearitai]

Hmm... Windows CI is failed.

[ndossche]

Where are the error checks for these calls and the next ones?

[ndossche]

The error checks seem to be still missing.

[ndossche]

You also don't need the void* cast, you can remove that.

[ndossche]

Where possible, I think it's better to merge the declaration and assignment as it reduces the area/scope where something is valid/can be used.

[ndossche]

The declarations and assignments are still not merged

[ndossche]

Second review round.
As a more general comment: the code can probably be simplified even more by using goto-style error handling.

[ndossche]

The entire comparison is too complex and inefficient.
You can just do something like this (I can't use a suggestion because the changes are not centralised):

diff --git a/ext/intl/grapheme/grapheme_string.c b/ext/intl/grapheme/grapheme_string.c
index c4d0c3987a8..c0f21f1ad7e 100644
--- a/ext/intl/grapheme/grapheme_string.c
+++ b/ext/intl/grapheme/grapheme_string.c
@@ -1041,6 +1041,9 @@ PHP_FUNCTION(grapheme_levenshtein)
 		RETURN_FALSE;
 	}
 
+	UCollator *collator = ucol_open("", &ustatus1);
+	// TODO: handle error
+
 	p1 = safe_emalloc(strlen_2 + 1, sizeof(zend_long), 0);
 	p2 = safe_emalloc(strlen_2 + 1, sizeof(zend_long), 0);
 
@@ -1052,7 +1055,6 @@ PHP_FUNCTION(grapheme_levenshtein)
 	int32_t current2 = 0;
 	int32_t pos1 = 0;
 	int32_t pos2 = 0;
-	int32_t usrch_pos = 0;
 
 	while (true) {
 		current1 = ubrk_current(bi1);
@@ -1067,37 +1069,8 @@ PHP_FUNCTION(grapheme_levenshtein)
 			if (pos2 == UBRK_DONE) {
 				break;
 			}
-			UStringSearch *srch = usearch_open(ustring1 + current1, pos1 - current1, ustring2 + current2, pos2 - current2, "", NULL, &ustatus2);
-			if (U_FAILURE(ustatus2)) {
-				intl_error_set_code(NULL, ustatus2);
-				intl_error_set_custom_msg(NULL, "Error usearch_open", 0);
-				ubrk_close(bi1);
-				ubrk_close(bi2);
-
-				efree(ustring1);
-				efree(ustring2);
-
-				efree(p1);
-				efree(p2);
-				RETURN_FALSE;
-			}
-			usrch_pos = usearch_first(srch, &ustatus2);
-			if (U_FAILURE(ustatus2)) {
-				intl_error_set_code(NULL, ustatus2);
-				intl_error_set_custom_msg(NULL, "Error usearch_first", 0);
-				ubrk_close(bi1);
-				ubrk_close(bi2);
-
-				efree(ustring1);
-				efree(ustring2);
-
-				efree(p1);
-				efree(p2);
-				RETURN_FALSE;
-			}
-			usearch_close(srch);
 
-			if (usrch_pos != USEARCH_DONE) {
+			if (ucol_strcoll(collator, ustring1 + current1, pos1 - current1, ustring2 + current2, pos2 - current2) == UCOL_EQUAL) {
 				c0 = p1[i2];
 			} else {
 				c0 = p1[i2] + cost_rep;
@@ -1118,6 +1091,8 @@ PHP_FUNCTION(grapheme_levenshtein)
 		p2 = tmp;
 	}
 
+	ucol_close(collator);
+
 	ubrk_close(bi1);
 	ubrk_close(bi2);
 

[youkidearitai]

I didn't intend to use collation. Please give me some time to judge.

[youkidearitai]

Thanks for suggestion. I could not find problem. Use your code.

[ndossche]

The error checks seem to be still missing.

[ndossche]

The declarations and assignments are still not merged

[ndossche]

I think you can just use a single status variable that you reuse all the time, that makes things a bit less complex.

[ndossche]

You also don't need the void* cast, you can remove that.

[ndossche]

The error message could probably be improved (same below)
e.g. "Error on ubrk_setText for argument #1 ($string1)"

[ndossche]

The code looks almost fully good now.
I wonder if the repetition in the error recovery can be reduced by using an error label with gotos. Maybe that's something you can try

[ndossche]

Can't this include be deleted now that you no longer rely on usearch API?

[ndossche]

Please get rid of this void cast as well.

[ndossche]

Duplicated line

[ndossche]

This is ok apart from the merging of error-handling code. This is something I might try later if you don't do it.

[ndossche]

Please don't forget to add this to UPGRADING & NEWS upon merging.

[youkidearitai]

Thanks for review, UPGRADING and NEWS is updated.

[ndossche]

Feel free to merge

[youkidearitai]

Thank you very much.