Skip to content

[String] Example from the documentation is not working (bug report + failing test) #34751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

VasekPurchart
Copy link
Contributor

Symfony version(s) affected: 5.0.*

Description
The code example for AsciiSlugger is not working as shown in the documentation.

The article shows:

use Symfony\Component\String\Slugger\AsciiSlugger;

$slugger = new AsciiSlugger();
$slug = $slugger->slug('Wôrķšƥáçè ~~sèťtïñğš~~');
// $slug = 'Workspace-settings'

but the actual result is (note the missing "p"):

Works-ace-settings

Or another possibility is that this is locale dependent, but I have also tried specifying some locales (like cs, de, fr, el) but everything with the same result.

How to reproduce

Either by running the code above or by running the test in the attached PR.

@VasekPurchart
Copy link
Contributor Author

According to the Travis results this might be PHP version dependent - it works on 7.3 but neither on 7.2 or 7.4. And I guess it is reasonable to expect that at the time of writing the author of the documentation might have been using PHP 7.3.

I think this should be treated as a bug in code (and not documentation), especially with the different results for different PHP versions.

@VasekPurchart VasekPurchart changed the title [String] Example from the documentation is not working [String] Example from the documentation is not working (bug report + failing test) Dec 2, 2019
@javiereguiluz
Copy link
Member

@VasekPurchart when I prepared the docs I run all examples to double check that the result was correct. I've just tested this again with the latest stable String component.

I used this code:

require_once __DIR__.'/vendor/autoload.php';

use Symfony\Component\String\Slugger\AsciiSlugger;

$slugger = new AsciiSlugger();
$slug = $slugger->slug('Wôrķšƥáçè ~~sèťtïñğš~~');
echo $slug;

I run composer require symfony/string and I'm using PHP 7.2.18 (I know, I have to update it 😓). This is the result:

image

@VasekPurchart
Copy link
Contributor Author

@javiereguiluz thanks for the additional info, it is even weirder then, that it works for you but not here in tests.

Could you please check, if you have the intl extension loaded? I checked with:

var_dump(extension_loaded('intl'));

and I have it enabled in my Docker image where I found out about this.

Also I think it is enabled on Travis, because the unit thest I used requires it. But I am not sure if this is not somehow modified due to the polyfills etc.

If you do not have intl enabled, then I think that might be the source of the issue.

@VasekPurchart
Copy link
Contributor Author

VasekPurchart commented Dec 2, 2019

@javiereguiluz otherwise could you please try running the attached test on your machine?

php vendor/bin/simple-phpunit src/Symfony/Component/String/Tests/SluggerTest.php

Thanks! :)

@nicolas-grekas
Copy link
Member

Maybe a different version of the intl extension?

@javiereguiluz
Copy link
Member

javiereguiluz commented Dec 2, 2019

Tests seem to be OK:

image

The intl extension is indeed enabled and these are the details for it:

image

@VasekPurchart
Copy link
Contributor Author

Thanks again, from my image:

intl

Internationalization support => enabled
version => 1.1.0
ICU version => 57.1
ICU Data version => 57.1
ICU TZData version => 2016b
ICU Unicode version => 8.0

Directive => Local Value => Master Value
intl.default_locale => no value => no value
intl.error_level => 0 => 0
intl.use_exceptions => 0 => 0

So yours seems to be more up to date, will try to update the image tomorrow and report back here whether it helped.

@VasekPurchart
Copy link
Contributor Author

VasekPurchart commented Dec 3, 2019

Just tried rebuilding the image based on php:7.2-fpm PHP 7.2.25 (fpm-fcgi) and it is still not working with these versions:

intl

Internationalization support => enabled
version => 1.1.0
ICU version => 63.1
ICU Data version => 63.1
ICU TZData version => 2018e
ICU Unicode version => 11.0

Directive => Local Value => Master Value
intl.default_locale => no value => no value
intl.error_level => 0 => 0
intl.use_exceptions => 0 => 0

I use this image because I have not dropped support for PHP 7.2 in all projects, but will try to build a new one based on 7.3 if I can get newer versions out of the box.

@VasekPurchart
Copy link
Contributor Author

VasekPurchart commented Dec 3, 2019

I upgraded the image to be based on php:7.3-fpm PHP 7.3.12 (fpm-fcgi) and got the same versions as with the updated 7.2:

intl

Internationalization support => enabled
ICU version => 63.1
ICU Data version => 63.1
ICU TZData version => 2018e
ICU Unicode version => 11.0

Directive => Local Value => Master Value
intl.default_locale => no value => no value
intl.error_level => 0 => 0
intl.use_exceptions => 0 => 0

So I guess this is due to the fact that the image is based on Debian which has rather slow update policy. If this is the case I am afraid a lot of users will have the same problem (maybe majority at least for Docker uses?).

@javiereguiluz could you please share how your PHP was built (+ image/system?)

@javiereguiluz
Copy link
Member

javiereguiluz commented Dec 3, 2019

I use macOS and my PHP (7.2) was installed via Homebrew.

In any case, the problem seems to be the "intl" version. I'm using a very recent version:

intl

Internationalization support => enabled
version => 1.1.0
ICU version => 64.2
ICU Data version => 64.2
ICU TZData version => 2019a
ICU Unicode version => 12.1

@nicolas-grekas
Copy link
Member

@VasekPurchart can you please give me the output of this script?

<?php
  
$c = file('Latin-ASCII.xml');

$map = [];

foreach ($c as $l) {
    $l = explode('', $l);

    if (2 !== count($l) ||'[' === $l[0][0]) {
        continue;
    }

    $l[1] = preg_replace('/ ; #.*/s', '', $l[1]);

    if (Normalizer::normalize($l[1], Normalizer::NFKD) === $l[1]) {
        continue;
    }

    $map[$l[0]] = $l[1];
}

var_export($map);

With https://github.com/unicode-org/cldr/blob/master/common/transforms/Latin-ASCII.xml as input?

@VasekPurchart
Copy link
Contributor Author

@javiereguiluz thanks!

@nicolas-grekas it returns an empty array in both 7.2 and 7.3 for me (used images from which the intl version output is above):

~symfony $ docker run --rm -it -v $PWD:/workspace -w="/workspace" 53d1d8ab2997 bash -c 'php -v && php test.php'
PHP 7.2.25 (cli) (built: Nov 22 2019 17:34:02) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.2.25, Copyright (c) 1999-2018, by Zend Technologies
array (
)
~symfony $ docker run --rm -it -v $PWD:/workspace -w="/workspace" aa83cfc7fa6b bash -c 'php -v && php test.php'
PHP 7.3.12 (cli) (built: Nov 22 2019 16:38:39) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.3.12, Copyright (c) 1998-2018 Zend Technologies
    with Zend OPcache v7.3.12, Copyright (c) 1999-2018, by Zend Technologies
array (
)

@nicolas-grekas
Copy link
Member

Please check if #34782 fixes the issue.

@VasekPurchart
Copy link
Contributor Author

Yes, thanks very much! :)

Looking forward to using the String component.

@nicolas-grekas
Copy link
Member

OK, green in #34782, thanks for reporting!

@VasekPurchart VasekPurchart deleted the not-working-string-example branch December 3, 2019 13:49
fabpot added a commit that referenced this pull request Dec 7, 2019
This PR was merged into the 5.0 branch.

Discussion
----------

[String] inline Latin-ASCII rules

| Q             | A
| ------------- | ---
| Branch?       | 5.0
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34751
| License       | MIT
| Doc PR        | -

Makes the component a bit more portable.

Commits
-------

976a938 [String] inline Latin-ASCII rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants