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

Do not throw away indices from Var Exporter#29715

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

Closed
simPod wants to merge3 commits intosymfony:masterfromsimPod:fix-indices
Closed

Do not throw away indices from Var Exporter#29715

simPod wants to merge3 commits intosymfony:masterfromsimPod:fix-indices

Conversation

@simPod
Copy link
Contributor

QA
Bug fix?yes
New feature?no
BC breaks?yes
Deprecations?no
Tests pass?yes
LicenseMIT

The Var Exporter component was throwing away array indices, eg. when exporting array[1=>1, 2=>2], it exported[1 => 1, 2]. This fixes this faulty behaviour, however, it preserves indices everywhere. That is IMHO the correct way as there's no safe way to detect whether array indices are explicit or just implicit.

Copy link
Contributor

@OskarStarkOskarStark left a comment

Choose a reason for hiding this comment

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

Unlike the improvement of the provideExport() method but my feelings about keeping all indices are 👎 Indeed your case looks wrong, but couldn’t we preserve the old behavior and fix your bug too?

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedDec 28, 2018
edited
Loading

Hi, thanks for the PR, but that's not a bug. Not displayed indices are increasing numbers from the last explicit index. That matches how PHP indices work.

@simPod
Copy link
ContributorAuthor

How do you propose to solve it then? Scanning the whole array and checking whether indices are sequence or not doesn't sound very performance sensitive to me. Especially for larger arrays.

@nicolas-grekas
Copy link
Member

I propose to close as this output is expected.

@simPod
Copy link
ContributorAuthor

For array[100 => true, 101 => true] is not expected output[100 => true, true]

@nicolas-grekas
Copy link
Member

To me it is, and the code matches :)
Seehttps://3v4l.org/CXXvn

@simPod
Copy link
ContributorAuthor

simPod commentedDec 29, 2018
edited
Loading

Got it. I'll show you my use case:

Consider this array:

[20 =>true,21 =>true,5 =>true,3 =>true,80 =>true,81 =>true,82 =>true,120 =>true,150 =>true,1 =>true,9 =>true,10 =>true,60 =>true,61 =>true,62 =>true,8 =>true,50 =>true,51 =>true,100 =>true,]

Current version of VarExporter produces this:

[20 =>true,true,5 =>true,3 =>true,80 =>true,true,true,120 =>true,150 =>true,1 =>true,9 =>true,true,60 =>true,true,true,8 =>true,50 =>true,true,100 =>true,]

And that gives this when php array is hydrated from VarExporter's output:

[20 =>true,21 =>true,5 =>true,3 =>true,80 =>true,81 =>true,82 =>true,120 =>true,150 =>true,1 =>true,9 =>true,151 =>true,60 =>true,152 =>true,153 =>true,8 =>true,50 =>true,154 =>true,100 =>true,]

You can see it's not the same array as the origin

[     20 => true,     21 => true,     5 => true,     3 => true,     80 => true,     81 => true,     82 => true,     120 => true,     150 => true,     1 => true,     9 => true,-    10 => true,+    151 => true,     60 => true,-    61 => true,-    62 => true,+    152 => true,+    153 => true,     8 => true,     50 => true,-    51 => true,+    154 => true,     100 => true, ];
Koc reacted with thumbs up emoji

@xabbuh
Copy link
Member

Happy new year everyone! 🎉 IMO we only need to improve the dump to take into account whether or not the currently exported index is greater than the greatest one (see#29741 for my suggestion to fix this).

fabpot added a commit that referenced this pull requestJan 1, 2019
This PR was merged into the 4.2 branch.Discussion----------[VarExporter] fix exporting array indexes| Q             | A| ------------- | ---| Branch?       | 4.2| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#29715| License       | MIT| Doc PR        |Commits-------3c936f4 [VarExporter] fix exporting array indexes
@fabpotfabpot closed thisJan 1, 2019
@simPodsimPod deleted the fix-indices branchJanuary 2, 2019 09:30
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@OskarStarkOskarStarkOskarStark left review comments

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@simPod@nicolas-grekas@xabbuh@OskarStark@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp