Skip to content

[Serializer] CsvEncoder::NO_HEADERS_KEY ignored when used in constructor #34019

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 28, 2019
Merged

[Serializer] CsvEncoder::NO_HEADERS_KEY ignored when used in constructor #34019

merged 1 commit into from
Nov 28, 2019

Conversation

savedario
Copy link

@savedario savedario commented Oct 17, 2019

Q A
Branch? 4.3
Bug fix? yes
New feature? no
Deprecations? no
License MIT

My first pull request...
The following code:

$data = <<<EOD
a,b
c,d
EOD;
$encoder = new CsvEncoder([CsvEncoder::NO_HEADERS_KEY=>true]);
var_dump($encoder->decode($data,'csv'));

produces:

array(2) {
  'a' =>
  string(1) "c"
  'b' =>
  string(1) "d"
}

instead of the expected:

array(2) {
  [0] =>
  array(2) {
    [0] =>
    string(1) "a"
    [1] =>
    string(1) "b"
  }
  [1] =>
  array(2) {
    [0] =>
    string(1) "c"
    [1] =>
    string(1) "d"
  }
}

@nicolas-grekas
Copy link
Member

It looks like reading $context[self::AS_COLLECTION_KEY] has the same issue.
Wouldn't it make sense to patch and use getCsvOptions() instead?

@savedario
Copy link
Author

As in: make getCsvOptions() return two extra variables ?

@savedario
Copy link
Author

I tried, but when it was time to update the test, I realized that getCsvOptions() returns a different set of variables in encode() v/s decode().
The original version seemed less invasive... and AS_COLLECTION_KEY will soon be deprecated.

@savedario
Copy link
Author

My proposed changes are done, but I don't know how to correct the failing AppVeyor...

@nicolas-grekas nicolas-grekas changed the title CsvEncoder::NO_HEADERS_KEY ignored when used in constructor [Serializer] CsvEncoder::NO_HEADERS_KEY ignored when used in constructor Nov 28, 2019
@savedario savedario requested a review from dunglas as a code owner November 28, 2019 11:29
@nicolas-grekas
Copy link
Member

Thank you @savedario.

nicolas-grekas added a commit that referenced this pull request Nov 28, 2019
…in constructor (Dario Savella)

This PR was squashed before being merged into the 4.3 branch.

Discussion
----------

[Serializer] CsvEncoder::NO_HEADERS_KEY ignored when used in constructor

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| License       | MIT

My first pull request...
The following code:
```
$data = <<<EOD
a,b
c,d
EOD;
$encoder = new CsvEncoder([CsvEncoder::NO_HEADERS_KEY=>true]);
var_dump($encoder->decode($data,'csv'));
```
produces:
```
array(2) {
  'a' =>
  string(1) "c"
  'b' =>
  string(1) "d"
}
```
instead of the expected:
```
array(2) {
  [0] =>
  array(2) {
    [0] =>
    string(1) "a"
    [1] =>
    string(1) "b"
  }
  [1] =>
  array(2) {
    [0] =>
    string(1) "c"
    [1] =>
    string(1) "d"
  }
}
```

Commits
-------

a0430f6 [Serializer] CsvEncoder::NO_HEADERS_KEY ignored when used in constructor
@nicolas-grekas nicolas-grekas merged commit a0430f6 into symfony:4.3 Nov 28, 2019
@savedario savedario deleted the ignored-no-headers-key branch November 28, 2019 15:30
This was referenced Dec 1, 2019
hultberg pushed a commit to hultberg/symfony that referenced this pull request Sep 17, 2021
…n used in constructor (Dario Savella)

This PR was squashed before being merged into the 4.3 branch.

Discussion
----------

[Serializer] CsvEncoder::NO_HEADERS_KEY ignored when used in constructor

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| License       | MIT

My first pull request...
The following code:
```
$data = <<<EOD
a,b
c,d
EOD;
$encoder = new CsvEncoder([CsvEncoder::NO_HEADERS_KEY=>true]);
var_dump($encoder->decode($data,'csv'));
```
produces:
```
array(2) {
  'a' =>
  string(1) "c"
  'b' =>
  string(1) "d"
}
```
instead of the expected:
```
array(2) {
  [0] =>
  array(2) {
    [0] =>
    string(1) "a"
    [1] =>
    string(1) "b"
  }
  [1] =>
  array(2) {
    [0] =>
    string(1) "c"
    [1] =>
    string(1) "d"
  }
}
```

Commits
-------

a0430f6 [Serializer] CsvEncoder::NO_HEADERS_KEY ignored when used in constructor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants