Re: [Tiki-devel] Unicode-aware sort plugin

Volker Wysk <post-hhF2Jplw28UoZk/[email protected]> Fri, 07 Jul 2023 14:25:46 +0200
Newsgroups	gmane.comp.cms.tiki.devel
Message-ID	<[email protected]>
Am Freitag, dem 07.07.2023 um 14:48 +0300 schrieb Victor Emanouilov via
TikiWiki-devel:
> Answers inline:
> 
> On 7/7/23 12:33 PM, Volker Wysk wrote:
> > Am Freitag, dem 07.07.2023 um 11:13 +0300 schrieb Victor Emanouilov via
> > TikiWiki-devel:
> > > Hi Volker,
> > Hi there
> > 
> > > Yes, you are right, I was looking at the wrong place. Collator depdends
> > > on intl extension but it might still not be installed everywhere.

> > My PHP knowledge is very limited, but isn't the intl extension the standard
> > place for internationalisation things? Tiki deals a lot with
> > internationalisation, so couldn't we rely on it being installed - with an
> > error message in the installer/tiki-check if it isn't?

> V: I could find just 5 occurences of Intl* usage or NumberFormatter in 
> Tiki and these were probably improperly used without actual check if the 
> intl extension is loaded. tiki-check has a check if intl extenion is 
> loaded, so user can see if there is an error. 

Good, so we don't have a problem here.

> Still, I believe it is a 
> bad practice and against Tiki philosophy to fail with an error message 
> when a 3rd party feature is unavailable instead of falling back to 
> existing solution, especially when these solutions were the default ones 
> for the past years. It is enough to warn user that sorting can be 
> incorrect if they don't install the intl extension.

As long as we have that warning in tiki-check, I'm fine with this.

> > 
> > > It
> > > will be nice to add this check to installer and tiki-check as well.
> > > 
> > > I do think, however, that Tiki code must proceed and not hard-quit in
> > > case Collator is not found, so a check for the existence of this class
> > > is necessary.
> > See above.
> > 
> > > Note that plugin sort is one of the very few places where sorting is
> > > used. Most of the time Tiki depends on the database to sort or the
> > > search index, so we are not doing this on the PHP side.
> > Like I've written: The natsort and natcasesort functions are used about a
> > dozen times, each. A search for "\bsort\(" reveals even more, 133
> > occurrences.
> 
> V: I reviewed the places where php sort is used and still think these 
> are a small portion of the total places where Tiki sorts. 

I've looked again, and there are still over 50 places, when you don't count
the vendor_bundled stuff and you don't count places where sort() is called
as a method. It's not always easy to see where it's the PHP function sort()
and where it's some method of a Tiki class.

I'm not contesting that this is a small portion of the total places where
Tiki sorts, but I wouldn't call that "very few places".

> Most of these 
> are in the database with order by clause. To be exact: natsort is used 3 
> times and natcasesort 5 times in current Tiki master.

You aren't counting the occurrences in vendor_bundled. That's probably
right.

> So, the question remains - after you do mb_natcasesort function - where 
> would you use it?

I'm not sure. Of course, I'd use it in wikiplugin_sort. And I expect that
mb_nat(case)sort should be used in most of the eight places which you've
counted. But don't take my word for it, I'm just guessing.


Cheers,
Volker

> > > On 7/6/23 4:08 PM, Volker Wysk wrote:
> > > > Am Mittwoch, dem 05.07.2023 um 09:45 +0300 schrieb Victor Emanouilov via
> > > > TikiWiki-devel:
> > > > > Hi Volker,
> > > > > 
> > > > > Looks good. Just a small note that Collator depends on enchant library
> > > > > and might not be available on all systems.
> > > > That's strange. I don't have the PHP enchant library installed:
> > > > 
> > > > desktop ~ $ LANG=C apt list '*php*enchant*'
> > > > Listing... Done
> > > > php-enchant/jammy,jammy 2:8.2+93+ubuntu22.04.1+deb.sury.org+2 all
> > > > php5.6-enchant/jammy 5.6.40-66+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > php7.0-enchant/jammy 7.0.33-66+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > php7.1-enchant/jammy 7.1.33-53+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > php7.2-enchant/jammy 7.2.34-39+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > php7.3-enchant/jammy 7.3.33-11+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > php7.4-enchant/jammy 1:7.4.33-6+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > php8.0-enchant/jammy 1:8.0.29-1+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > php8.1-enchant/jammy 8.1.20-1+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > php8.1-enchant/jammy 8.1.8-1+ubuntu22.04.1+deb.sury.org+1 i386
> > > > php8.2-enchant/jammy 8.2.7-1+ubuntu22.04.1+deb.sury.org+1 amd64
> > > > 
> > > > desktop ~ $ LANG=C apt list '*php*enchant*' --installed
> > > > Listing... Done
> > > > 
> > > > But the wikiplugin_sort.php version, which I've built, and uses it, DOES
> > > > work. Are you sure about the enchant library?
> > > > 
> > > > I've looked in the php.net pages, and it says that the Collator class is
> > > > part of the intl library, which depends on the C/C++ ICU library (and not
> > > > enchant).
> > > > 
> > > > 
> > > > > I think you should make the
> > > > > use of Collator sort optional - if the feature is not present, fall back
> > > > > to original sorting methods.
> > > > I'm not sure if this is such a great idea. I'd rather check in the Tiki
> > > > installer if the library is available and output a warning or error message
> > > > if it isn't.
> > > > 
> > > > I think this ordering, which is locale aware and at the same time
> > > > natural, is needed in quite some places. The natsort and natcasesort
> > > > functions are used about a dozen times, each, in Tiki 25 (many in
> > > > vendor_bundled). I expect that most of those occurences should made be
> > > > Unicode aware, meaning they need to be modified.
> > > > 
> > > > We could make mb_natsort() and mb_natcasesort() functions which do just
> > > > that, by using the Collator class. Then just "mb_" needs to be added each
> > > > time.
> > > > 
> > > > 
> > > > Cheers,
> > > > Volker
> > > > 
> > > > > On 7/1/23 12:49 PM, Volker Wysk wrote:
> > > > > > Am Donnerstag, dem 29.06.2023 um 19:06 +0200 schrieb Volker Wysk:
> > > > > > > Hi!
> > > > > > > 
> > > > > > > The Sort Plugin is kinda broken, because it isn't Unicode aware. That's not
> > > > > > > that easy to fix, because the PHP natcasesort() function isn't Unicode
> > > > > > > aware. And there is no mb_natcasesort(). It needs to be done differently.
> > > > > > > 
> > > > > > > I've found on the web, how it is to be done, here:
> > > > > > > 
> > > > > > > https://stackoverflow.com/questions/58577558/how-to-use-multiple-sort-flags-in-php-array-sorting-using-sort-locale-string
> > > > > > > 
> > > > > > > I've applied it to the wikiplugin_sort and it works. However, this plugin
> > > > > > > needs to use the correct locale for the user. I don't know how to get this
> > > > > > > locale. This bit is still missing.
> > > > > > > 
> > > > > > > I've attached the new wikiplugin_sort.php.
> > > > > > > 
> > > > > > > Someone with more Tiki knowledge should improve this version, by querying
> > > > > > > the locale to be used and pass it to the collator_create function.
> > > > > > I've figured it out (hopefully). The new version sorts naturally and in
> > > > > > accordance to the user's locale. I've attached it, again.
> > > > > > 
> > > > > > I'm about to do a merge request next.
> > > > > > 
> > > > > > Cheers,
> > > > > > Volker
> > > > > > 
> > > > > > 
> > > > > > _______________________________________________
> > > > > > TikiWiki-devel mailing list
> > > > > > [email protected]
> > > > > > https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel
> > > > > _______________________________________________
> > > > > TikiWiki-devel mailing list
> > > > > [email protected]
> > > > > https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel
> > > > 
> > > > _______________________________________________
> > > > TikiWiki-devel mailing list
> > > > [email protected]
> > > > https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel
> > > 
> > > _______________________________________________
> > > TikiWiki-devel mailing list
> > > [email protected]
> > > https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel
> > 
> > 
> > _______________________________________________
> > TikiWiki-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel
> 
> 
> _______________________________________________
> TikiWiki-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel

_______________________________________________
TikiWiki-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tikiwiki-devel
signature.asc (application/pgp-signature, 833 B)