Re: [PHP-DEV] automatic formatting checks for pull requests?
i have tested running clang-format against the entire php-src codebase, and there is only 1 file it breaks: ext/spl/spl_directory_arginfo.h more details at https://github.com/php/php-src/pull/13417#issuecomment-1950920114 On Sun, 18 Feb 2024 at 01:45, Derick Rethans wrote: > > On 17 February 2024 22:18:05 GMT, Ilija Tovilo wrote: > >Hi Hans > > > >On Sat, Feb 17, 2024 at 3:31 PM Gina P. Banyard wrote: > >> > >> On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan > >> wrote: > >> > >> > Can we add automatic formatting checks for pull requests? > >> > Made a PR: https://github.com/php/php-src/pull/13417 > >> > >> It would be nice to have some formatting rules to harmonize the codebase > >> as it is somewhat the wild west, > >> but as far as my understanding goes is that Clang format struggles to > >> understand our codebase (namely macros) and is difficult to set-up for > >> php-src. > > > >Right. Consistent code style is nice, but what we have now is really > >not that bad. There are a couple things I'd want if we enforce code > >style: > > > >* Fixing the style should be easy, running a single command without > >first pushing to CI. > >* It should be fast too, so that I can easily run it for every commit, > >preferably even on-save in my editor. > >* The new code style should be applied only to newly added sections or > >changed code, not entire files. Otherwise, we'll have many changes in > >large files, with endless merge conflicts when merging up from lower > >branches. > >* The formatting tool should work for all php-src code, not just plain > >C code. We don't want to be forced to refactor old macros just because > >we need to add a single line to some long-standing code. Last time I > >tried clang-format, it utterly failed with our macros. > > > >I haven't looked at your PR in detail, so I'm not sure which of these > >points it satisfies. It would be great if you could quickly describe > >how it works, and what the goals are. > > > >Essentially, I'm just sceptical that this isn't more trouble than it's worth. > > > >Ilija > > IMO, clang-format isn't really suitable. Its untunable style is often far > from the coding style that we currently have, and it makes some really odd > choices as to when and how to wrap lines, making code definitely less > readable. > > cheers > Derick > IMO, clang-format isn't really suitable. Its untunable style is often far > from the coding style that we currently have, and it makes some really odd > choices as to when and how to wrap lines, making code definitely less > readable. it is far from "untunable", the manpage for tuning is huge: https://clang.llvm.org/docs/ClangFormatStyleOptions.html
Re: [PHP-DEV] automatic formatting checks for pull requests?
On Sun, 18 Feb 2024 at 00:51, Ilija Tovilo wrote:
>
> Hi Hans
>
> On Sat, Feb 17, 2024 at 3:31 PM Gina P. Banyard wrote:
> >
> > On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan
> > wrote:
> >
> > > Can we add automatic formatting checks for pull requests?
> > > Made a PR: https://github.com/php/php-src/pull/13417
> >
> > It would be nice to have some formatting rules to harmonize the codebase as
> > it is somewhat the wild west,
> > but as far as my understanding goes is that Clang format struggles to
> > understand our codebase (namely macros) and is difficult to set-up for
> > php-src.
>
> Right. Consistent code style is nice, but what we have now is really
> not that bad. There are a couple things I'd want if we enforce code
> style:
>
> * Fixing the style should be easy, running a single command without
> first pushing to CI.
> * It should be fast too, so that I can easily run it for every commit,
Yeah.. Basically clang-format-diff (not clang-format, but clang-format-diff)
is difficult to work with, the easiest approach would be to make a
full copy of the current working dir,
and in that copy instruct git to reset to master and apply the diff
between master and the parent working dir,
and then have clang-format-diff analyze it and make its formatting suggestions,
-but- that will not be fast, that will be slow. Haven't found a fast
way to get clang-format-diff to work with a *dirty* workingdir against
master,
-yet- at least. Feel free to investigate, I'm pretty sure it can be
done, but don't know how to do it.
> preferably even on-save in my editor.
If we can make it fast, we could make some opt-in git-commit-hook to format it
(but we need to figure out the dirty-working-dir-speed thing first)
> * The new code style should be applied only to newly added sections or
> changed code, not entire files. Otherwise, we'll have many changes in
> large files, with endless merge conflicts when merging up from lower
> branches.
Yeah, that's already in place :)
> * The formatting tool should work for all php-src code, not just plain
> C code. We don't want to be forced to refactor old macros just because
> we need to add a single line to some long-standing code. Last time I
> tried clang-format, it utterly failed with our macros.
Seems to work with macros when I tested it :) Tested it with the
following macros as of writing:
ZEND_PARSE_PARAMETERS_START
Z_PARAM_NUMBER
ZEND_PARSE_PARAMETERS_END
Z_TYPE_P
Z_LVAL_P
UNEXPECTED
ZEND_ASSERT
RETURN_DOUBLE
RETURN_LONG
RETURN_THROWS
While .php and .phpt and whatever formatting can be implemented in the future,
I would prefer baby steps, once we have *.[ch] accepted, improving it
with .php/whatever
should be easier in the future.
>
> I haven't looked at your PR in detail, so I'm not sure which of these
> points it satisfies. It would be great if you could quickly describe
> how it works, and what the goals are.
>
> Essentially, I'm just sceptical that this isn't more trouble than it's worth.
>
> Ilija
Here's how it works, roughly:
for pull requests:
- creates a new dedicated CI job specifically to check for formatting errors
- If formatting errors are detected, the CI fails, with the last
message being a command to fix the styling issue, it looks like, and I
quote:
```
please run
curl https://termbin.com/zdzn | git apply -v
```
running that command, followed by git commit+git push should make the CI happy,
git apply will not stage/commit/push automatically however, so in
practice people will have to manually do:
curl https://termbin.com/zdzn | git apply -v;
(optionally run a `git diff` to just check that it looks right)
git commit -a --message 'formatting';
git push;
- we could add the last 2 steps to the actual error message, to make
all 3 steps a copypasta.. would not be opposed.
More technically, it
- creates a new dedicated CI job specifically to check for formatting errors
- Ubuntu 22.04, clang-format-diff, php-cli, git, netcat
- creates a diff between current PR code and master branch
- does *a bunch of tricks/hacks to get the environment suitable for
clang-format-diff* including switching to master,
applying the diff to master, commiting the diff under a fake
username/email (because CI git doesn't have a username/email by
default and git refuse to commit without them)
- actually runs clang-format-diff against the last commit to format
all the code in the last commit
- generate a diff against the changes clang-format-diff just made
- revert the last "fake username/email commit"
- if that clang-format-diff is empty, it just prints "code formatting
check succeeded :)" and the CI finishes successfully
- if it's not empty, it prints, and I quote:
```
+ cat ../formatted.diff
diff --git a/test.c b/test.c
index 6df39be994..64cfab717d 100644
--- a/test.c
+++ b/test.c
@@ -1,5 +1,7 @@
#include
-int main(){
- printf("Hello, World!\n");return 0;
+int main()
+{
+ printf("Hello, World!\n");
+ return 0;
}
\ No newline at end of file
+ echo 'code
Re: [PHP-DEV] RFC Proposal : Allows calling non-static public methods through the __callStatic magic method instead of throwing an error.
For the reference to the others: https://3v4l.org/UmPVo Kind regards, Jorg
Re: [PHP-DEV] automatic formatting checks for pull requests?
On 17 February 2024 22:18:05 GMT, Ilija Tovilo wrote: >Hi Hans > >On Sat, Feb 17, 2024 at 3:31 PM Gina P. Banyard wrote: >> >> On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan >> wrote: >> >> > Can we add automatic formatting checks for pull requests? >> > Made a PR: https://github.com/php/php-src/pull/13417 >> >> It would be nice to have some formatting rules to harmonize the codebase as >> it is somewhat the wild west, >> but as far as my understanding goes is that Clang format struggles to >> understand our codebase (namely macros) and is difficult to set-up for >> php-src. > >Right. Consistent code style is nice, but what we have now is really >not that bad. There are a couple things I'd want if we enforce code >style: > >* Fixing the style should be easy, running a single command without >first pushing to CI. >* It should be fast too, so that I can easily run it for every commit, >preferably even on-save in my editor. >* The new code style should be applied only to newly added sections or >changed code, not entire files. Otherwise, we'll have many changes in >large files, with endless merge conflicts when merging up from lower >branches. >* The formatting tool should work for all php-src code, not just plain >C code. We don't want to be forced to refactor old macros just because >we need to add a single line to some long-standing code. Last time I >tried clang-format, it utterly failed with our macros. > >I haven't looked at your PR in detail, so I'm not sure which of these >points it satisfies. It would be great if you could quickly describe >how it works, and what the goals are. > >Essentially, I'm just sceptical that this isn't more trouble than it's worth. > >Ilija IMO, clang-format isn't really suitable. Its untunable style is often far from the coding style that we currently have, and it makes some really odd choices as to when and how to wrap lines, making code definitely less readable. cheers Derick
[PHP-DEV] New ext-dom features
Hi internals After (and if) my current in-voting RFC about spec-compliance DOM passes, some features become more easily possible to implement due to the necessary building blocks becoming in place in the extension code. I would like to start implementing those. Therefore, I'd like to gauge the opinion of the community on the following two features before making an actual RFC and sitting through the process again. 1) Helper functions for CSS classes (i.e. classList). Requested here: https://github.com/php/php-src/issues/11688 This feature would add the DOMTokenList class along with the classList property to the Element class. It exists to ease the managing of CSS classes on elements, i.e. it deals with whitespace, duplicate class names, etc all for you using a simple set-like API. I expect that implementing this into the DOM extension is not too difficult, as the spec steps are really straight-forward [1] and we just have to reuse the internal APIs to manage attributes. 2) Built-in CSS selector API support In particular, Element::querySelector(All), Element::matches, and Element::closest support. For "old DOM" there is a trick to at least implement querySelector(All) in userland, and that is to translate the selector to XPath and use XPath queries internally. However, that's slower and more cumbersome than a native implementation. It's also just silly to not have this natively available since it's so commonly used. I have a prototype implementation of this already actually, because I got bored while fixing spec-compliance issues and wanted some funner stuff to do. It's based on Lexbor's CSS support. Kind regards Niels [1] https://dom.spec.whatwg.org/#interface-domtokenlist
Re: [PHP-DEV] automatic formatting checks for pull requests?
Hi Hans On Sat, Feb 17, 2024 at 3:31 PM Gina P. Banyard wrote: > > On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan > wrote: > > > Can we add automatic formatting checks for pull requests? > > Made a PR: https://github.com/php/php-src/pull/13417 > > It would be nice to have some formatting rules to harmonize the codebase as > it is somewhat the wild west, > but as far as my understanding goes is that Clang format struggles to > understand our codebase (namely macros) and is difficult to set-up for > php-src. Right. Consistent code style is nice, but what we have now is really not that bad. There are a couple things I'd want if we enforce code style: * Fixing the style should be easy, running a single command without first pushing to CI. * It should be fast too, so that I can easily run it for every commit, preferably even on-save in my editor. * The new code style should be applied only to newly added sections or changed code, not entire files. Otherwise, we'll have many changes in large files, with endless merge conflicts when merging up from lower branches. * The formatting tool should work for all php-src code, not just plain C code. We don't want to be forced to refactor old macros just because we need to add a single line to some long-standing code. Last time I tried clang-format, it utterly failed with our macros. I haven't looked at your PR in detail, so I'm not sure which of these points it satisfies. It would be great if you could quickly describe how it works, and what the goals are. Essentially, I'm just sceptical that this isn't more trouble than it's worth. Ilija
Re: [PHP-DEV] RFC Proposal : Allows calling non-static public methods through the __callStatic magic method instead of throwing an error.
> Le 17 févr. 2024 à 16:51, Larry Garfield a écrit :
>
> On Fri, Feb 16, 2024, at 7:56 PM, 하늘아부지 wrote:
>> Hi.
>> I'd like to propose an RFC, but I don't have the authority.
>> Below is my suggestion.
>> If you think this makes sense, please write an RFC for me.
>> Sincerely.
>>
>> --
>>
>> = Introduction =
>> Allows calling non-static public methods through the __callStatic magic
>> method instead of throwing an error.
>>
>> = Proposal =
>>
>> From a conceptual perspective:
>> It goes without saying that calling a non-static method statically will
>> result in an error.
>> However, a confusing situation occurred when the __callStatic magic
>> method exists.
>> Non-public methods can be called, but public methods cannot.
>> This is the opposite of the method visibility policy.
>>
>> From a practical point of view:
>> If you can call Non-static public methods through the __callStatic
>> magic method, you can write code like Laravel ORM more simply and tidy.
>>
>>
>> User::foo()->bar();
>>
>>
>> Before
>>
>>
>> class Foo
>> {
>>protected static ?Foo $instance = null;
>>
>>public static function __callStatic($method, $args)
>>{
>>$instance = self::$instance ?? self::$instance = new static();
>>return $instance->__call($method, $args);
>>}
>>
>>public function __call($method, $args)
>>{
>>if (method_exists($this, $method)) {
>>return $instance->$method(...$args);
>>}
>>
>>return $this;
>>}
>>
>>protected function bar()
>>{
>>echo __METHOD__ . '';
>>
>>return $this;
>>}
>>
>>protected function baz()
>>{
>>echo __METHOD__ . '';
>>
>>return $this;
>>}
>> }
>>
>> Foo::bar()->baz();
>> (new Foo())->bar()->baz();
>>
>>
>> There is no error, but the concept of method visibility is broken.
>> All Non-public methods can be called at instance scope.
>>
>> After
>>
>>
>> class Foo
>> {
>>protected static ?Foo $instance = null;
>>
>>public static function __callStatic($method, $args)
>>{
>>$instance = self::$instance ?? self::$instance = new static();
>>
>>if (method_exists($instance, $method)) {
>>return $instance->$method(...$args);
>>}
>>
>>return $instance;
>>}
>>
>>public function bar()
>>{
>>echo __METHOD__ . '';
>>
>>return $this;
>>}
>>
>>public function baz()
>>{
>>echo __METHOD__ . '';
>>
>>return $this;
>>}
>> }
>>
>> Foo::bar()->baz();
>> (new Foo())->bar()->baz();
>>
>>
>> This is more tidy.
>> Only public methods are callable at instance scope.
>
> That calling bar() works at all in this example is rather accidental. The
> whole point of a non-static method is that it has an implicit required $this
> argument to it. Without a required argument, a method cannot be called,
> because it is supposed to fail.
>
> In fact, even your "before" example fails: https://3v4l.org/moH0s
It does work after having corrected the obvious bug in the `bar()` method.
>
> I don't think what you describe is even possible, nor would it be a good idea.
Why wouldn’t it possible? Couldn’t the engine invoke __callStatic() instead of
complaining that the targeted method is not static? (I don’t have opinion on
whether it is a good idea, though)
—Claude
Re: [PHP-DEV] automatic formatting checks for pull requests?
On Sat, Feb 17, 2024, 15:27 Gina P. Banyard wrote: > On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan < > h...@loltek.net> wrote: > > > Can we add automatic formatting checks for pull requests? > > Made a PR: https://github.com/php/php-src/pull/13417 > > > > php-src use "tabs" instead of "spaces", that is... quite unusual, > > and I'm probably not the first person to accidentally use spaces > > instead of tabs, ref > > > https://github.com/php/php-src/pull/13401/files/d64a8ccdc1d21576827059ee86c0fa073c95ffcc#r1492699756 > > That's quite standard in the C world AFAIK, yes it's not what is standard > for PHP written code but that's kinda irrelevant. > > It would be nice to have some formatting rules to harmonize the codebase > as it is somewhat the wild west, > but as far as my understanding goes is that Clang format struggles to > understand our codebase (namely macros) and is difficult to set-up for > php-src. > > Best regards, > > Gina P. Banyard > It worked great with the macros I've tested so far: ZEND_PARSE_PARAMETERS_START Z_PARAM_NUMBER ZEND_PARSE_PARAMETERS_END Z_TYPE_P Z_LVAL_P UNEXPECTED ZEND_ASSERT RETURN_DOUBLE RETURN_LONG RETURN_THROWS Is there any specific macros I can test?
[PHP-DEV] Re: Testing new list server
On Feb 14, 2024, at 11:16 AM, Derick Rethans wrote: > > Please don't be alarmed, we're moving the lists over to a new machine. I've been getting a ton of spam from php-general@ recently. I suspect the changeover might be related.
Re: [PHP-DEV] Re: [RFC] OOP API for cURL extension
On Thu, Feb 15, 2024, at 9:44 AM, Sara Golemon wrote: > Summarizing replies so far. Won't be able to update the RFC > immediately as my day job needs me, but some great discussions already, > gang. Thanks! > > * Define the conditions under which exceptions will be thrown (and > which exceptions) - I'll add these to the RFC, but in short: > * CurlException - Never, it's an interface type to group the other > exceptions. > * CurlHandleException - Whenever a CurlHandle::method() fails (in > lieu of returning false) > * CurlMultiException - Same, but for the CurlMultiHandle class. > * CurlShareException - Same, but for the CurlShareHandle class. > > * Naming styles, e.g getErrorNumber(): int vs errno() > Comment: Agreed with your points. We don't have to stick to a hard > line mapping of the function names. My initial translation did because > my bugbear is with the lack of fluency and all the rest is just window > dressing on that. > Proposal: I'll rename all the new APIs according to a get*/set* > scheme without abbreviations, e.g.: > * CurlHandle::getErrorNumber(): int > * CurlHandle::getError(): ?string > * static CurlHandle::getErrorTextFromCode(int $code): ?string > * CurlHandle::execute(): ?string // See late note about exec return > values > * CurlHandle::setOptionInt(int $option, int $value): CurlHandle > > * Better typing for setOpt() methods. > Comment: Yep, this is a good and fair point. It's going to take a > little more dicing up of the current implementation, but hopefully not > too rough. > Proposal: Keep untyped setOption() method (1/ Easier migration of > existing code, 2/ Some options may prove difficult to type), but add: > * CurlHandle::setOptionBool(int $option, bool $value): CurlHandle > * CurlHandle::setOptionInt(int $option, int $value): CurlHandle > * CurlHandle::setOptionString(int $option, string $value): CurlHandle > * etc... as needed, will this get weird when we get to Array since > there IS a setOptArray? Maybe we just don't mirror that one, or we > call it setOptionMany(). Yeah, I like Many for the multiple option set, > and Array for the single option of array type. > Internally, the setopt handler will be split by type so that each > typed setting can call explicitly, and the untyped one can call all. > > * CurlHandle::exec() mixed typing of return values. > Comment: Agreed. The `true` return value becomes meaningless in the > RETURNTRANSFER==false case. > Proposal: Update the RFC for CurlHandle::execute() to return ?string. > > * https://php.watch/articles/php-curl-security-hardening > Comment: I'll read this later when I'm updating the RFC. > > * Prefer class constants to global constants. > Comment: I'm less compelled by this. The global scope is polluted > with the constants whether we add copies or not. Adding a second way > to spell the constant name only adds a second way to spell the constant > name. > Proposal: Change my mind? > > * Request and Response objects, along the lines of PSR-18 > Comment: I'd be in favor of that, but it's not the mountain I'm > personally trying to climb today. No offense taken if you vote no > because this isn't enough, but I don't have the cycles to go in that > hard. >Proposal: Write an RFC (and get it passed) and I can probably help > you implement it? > > -Sara Obligatory: This seems like something that could be done in user-space as a wrapper around Curl. (That's basically what many HTTP clients are.) Why should this be done in C? The reasoning for that needs to be included in the RFC. The RFC would also benefit greatly from some practical examples of using the new API. Right now it's not clear to me (as someone who almost never uses Curl directly) how/why I'd use any of these, since there's still "a whole crapton of int constants I don't understand floating around." The suggestion to use an Enum (or several) here is a good one and would help a lot with that, so I'm +1 there. Overall I'm not opposed, but there's more fleshing that is needed before it's ready. (Which seems like it's happening based on Sara's response above, so that's good. --Larry Garfield
Re: [PHP-DEV] RFC Proposal : Allows calling non-static public methods through the __callStatic magic method instead of throwing an error.
On Fri, Feb 16, 2024, at 7:56 PM, 하늘아부지 wrote:
> Hi.
> I'd like to propose an RFC, but I don't have the authority.
> Below is my suggestion.
> If you think this makes sense, please write an RFC for me.
> Sincerely.
>
> --
>
> = Introduction =
> Allows calling non-static public methods through the __callStatic magic
> method instead of throwing an error.
>
> = Proposal =
>
> From a conceptual perspective:
> It goes without saying that calling a non-static method statically will
> result in an error.
> However, a confusing situation occurred when the __callStatic magic
> method exists.
> Non-public methods can be called, but public methods cannot.
> This is the opposite of the method visibility policy.
>
> From a practical point of view:
> If you can call Non-static public methods through the __callStatic
> magic method, you can write code like Laravel ORM more simply and tidy.
>
>
> User::foo()->bar();
>
>
> Before
>
>
> class Foo
> {
> protected static ?Foo $instance = null;
>
> public static function __callStatic($method, $args)
> {
> $instance = self::$instance ?? self::$instance = new static();
> return $instance->__call($method, $args);
> }
>
> public function __call($method, $args)
> {
> if (method_exists($this, $method)) {
> return $instance->$method(...$args);
> }
>
> return $this;
> }
>
> protected function bar()
> {
> echo __METHOD__ . '';
>
> return $this;
> }
>
> protected function baz()
> {
> echo __METHOD__ . '';
>
> return $this;
> }
> }
>
> Foo::bar()->baz();
> (new Foo())->bar()->baz();
>
>
> There is no error, but the concept of method visibility is broken.
> All Non-public methods can be called at instance scope.
>
> After
>
>
> class Foo
> {
> protected static ?Foo $instance = null;
>
> public static function __callStatic($method, $args)
> {
> $instance = self::$instance ?? self::$instance = new static();
>
> if (method_exists($instance, $method)) {
> return $instance->$method(...$args);
> }
>
> return $instance;
> }
>
> public function bar()
> {
> echo __METHOD__ . '';
>
> return $this;
> }
>
> public function baz()
> {
> echo __METHOD__ . '';
>
> return $this;
> }
> }
>
> Foo::bar()->baz();
> (new Foo())->bar()->baz();
>
>
> This is more tidy.
> Only public methods are callable at instance scope.
That calling bar() works at all in this example is rather accidental. The
whole point of a non-static method is that it has an implicit required $this
argument to it. Without a required argument, a method cannot be called,
because it is supposed to fail.
In fact, even your "before" example fails: https://3v4l.org/moH0s
I don't think what you describe is even possible, nor would it be a good idea.
See also: https://peakd.com/hive-168588/@crell/cutting-through-the-static
--Larry Garfield
Re: [PHP-DEV] RE: Testing new list server
On Fri, 16 Feb 2024 at 23:50, Jorg Sowa wrote: > Hello Derick, > there is something wrong. I don't get all of the emails from the new > setup, only part. Examples of emails I didn't receive: > - https://externals.io/message/122391 > - https://externals.io/message/122390 > - https://externals.io/message/122388 > > I'm using Gmail and Spam doesn't contain any of them. > > Kind regards, > Jorg > Same, I did not receive messages - 122402 - 122372 - 122376
Re: [PHP-DEV] Re: [RFC] OOP API for cURL extension
On Saturday, 17 February 2024 at 07:41, Kalle Sommer Nielsen
wrote:
> Hi Sara
>
> Den tors. 15. feb. 2024 kl. 21.08 skrev Sara Golemon poll...@php.net:
>
> > * Better typing for setOpt() methods.
> > Comment: Yep, this is a good and fair point. It's going to take a little
> > more dicing up of the current implementation, but hopefully not too rough.
> > Proposal: Keep untyped setOption() method (1/ Easier migration of existing
> > code, 2/ Some options may prove difficult to type), but add:
> > * CurlHandle::setOptionBool(int $option, bool $value): CurlHandle
> > * CurlHandle::setOptionInt(int $option, int $value): CurlHandle
> > * CurlHandle::setOptionString(int $option, string $value): CurlHandle
> > * etc... as needed, will this get weird when we get to Array since there IS
> > a setOptArray? Maybe we just don't mirror that one, or we call it
> > setOptionMany(). Yeah, I like Many for the multiple option set, and Array
> > for the single option of array type.
> > Internally, the setopt handler will be split by type so that each typed
> > setting can call explicitly, and the untyped one can call all.
>
>
> What about making the CURL options an enumeration? It would allow us
> to typehint the setOptions() method and we can also make it backwards
> compatible with the existing global constants:
> `php public function setOption(CurlOption|int $option, mixed $value): static
> { if (is_int($option)) { $option = CurlOption::createFromConstant($option); }
> // ... }`
>
> The enumeration would then have a static method to transform the
> global constant into a native type (which we can do internally).
> Naming is obvious just TBD. The biggest potential issue I can see with
> this approach is the many conditionally defined constants from older
> CURL versions/feature flags from compile time.
>
> From the many type variants of setOptions(), I am uncertain about
> that, because with an enumeration, we would still need to have some
> sort of whitelisting of what options can be passed into the method.
>
I was also going to suggest to use enums for the options, and have them be
grouped by what value type they need.
This would simplify the C implementation a lot, as most of the logic to handle
the options and the value type would be done by the engine and ZPP.
I don't think it is _that_ problematic to have enum cases be conditionally
defined as trying to use a case/constant that does not exist already throws an
Error in PHP 8 if the constant does not exist.
I will note that this approach _technically_ breaks the handling of options
that require a callables, as the callable is checked to exist _when_ attempting
to call it and not when actually passing it to curl_setopt.
But this should change anyway if I merge my PR which "fixes" this. [1]
Best regards,
Gina P. Banyard
[1] https://github.com/php/php-src/pull/13291
Re: [PHP-DEV] automatic formatting checks for pull requests?
On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan wrote: > Can we add automatic formatting checks for pull requests? > Made a PR: https://github.com/php/php-src/pull/13417 > > php-src use "tabs" instead of "spaces", that is... quite unusual, > and I'm probably not the first person to accidentally use spaces > instead of tabs, ref > https://github.com/php/php-src/pull/13401/files/d64a8ccdc1d21576827059ee86c0fa073c95ffcc#r1492699756 That's quite standard in the C world AFAIK, yes it's not what is standard for PHP written code but that's kinda irrelevant. It would be nice to have some formatting rules to harmonize the codebase as it is somewhat the wild west, but as far as my understanding goes is that Clang format struggles to understand our codebase (namely macros) and is difficult to set-up for php-src. Best regards, Gina P. Banyard
Re: [PHP-DEV] Requesting RFC karma
On Sat, 17 Feb 2024 at 08:22, Hans Henrik Bergan wrote: > > My name is "Hans Henrik Bergan", usually go by the nickname > "divinity76", I've contributed to OSS (including PHP) for years, and > am currently involved in 3 things that might require an RFC, and > requesting RFC karma for wiki account "divinity76". > > 1/3: adding BLAKE3 to PHP, a *very* fast cryptographically secure > hash: https://github.com/php/php-src/pull/13194 > > 2/3: int|float for sleep, sleep(0.1) => sleep 0.1 seconds: > https://github.com/php/php-src/pull/13401 > > 3/3: int|float for DateTime::setTimestamp, setTimestamp(0.123456) => > 1970-01-01 00:00:00.123456 : https://github.com/php/php-src/pull/13383 and possibly 4/4: automatic code formatting check for pull requests: https://github.com/php/php-src/pull/13417
Re: [PHP-DEV] Re: [RFC] OOP API for cURL extension
Hi Sara
Den tors. 15. feb. 2024 kl. 21.08 skrev Sara Golemon :
> * Better typing for setOpt() methods.
> Comment: Yep, this is a good and fair point. It's going to take a little
> more dicing up of the current implementation, but hopefully not too rough.
> Proposal: Keep untyped setOption() method (1/ Easier migration of existing
> code, 2/ Some options may prove difficult to type), but add:
> * CurlHandle::setOptionBool(int $option, bool $value): CurlHandle
> * CurlHandle::setOptionInt(int $option, int $value): CurlHandle
> * CurlHandle::setOptionString(int $option, string $value): CurlHandle
> * etc... as needed, will this get weird when we get to Array since there IS
> a setOptArray? Maybe we just don't mirror that one, or we call it
> setOptionMany(). Yeah, I like Many for the multiple option set, and Array for
> the single option of array type.
> Internally, the setopt handler will be split by type so that each typed
> setting can call explicitly, and the untyped one can call all.
What about making the CURL options an enumeration? It would allow us
to typehint the setOptions() method and we can also make it backwards
compatible with the existing global constants:
```php
public function setOption(CurlOption|int $option, mixed $value): static
{
if (is_int($option)) {
$option = CurlOption::createFromConstant($option);
}
// ...
}
```
The enumeration would then have a static method to transform the
global constant into a native type (which we can do internally).
Naming is obvious just TBD. The biggest potential issue I can see with
this approach is the many conditionally defined constants from older
CURL versions/feature flags from compile time.
>From the many type variants of setOptions(), I am uncertain about
that, because with an enumeration, we would still need to have some
sort of whitelisting of what options can be passed into the method.
--
regards,
Kalle Sommer Nielsen
ka...@php.net
Re: [PHP-DEV] int|float for sleep? sleep(0.1) => sleep 0.1 seconds
On Fri, 16 Feb 2024 at 09:36, Alexandru Pătrănescu wrote: > > > On Fri, Feb 16, 2024 at 10:18 AM Hans Henrik Bergan wrote: >> >> Can we make sleep accept int|float? >> Made a PR: https://github.com/php/php-src/pull/13401 >> >> For years when I wanted to sleep for 0.1 seconds, it annoyed me that I >> couldn't do >> `sleep(0.1);` >> instead I had to do >> `usleep(figure out how many microseconds there are in 0.1 seconds and put it >> here);` >> > > Based on previous discussions https://externals.io/message/111448 > it seems the desired way was for a RFC to be created for this change, as it > involves some small changes that break BC. > > There is also the PR done by Michael Voříšek from 3.5 years ago where some > technical discussions took place: > https://github.com/php/php-src/pull/5961 > > Regards, > Alex > Interesting indeed. I implemented some improvements from that PR now (higher precision sleep and fixing the windows 192-issue), and got Michael Voříšek's attention :) also applied for RFC Karma. On Fri, 16 Feb 2024 at 14:20, Claude Pache wrote: > > Hi, > > > FWIW Python's `time.sleep` also works like this: > https://docs.python.org/3/library/time.html#time.sleep > > > Python also implements the following: > > If the sleep is interrupted by a signal and no exception is raised by the > signal handler, the sleep is restarted with a recomputed timeout. > > > I think we should also implement that. As a consequence, `sleep(...)` will > always return `0`. > > If we “fix” sleep(), let’s “fix” it completely. > > —Claude it seems we already have a function for that, it's called `time_sleep_until()`, quoting the documentation >Note: All signals will be delivered after the script wakes up. in contrast to normal sleep() that is signal interruptible. Sorry, I'm not interested in making sleep() uninterruptible, at least not until after I get int|float accepted upstream.
Re: [PHP-DEV] RE: Testing new list server
Hi On Sat, Feb 17, 2024 at 12:17 AM Jorg Sowa wrote: > > Hello Derick, > there is something wrong. I don't get all of the emails from the new setup, > only part. Examples of emails I didn't receive: > - https://externals.io/message/122391 > - https://externals.io/message/122390 > - https://externals.io/message/122388 > > I'm using Gmail and Spam doesn't contain any of them. Same here. I'm using Gmail and have not received various e-mails. Ilija
[PHP-DEV] Re: [RFC] OOP API for cURL extension
On 16 February 2024 16:09:32 GMT, Rowan Tommins wrote: >public function executeAndReturn(): string >public function executeAndOutput(): void I guess I missed: public function executeToFile(Stream $fileHandle): void public function executeWithCallback(callable $wrIteFunction): void which would imply CURLOPT_FILE and CURLOPT_WRITEFUNCTION, respectively. From what I can see, these four modes are actually mutually exclusive (populating ch->handlers.write->method) with whichever option is touched last governing the actual behaviour of curl_exec(). For instance, setting CURLOPT_FILE to null or CURLOPT_RETURNTRANSFER to false always selects stdout mode, effectively clearing any value set with CURLOPT_WRITEFUNCTION. Having separate execute methods would make that much more obvious. Incidentally, I notice there is currently some code in _php_curl_verify_handlers where a bad stream in CURLOPT_FILE will fall back to writing the result to stdout. Is it me, or is that a really terrible idea, potentially exposing private data to the user? Should that scenario be promoted to an immediate false return in curl_exec, and Error in the new OO wrapper? Regards, -- Rowan Tommins [IMSoP]
[PHP-DEV] automatic formatting checks for pull requests?
Can we add automatic formatting checks for pull requests? Made a PR: https://github.com/php/php-src/pull/13417 php-src use "tabs" instead of "spaces", that is... quite unusual, and I'm probably not the first person to accidentally use spaces instead of tabs, ref https://github.com/php/php-src/pull/13401/files/d64a8ccdc1d21576827059ee86c0fa073c95ffcc#r1492699756
Re: [PHP-DEV] Requesting RFC karma
Hi Hans On 16.02.24 13:05, Hans Henrik Bergan wrote: My name is "Hans Henrik Bergan", usually go by the nickname "divinity76", I've contributed to OSS (including PHP) for years, and am currently involved in 3 things that might require an RFC, and requesting RFC karma for wiki account "divinity76". RFC karma was granted, good luck! Ilija
[PHP-DEV] R: [RFC] OOP API for cURL extension
Hi Sara,
i like this proposal.
Silvio
From: Sara Golemon
Sent: Wednesday, February 14, 2024 7:47 PM
To: PHP internals
Subject: [RFC] OOP API for cURL extension
Good afternoon folks, I'd like to open discussion on adding OOP APIs to the
cURL extension.
https://wiki.php.net/rfc/curl-oop
This has been a long standing bug-bear of mine, and I think its time has come.
try {
(new \CurlHandle)->setOpt(YOUR_VOTE, true)->exec();
} catch (\CurlHandleException $ex) {
assert(false); // Why not?!
}
-Sara
[PHP-DEV] RFC Proposal : Allows calling non-static public methods through the __callStatic magic method instead of throwing an error.
Hi.
I'd like to propose an RFC, but I don't have the authority.
Below is my suggestion.
If you think this makes sense, please write an RFC for me.
Sincerely.
--
= Introduction =
Allows calling non-static public methods through the __callStatic magic
method instead of throwing an error.
= Proposal =
>From a conceptual perspective:
It goes without saying that calling a non-static method statically will
result in an error.
However, a confusing situation occurred when the __callStatic magic method
exists.
Non-public methods can be called, but public methods cannot.
This is the opposite of the method visibility policy.
>From a practical point of view:
If you can call Non-static public methods through the __callStatic magic
method, you can write code like Laravel ORM more simply and tidy.
User::foo()->bar();
Before
class Foo
{
protected static ?Foo $instance = null;
public static function __callStatic($method, $args)
{
$instance = self::$instance ?? self::$instance = new static();
return $instance->__call($method, $args);
}
public function __call($method, $args)
{
if (method_exists($this, $method)) {
return $instance->$method(...$args);
}
return $this;
}
protected function bar()
{
echo __METHOD__ . '';
return $this;
}
protected function baz()
{
echo __METHOD__ . '';
return $this;
}
}
Foo::bar()->baz();
(new Foo())->bar()->baz();
There is no error, but the concept of method visibility is broken.
All Non-public methods can be called at instance scope.
After
class Foo
{
protected static ?Foo $instance = null;
public static function __callStatic($method, $args)
{
$instance = self::$instance ?? self::$instance = new static();
if (method_exists($instance, $method)) {
return $instance->$method(...$args);
}
return $instance;
}
public function bar()
{
echo __METHOD__ . '';
return $this;
}
public function baz()
{
echo __METHOD__ . '';
return $this;
}
}
Foo::bar()->baz();
(new Foo())->bar()->baz();
This is more tidy.
Only public methods are callable at instance scope.
Re: [PHP-DEV] int|float for sleep? sleep(0.1) => sleep 0.1 seconds
On Fri, Feb 16, 2024 at 10:18 AM Hans Henrik Bergan wrote: > Can we make sleep accept int|float? > Made a PR: https://github.com/php/php-src/pull/13401 > > For years when I wanted to sleep for 0.1 seconds, it annoyed me that I > couldn't do > `sleep(0.1);` > instead I had to do > `usleep(figure out how many microseconds there are in 0.1 seconds and put > it here);` > > Based on previous discussions https://externals.io/message/111448 it seems the desired way was for a RFC to be created for this change, as it involves some small changes that break BC. There is also the PR done by Michael Voříšek from 3.5 years ago where some technical discussions took place: https://github.com/php/php-src/pull/5961 Regards, Alex
Re: [PHP-DEV] int|float for sleep? sleep(0.1) => sleep 0.1 seconds
Hi, > > FWIW Python's `time.sleep` also works like this: > https://docs.python.org/3/library/time.html#time.sleep Python also implements the following: > If the sleep is interrupted by a signal and no exception is raised by the > signal handler, the sleep is restarted with a recomputed timeout. > I think we should also implement that. As a consequence, `sleep(...)` will always return `0`. If we “fix” sleep(), let’s “fix” it completely. —Claude
[PHP-DEV] Re: [RFC] OOP API for cURL extension
On 15 February 2024 15:44:13 GMT, Sara Golemon wrote: >* CurlHandle::exec() mixed typing of return values. > Comment: Agreed. The `true` return value becomes meaningless in the >RETURNTRANSFER==false case. > Proposal: Update the RFC for CurlHandle::execute() to return ?string. Should we take this a step further, and remove CURLOPT_RETURNTRANSFER as a valid option on the object completely? Instead of an overloaded exec() method, provide: public function executeAndReturn(): string public function executeAndOutput(): void Perhaps the option could be accepted in the relevant setOpt methods, but issue a warning that it has no effect. Since both the default for the option and the name of the method are changing anyway, I don't think this significantly affects the migration effort for the tiny minority of cases where you actually want the direct output behaviour. Regards, -- Rowan Tommins [IMSoP]
Re: [PHP-DEV] [RFC] Deprecate implicitly nullable parameter type
Hi Nicolas, Thanks for your input and proactive measures! I've just finished the analysis you asked for: - code: https://gist.github.com/kocsismate/c0d10c820606dfe297f09374aa634df5 - results: https://gist.github.com/kocsismate/cf3bdfbf35eb10224ee5ecd29b39656b TLDR: there are 880 packages out of 2000 which use implicit nullable parameter types. We will mention this fact in the RFC soon, as well as the need for additionally taking care of non-optional arguments with default values. Regards, Máté
Re: [PHP-DEV] Registration apply for php wiki
Hi JaeHan! On Fri, Feb 16, 2024 at 10:53 AM 하늘아부지 wrote: > > Hi, My name is JaeHan Seo. (wiki username is daddyofsky) > > I hope I can make a proposal by registering on the php wiki. I can give you karma. Usually, RFC karma (which is what I'm guessing you're asking for) is granted for specific RFC ideas. Could you quickly summarize what your idea is? See step 1 in the RFC howto: https://wiki.php.net/rfc/howto Thank you! Ilija
