Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[VarDumper] Add casters for object-converted resources#59035

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

alexandre-daubois
Copy link
Member

@alexandre-dauboisalexandre-daubois commentedNov 29, 2024•
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?yes
Issues-
LicenseMIT

This PR makes up for the backlog of resources transformed into objects in the latest versions of PHP.

✅ Support added (or already existing with an update inAbstractCloner):

  • ext-curl
    • CurlMultiHandle:no information to gather
  • ext-openssl
    • OpenSSLCertificateSigningRequest
    • OpenSSLAsymmetricKey
  • ext-finfo
    • No information to gather
  • ext-sqlite3
    • Sqlite3Result
  • ext-sockets
  • ext-pgsql
    • PgSql\Lob
    • PgSql\Connection
    • PgSql\Result

⚠️ Remaining classes I couldn't test/find enough info yet (introduced in PHP 8.4):

  • Odbc\Connection
  • Odbc\Result
  • Soap\Url
  • Soap\Sdl

alamirault and derrabus reacted with thumbs up emoji
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

tedious topic, thanks for working on it

@alexandre-dauboisalexandre-dauboisforce-pushed thecasters-resources branch 3 times, most recently fromed8fdf2 tod382421CompareNovember 29, 2024 12:41
@alexandre-daubois
Copy link
MemberAuthor

Revamped the PR, so most things are still in ResourceCaster now. The regression in PHP has been reported tophp/php-src#16990, and most casters now "simply" map infos to$a.

@alexandre-dauboisalexandre-dauboisforce-pushed thecasters-resources branch 3 times, most recently from9830222 to8ec4800CompareNovember 29, 2024 12:51
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

@internal since 7.3 👍

@alexandre-dauboisalexandre-dauboisforce-pushed thecasters-resources branch 3 times, most recently from34d936d tod0b95fcCompareNovember 29, 2024 14:10
@alexandre-daubois
Copy link
MemberAuthor

Marked all casters as internal and addedUPGRADE-7.3.md.

@alexandre-dauboisalexandre-dauboisforce-pushed thecasters-resources branch 4 times, most recently from5cf62d8 to27a573bCompareNovember 29, 2024 15:15
@alexandre-dauboisalexandre-dauboisforce-pushed thecasters-resources branch 2 times, most recently from4952bc6 to7ea55ceCompareNovember 29, 2024 15:39
@alexandre-daubois
Copy link
MemberAuthor

Solved by silencingsocket_atmark() in the case it cannot be applied on the given socket.

@alexandre-dauboisalexandre-dauboisforce-pushed thecasters-resources branch 3 times, most recently fromec86ba0 to9d65675CompareDecember 2, 2024 09:25
@alexandre-daubois
Copy link
MemberAuthor

Thedba_list() regression fix will be released in PHP 8.4.2:

fabpot added a commit that referenced this pull requestDec 6, 2024
This PR was merged into the 6.4 branch.Discussion----------Add CI job for PHP 8.5| Q             | A| ------------- | ---| Branch?       | 6.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | -| License       | MITEnables PHP 8.5 nightlies on the CI.What motivated this early addition is the continuous effort of converting resources to actual classes. PHP 8.5 already embeds features likehttps://wiki.php.net/rfc/directory-opaque-object, which could already be handled in PRs such as#59035.Also we'll be able to fix future deprecations as soon as they appear during the next year.Commits-------15d56bb Add an experimental CI job for PHP 8.5
{
if (\PHP_VERSION_ID < 80402 && !\is_resource($dba)) {
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

The fix was shipped with PHP 8.4.2.

@alexandre-dauboisalexandre-dauboisforce-pushed thecasters-resources branch 2 times, most recently from392eb5a toece56dbCompareJanuary 7, 2025 11:12
public static function castGd(\GdImage $gd, array $a, Stub $stub, bool $isNested): array
{
$a['size'] = imagesx($gd).'x'.imagesy($gd);
$a['trueColor'] = imageistruecolor($gd);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

don't these miss the virtual prefix?

Copy link
MemberAuthor

@alexandre-dauboisalexandre-dauboisJan 7, 2025•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Virtual prefixes weren't present before the refactoring, but I'm fine adding them, it makes sense

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

they weren't present because resource's properties are always virtual
now that we deal with objects, we have to make the virtual explicitly

@carsonbotcarsonbot changed the titleAdd casters for object-converted resources[VarDumper] Add casters for object-converted resourcesJan 7, 2025
@alexandre-dauboisalexandre-dauboisforce-pushed thecasters-resources branch 2 times, most recently from48619eb to8c99b6aCompareJanuary 7, 2025 12:53
@nicolas-grekas
Copy link
Member

Thank you@alexandre-daubois.

@nicolas-grekasnicolas-grekas merged commitc98cfb6 intosymfony:7.3Jan 9, 2025
10 of 11 checks passed
@alexandre-dauboisalexandre-daubois deleted the casters-resources branchJanuary 9, 2025 16:38
@fabpotfabpot mentioned this pull requestMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof left review comments

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

4 participants
@alexandre-daubois@nicolas-grekas@stof@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp