Skip to content
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

[Bug] Removing column shifts removed column's value into next column if next column value is empty #2535

Closed
orkhanahmadov opened this issue Jan 28, 2022 · 1 comment

Comments

@orkhanahmadov
Copy link
Contributor

orkhanahmadov commented Jan 28, 2022

This is:

- [x] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

When removing a column from the worksheet, all remaining column and row values should stay in place, including empty and null values.

What is the current behavior?

When removing a column before the last column, if the next column's value is empty or null instead of keeping that column's value it shifts the removed column's value into that column.

What are the steps to reproduce?

Take following data as example:

// file.csv
[
  ['a1', 'b1', ''],
  ['a2', 'b2', 'c2'],
]
<?php

require __DIR__ . '/vendor/autoload.php';

$reader = new \PhpOffice\PhpSpreadsheet\Reader\Csv();
$reader->load('file.csv');

$worksheet = $reader->getActiveSheet();

// remove column B
$worksheet->removeColumn('B');

// Output result
var_dump($worksheet->toArray());

The result ends like this:

[
  ['a1', 'b1'],
  ['a2', 'c2'],
]

But it must be like this:

[
  ['a1', ''],
  ['a2', 'c2'],
]

Which versions of PhpSpreadsheet and PHP are affected?

Cause

With CSV reader when loading file with loadIntoExisting() method, following line checks if "cell" value is empty, and prevents inserting if it is empty

if ($rowDatum !== '' && $this->readFilter->readCell($columnLetter, $currentRow)) {

Possible solution

We can make a property and setter like:

protected $preventSettingEmptyValues = true;

public function setPreventSettingEmptyValues($preventSettingEmptyValues)
{
  $this->preventSettingEmptyValues = (bool) $preventSettingEmptyValues;

  return $this;
}

And change that if statement to:

if (($this->preventSettingEmptyValues && $rowDatum !== '') && $this->readFilter->readCell($columnLetter, $currentRow)) {
  ...
}

I'll be happy to create a PR with the solution if you agree with it.

The other possible solution would be modifying how \PhpOffice\PhpSpreadsheet\ReferenceHelper@insertNewBefore() method works and making it not rely on $worksheet->getCoordinates() to remove and replace columns. But I think it would make sense to be able to control if a cell needs to be created when $rowDatum is empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants