improved slugify function to support more characters

[diegocavalletti]

The old one had an issue with some UTF-8 character, for example italian character à,ò,è,é,ì would not be converted to their "flattened" version (a,o,e,e,i) but instead they would get truncated. I think this is not always the inted behaviour so i replaced with another one found online. Credit goes to the creator of it (mentioned in comments). Licence is cc so it's fine i guess

[diegocavalletti]

Also i would like to note that i can't seem to be able to get the last version via composer, the only way atm is to specify the last commit hash but that's not so good, can u fix that too please? I'm still living with my hack to not touch the source code i have locally of this lib

[adelowo]

@diegocavalletti is it possible to add tests for this ? Another thing is the code style, i had to make it compliant with most part of the codebase in the previous PR- the original author didn't take that into consideration and some part of the codebase still suffers from that -. Braces should go onto the other line.

[adelowo]

That is a little bit weird, i can pull the latest version. Let me try again to do that

[adelowo]

I can pull the latest tag, i don't know why you have this issue. Don't mind the warning there, my internet is currently too slow ^__^

What does your composer.json look like ? i mean the line that lists this package as a dependency.

[diegocavalletti]

Mmm, i just noticed that indeed one of the fixes made in my pull is present inside the official file but if you look here https://github.com/Gargron/fileupload/pull/42/files you will see i modified this part:

protected function process($tmp_name, $name, $size, $type, $error, $index = 0, $content_range = null)
      {
 -        $file = $this->fileContainer = new File($tmp_name);
 +      $file = new File( $tmp_name );
 +      if ( $index == 0 ) {
 +          $this->fileContainer = $file;
 +      }

which doesn't seem to be present in the master file https://github.com/Gargron/fileupload/blob/master/src/FileUpload/FileUpload.php.

Also there's no need for new test, you can use the one i made the last time i suppose. The final goal is the same, it just change how it do it.

About the code style i use a total different one, do you have an export PhpStorm (since it looks like you're using that IDE too) compatibile?

At least ill just use yours format standard for the next one

[adelowo]

Yes, as per #37 , #44 , it was best to have it inside the codebase back since it was what actually differentiated multiple uploads (in terms of error messages and the likes)

The tests i said you should add are those for the italian characters.. I am currently at school, not with my laptop but you can go to settings>>code style >> php >> set from predefined >> choose psr1/psr2

[diegocavalletti]

I was remembering of something when i fixed process function: https://github.com/Gargron/fileupload/issues/25 You added the cast from that pull request, but i think that now that u just removed the cast you are back with this error, therefore it is better so keep separated instances (modifying the process function as i suggested) to make it work again

[adelowo]

@diegocavallett i don't get this. Those issues i linked to are not related to the cast fix you did, they were related to the \SPLinfo class.. And a lack of adding proper flow to the codebase if the validation failed.

[diegocavalletti]

I'm lost now lol. What exactly changed in #37,#44 that u linked? I can't seem to understand the link between those you referenced and what's inside my fix i mentioned in my 3rd post.

When i made my test not changing $file = $this->fileContainer = new File($tmp_name); this to this:

$file = new File( $tmp_name );
if ( $index == 0 ) {
    $this->fileContainer = $file;
}

Would lead to problem. So what to do? I'm still using my fix (which includes the above fix aswell) and everything seems to work fine

[adelowo]

Maybe that was poorly worded, I am having a sort of bad day and would update this when I clear my head .

As an aside, I don't get any errors without your fix.. Another major reason why that is in there is because the validators rely on the getter to add errors and stuff onto the file instance. If we only set that when index is 0,we lose proper error messages

[diegocavalletti]

I can't really remember right now what i've faced that lead me to add that fix aswell, i'll do more tests and if i find a suitable case i'll let you know. In the while i'll just update the unit test so that you can merge it if you want

[adelowo]

@diegocavalletti PING

[diegocavalletti]

@adelowo Sorry mate, i'm in the middle of a very busy developing period, dead line coming soon and i had to leave behind the test on your library. I didn't forget this, just waiting for the developing run to end.

I'll keep you updated

[diegocavalletti]

Sorry for being so late, i had the time to make some tests lately and indeed i had no problems w/o https://github.com/Gargron/fileupload/pull/46#issuecomment-284333700 that fix.

I'll update the test file and make a new pull request, it should be fine now.

Sorry again for the long wait

[adelowo]

Tests also fail on <= PHP 7.0... And we can't bump library version right now ( #36 )

[diegocavalletti]

@adelowo I can't make my PhpStorm work with tests anymore, i'm trying to find a way. After that i'll fix the issues and push a new update

[adelowo]

No problem, would wait patiently.. Thanks a lot.

[diegocavalletti]

@adelowo I think i'm giving up on making this test unit to work, i don't know what else to do but it just gives me "Process finished with exit code 255"

In addition to that it seems that all my attemps to install phpunits on my mac failed since phpstorm can't even auto-complete the methods. In my company unit test are not used so i can't spend more time trying to make this work. Any chance that you can pull on your local and see what's wrong?

[adelowo]

@diegocavalletti Sure... I would look into this tomorrow morning..

Thanks a lot