amphp / ext-uv

Other
191 stars 28 forks source link

IDE documentation/stubs, updated and added examples mainly for Windows usage. #77

Open TheTechsTech opened 4 years ago

TheTechsTech commented 4 years ago
bwoebi commented 4 years ago

Hey,

cool that you're working on it - see it's WIP, but just a quick note at a first glance:

https://github.com/bwoebi/php-uv/blob/master/php_uv.c#L2626-L2676 is defining a lot of UV named classes. There are no resourced returned by uv_ functions, only properly typed objects.

And the functions are operating on these objects then. (You can compare that with the code, where it's using UV_PARAM_OBJ() with the appropriate class reference. These objects btw. have no methods.

The README is a little outdated in that regard... But these stubs are cool and we can keep them up to date; much better than that unwieldy README.

Thanks for working on it!

ghost commented 4 years ago

FYI: There are already uv stubs in PhpStorm. However these are wrong, since they say the functions return resources.

cc @WyriHaximus

TheTechsTech commented 4 years ago

@bwoebi, As things currently stand, the readme show resources begin returned for all functions, which should be typed objects. The real reason for WIP, it need more interface like stubs for them, and i'm still testing out, seeing all this library functionality under windows. This extension much better than Event and Ev. It can even handle what Parallel offer.

@CharlotteDunois, My searching did not reveal PhpStorm stubs, otherwise might have not even started this, anyway it does not really document or as informative as what's listed on libuv.org website.

WyriHaximus commented 4 years ago

@CharlotteDunois FYI I took the readme of this extension as the source for those stubs

ghost commented 4 years ago

@WyriHaximus That's what I figured. But that doesn't make it correct. :D

@techno-express You should not think that libuv is a replacement for parallel. While libuv offers queueing work for a separate thread, there is no special handling being taken care of. I would not be surprised if specific things would simply segfault. Parallel is explicitely made for parallelism in PHP and takes special care of everything. If you need parallelism in PHP, use parallel and not the threading work API of libuv.

TheTechsTech commented 4 years ago

@CharlotteDunois, maybe you can add some long overdue examples to Parallel repo.

From my programming background/schooling, I look at everything from an computer science perspective. It's just the concepts to me, and whether does it produce the results.

What is Parallel trying to achieve, how is it accomplish, how much effort is needed to get there, when is it needed?

Anyway, I did leave some comments there, https://github.com/krakjoe/parallel/issues/59, https://github.com/krakjoe/parallel/issues/25.

I read Parallel documentation, it's more wrapped up in the terminology from what I see, and that can be simply matched to what's presented in libuv. There are workarounds to everything, it's just the developers responsibility to find it.

Take the uv_spawn() example rewrite it using Parallel, much more work.

Parallel or parallelism have very limited uses, and can currently be accomplished using no external or third party extension.

WyriHaximus commented 4 years ago

@CharlotteDunois agreed, I trusted the readme here to be correct. Maybe we can generate the stubs and readme from the ext source code to make them correct

bwoebi commented 4 years ago

I've updated the inline proto stubs within php_uv.c: e76ac0dccb7908181ee715326e4f8061b77712f6 (i.e. grep for {{{ proto)

These should reflect the current state accurately. And also directly mirror the callable signatures. Now the stubs could be updated to be really correct.

TheTechsTech commented 4 years ago

I see, I believed I already had those types added. Look over whenever, I think i have most things covered.

Unless you have some corrections I need to be make, I'm mostly finish, some stuff mostly Windows I wanted to actually tests to report on.

bwoebi commented 4 years ago

At a quick glance it's not too bad, but there are still more than 10 signatures which still need improvement. I can check them out though in the next week.

TheTechsTech commented 4 years ago

but there are still more than 10 signatures which still need improvement.

You right, i pull in the stub's from from https://github.com/JetBrains/phpstorm-stubs/blob/master/uv/uv_functions.php.

I sectioned them off, now realize i didn't go thru all of them. I been going thru, update and move from the section. I will finish those.

I also see I have to rename UVHandle as base, to your UV that all other objects extends from.

bwoebi commented 4 years ago

I think we should remove the declarations from README.md completely (in favor of the stubs)

TheTechsTech commented 4 years ago

README.md completely (in favor of the stubs)

Will do shortly.

glickel commented 3 years ago

Hi, is there any news ? If you need help to finish this pull request, it would be a pleasure

TheTechsTech commented 3 years ago

I figure it's mostly done, when started it was focused on the previous ext-uv version, might need double checking on some callback signature changes.

glickel commented 3 years ago

@techno-express so you need some times to finish this pull request ? Or do you wait for @bwoebi to review ?

TheTechsTech commented 3 years ago

No, the PR is finish, needs review and not up to me to merge.