Hi Internals,
I created my first php-src PR
https://github.com/php/php-src/pull/13845 yesterday and anticipate I
may have to write an RFC for it. I created a wiki account under the
handle, "bilge", and the howto https://wiki.php.net/rfc/howto told me
to request RFC karma here.
About the PR: I sometimes find it would be useful to only update part of
the date. The PR makes all parameters to DateTime(Immutable)::setDate
https://www.php.net/manual/en/datetimeimmutable.setdate.php optional
in a backwards-compatible manner such that we can elect to update only
the day, month, year or any combination of the three (thanks, in part,
to named parameters). Without this modification, we must always specify
all of the day, month and year parameters to change the date.
To change just one part of the date, StackOverflow users have found
various novel
https://stackoverflow.com/questions/60415815/setting-just-the-year-with-php-datetime
ways
https://stackoverflow.com/questions/18176794/adjust-a-php-date-to-the-current-year
to work around setDate()'s current limitation, which generally involves
calling format() twice, to isolate the specific date parts that should
remain constant, and feeding them back into setDate() so they are
effectively unchanged. All this leads me to wonder why we can't just
call setDate() with only the parameters we want to change. This is
particularly useful when generating a collection of dates that only
differ in one aspect (year, day) and/or when working in a templating
environment, such as Twig, where doing all the format calls results in
multiple statements and logic overhead that we should like to avoid in
templating contexts.
Looking forward to hearing your thoughts.
Cheers,
Bilge
Hi,
About the PR: I sometimes find it would be useful to only update part of the
date. The PR makes all parameters to DateTime(Immutable)::setDate
https://www.php.net/manual/en/datetimeimmutable.setdate.php optional in a
backwards-compatible manner such that we can elect to update only the day,
month, year or any combination of the three (thanks, in part, to named
parameters). Without this modification, we must always specify all of the day,
month and year parameters to change the date.
As I mentioned to you in Room 11, I am not in favour of adhoc API
changes to Date/Time classes. It has now been nearly 18 years since they
were originally introduced, and they indeed could do with an overhaul.
I have been colllecting ideas in
https://docs.google.com/document/d/1pxPSRbfATKE4TFWw72K3p7ir-02YQbTf3S3SIxOKWsk/edit#heading=h.2jol7kfhmijb
Having different/better modifiers would also be a good thing to talk
about, albeit perhaps on the four mentioned new classes, instead of
adding them to the already existing DateTime and DateTimeImmutable
classes.
In any case, just allowing setDate to be able to just modify the month
is going to introduce confusion, as this will be counter intuitive:
$dt = new DateTimeImmutable("2024-03-31");
$newDt = $dt->setDate( month: 2 );
It is now representing 2024-03-02.
This might be the right answer, but it might also be that the developer
just cared about the month part (and not the day-of-month), in which
case this is a WTF moment.
Picking mofication APIs is not as trivial as it seems, and I would like
to do it right.
Feel free to add comments and wishes to the google doc document. In the
near future, I will be writing up an RFC from this.
cheers,
Derick
--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
mastodon: @derickr@phpc.social @xdebug@phpc.social
Hi,
About the PR: I sometimes find it would be useful to only update part of the
date. The PR makes all parameters to DateTime(Immutable)::setDate
https://www.php.net/manual/en/datetimeimmutable.setdate.php optional in a
backwards-compatible manner such that we can elect to update only the day,
month, year or any combination of the three (thanks, in part, to named
parameters). Without this modification, we must always specify all of the day,
month and year parameters to change the date.
As I mentioned to you in Room 11, I am not in favour of adhoc API
changes to Date/Time classes. It has now been nearly 18 years since they
were originally introduced, and they indeed could do with an overhaul.I have been colllecting ideas in
https://docs.google.com/document/d/1pxPSRbfATKE4TFWw72K3p7ir-02YQbTf3S3SIxOKWsk/edit#heading=h.2jol7kfhmijbHaving different/better modifiers would also be a good thing to talk
about, albeit perhaps on the four mentioned new classes, instead of
adding them to the already existing DateTime and DateTimeImmutable
classes.In any case, just allowing setDate to be able to just modify the month
is going to introduce confusion, as this will be counter intuitive:$dt = new DateTimeImmutable("2024-03-31");
$newDt = $dt->setDate( month: 2 );It is now representing 2024-03-02.
This might be the right answer, but it might also be that the developer
just cared about the month part (and not the day-of-month), in which
case this is a WTF moment.Picking mofication APIs is not as trivial as it seems, and I would like
to do it right.Feel free to add comments and wishes to the google doc document. In the
near future, I will be writing up an RFC from this.cheers,
Derick
Hi Derick,
Thanks for your reply!
Indeed, as per your code snippet, this is a WTF moment I had not
accounted for and confirm the same result with my patch applied.
Generally, my expectation here would be the month must be set to 2, so
if the day portion will be invalidated by that change, we should either
throw an exception or implicitly coerce it into range, i.e. (new
DateTime("2024-03-31"))->setDate(month: 2); == 2024-02-29. However, I
suppose this is not the conversation we're having as you do not wish to
change this API at all, which I respect.
Regarding your brainstorm document, I can't understand much of it in its
current state, and as I am not a subject matter expert, I think you will
receive much better feedback from others. In particular, I cannot glean
which four classes you are referring to in that document. Yet what I do
find interesting is the notion of adding setters to DateTimeImmutable.
For my particular use-case—producing a collection of dates incrementing
by year in a Twig template—a trivial year setter would do just fine,
with the significant caveat that it must implement fluent interface,
because I need to call it in an expression context (returning a value),
not a statement context (executing a void function separately). Not that
Twig cannot execute statements, but it just becomes more verbose,
cumbersome and less template-like.
If you were happy for me to add getters and fluent setters for year, I'd
be happy to work on that PR, but for month we're back to the same
problem outlined in the opening paragraph (and I suppose the same
problem occasionally applies to year, if the day happens to be set to
the leap day). Otherwise, I'll be happy to read over your RFC when it's
ready.
Kind regards, Bilge
About the PR: I sometimes find it would be useful to only update
part of the date. The PR makes all parameters to
DateTime(Immutable)::setDate
https://www.php.net/manual/en/datetimeimmutable.setdate.php
optional in a backwards-compatible manner such that we can elect
to update only the day, month, year or any combination of the
three (thanks, in part, to named parameters). Without this
modification, we must always specify all of the day, month and
year parameters to change the date.
As I mentioned to you in Room 11, I am not in favour of adhoc API
changes to Date/Time classes. It has now been nearly 18 years since
they were originally introduced, and they indeed could do with an
overhaul.I have been colllecting ideas in
https://docs.google.com/document/d/1pxPSRbfATKE4TFWw72K3p7ir-02YQbTf3S3SIxOKWsk/edit#heading=h.2jol7kfhmijbHaving different/better modifiers would also be a good thing to talk
about, albeit perhaps on the four mentioned new classes, instead of
adding them to the already existing DateTime and DateTimeImmutable
classes.In any case, just allowing setDate to be able to just modify the
month is going to introduce confusion, as this will be counter
intuitive:$dt = new DateTimeImmutable("2024-03-31");
$newDt = $dt->setDate( month: 2 );It is now representing 2024-03-02.
This might be the right answer, but it might also be that the
developer just cared about the month part (and not the
day-of-month), in which case this is a WTF moment.Picking mofication APIs is not as trivial as it seems, and I would
like to do it right.Feel free to add comments and wishes to the google doc document. In
the near future, I will be writing up an RFC from this.Thanks for your reply!
Indeed, as per your code snippet, this is a WTF moment I had not accounted for
and confirm the same result with my patch applied. Generally, my expectation
here would be the month must be set to 2, so if the day portion will be
invalidated by that change, we should either throw an exception or implicitly
coerce it into range, i.e. (new DateTime("2024-03-31"))->setDate(month: 2); ==
2024-02-29.
We can't do that, as it will be inconsistent with:
<?php
$d = new DateTimeImmutable("2024-01-31");
$d->modify("+1 month")
// (also 2024-03-02)
However, I suppose this is not the conversation we're having as
you do not wish to change this API at all, which I respect.
I think what you're actually after here, is a "YearMonth" type, which
doesn't concern itself with the day-of-month at all. Setting just the
month should perhaps just reset the day-of-month to 1 instead. ANd
setting just the year would result in January 1st, as well.
Regarding your brainstorm document, I can't understand much of it in its
current state, and as I am not a subject matter expert, I think you will
receive much better feedback from others.
It isn't particularly organised yet, and... I also haven't made sense of
all of it yet.
In particular, I cannot glean which four classes you are referring to
in that document. Yet what I do find interesting is the notion of
adding setters to DateTimeImmutable. For my particular
use-case—producing a collection of dates incrementing by year in a
Twig template—a trivial year setter would do just fine, with the
significant caveat that it must implement fluent interface, because I
need to call it in an expression context (returning a value), not a
statement context (executing a void function separately). Not that
Twig cannot execute statements, but it just becomes more verbose,
cumbersome and less template-like.If you were happy for me to add getters and fluent setters for year,
I'd be happy to work on that PR, but for month we're back to the same
problem outlined in the opening paragraph (and I suppose the same
problem occasionally applies to year, if the day happens to be set to
the leap day). Otherwise, I'll be happy to read over your RFC when
it's ready.
Feel free to add your suggestion (anywhere) in the document, preferrably
at the bottom :-)
I can envision that we'll have some actual setters and getters, beyond
just add, sub, and modify (which is really powerful).
But what is important to understand is rather why a specific method is
useful, and perhaps even more important is what you're actual goal is.
cheers,
Derick
--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
mastodon: @derickr@phpc.social @xdebug@phpc.social