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

Wrong calculation of highest column with specified row #700

Closed
michael-roth opened this issue Oct 2, 2018 · 9 comments
Closed

Wrong calculation of highest column with specified row #700

michael-roth opened this issue Oct 2, 2018 · 9 comments

Comments

@michael-roth
Copy link
Contributor

This is:

[X] a bug report

What is the expected behavior?

When reading an empty spreadsheet file, the highest (data) column for a given row should be A or null.

What is the current behavior?

When calling \PhpOffice\PhpSpreadsheet\Collection\Cells::getHighestColumn() with $row !== null the method returns the column 1 to high.

What are the steps to reproduce?

  1. Create an empty spreadsheet file (xlsx)
  2. Load it with \PhpOffice\PhpSpreadsheet\Reader\Xlsx::load()
  3. Call \PhpOffice\PhpSpreadsheet\Collection\Cells::getHighestColumn(1)
  4. The return value will be 'B'
<?php

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

$filePath = '/path/to/empty/file';

$reader = IOFactory::createReaderForFile($filePath);
$spreadsheet = $reader->load($filePath);

$highestColumn = $spreadsheet->getActiveSheet()->getHighestColumn(1);
var_dump($highestColumn);

Which versions of PhpSpreadsheet and PHP are affected?

  • PhpSpreadsheet: 1.4
  • PHP: 7.2
@michael-roth
Copy link
Contributor Author

michael-roth commented Oct 2, 2018

I think the error lies in \PhpOffice\PhpSpreadsheet\Collection\Cells::getHighestColumn():

public function getHighestColumn($row = null)
{
    if ($row == null) {
        $colRow = $this->getHighestRowAndColumn();
        return $colRow['column'];
    }

    $columnList = [1];

    // ...

    return Coordinate::stringFromColumnIndex(max($columnList) + 1);
}

$columnList always contains 1 and the method returns its max value +1

@poisa
Copy link

poisa commented Nov 7, 2018

Same here except sometimes the max value is higher than 1. I sometimes just change an unrelated -lesser- column, hit save in Excel and the getHighestColumn() returns a number that suggest 2 or more extra columns.

@stale
Copy link

stale bot commented Jan 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Jan 8, 2019
@poisa
Copy link

poisa commented Jan 8, 2019

For future reference, the way I went around this problem is by checking the values for each column using some heuristics to detect when there was no more data. For example, if you detect 50 empty columns then most likely your data has already finished. This is definitely not perfect but for my very basic use will suffice.

@BenKewell
Copy link
Contributor

BenKewell commented Jan 20, 2019

The wrong value is caused by two parts:

\PhpOffice\PhpSpreadsheet\Collection\Cells::getHighestColumn()

  • Calculated value is +1 before return.

\PhpOffice\PhpSpreadsheet\Reader\Xlsx::readColumnsAndRowsAttributes()

  • A wrong default value is cached when loading file.

Commit as referenced by pull request #856 fixes both parts and is tested on a production website.

On the production website, the produced XLSX files contain many blank pages when printed, which is due to the wrong +1 column count as described in issue #700. This commit fixes the printing issue with correct count value returned.

MarkBaker pushed a commit that referenced this issue Feb 17, 2019
* Fix wrong calculation of highest column

issue #700

* Revert "Fix wrong calculation of highest column"

This reverts commit ef39af1.

* Revert "Revert "Fix wrong calculation of highest column""

This reverts commit d13493e.

* Revert Xlsx reader

* Fix indentation
@stale
Copy link

stale bot commented Mar 21, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Mar 21, 2019
michael-roth added a commit to michael-roth/PhpSpreadsheet that referenced this issue Mar 27, 2019
- Add changelog entry for issue PHPOffice#700
@stale stale bot closed this as completed Mar 28, 2019
MarkBaker pushed a commit that referenced this issue Apr 15, 2019
- Add changelog entry for issue #700
guillaume-ro-fr pushed a commit to guillaume-ro-fr/PhpSpreadsheet that referenced this issue Jun 12, 2019
* Fix wrong calculation of highest column

issue PHPOffice#700

* Revert "Fix wrong calculation of highest column"

This reverts commit ef39af1.

* Revert "Revert "Fix wrong calculation of highest column""

This reverts commit d13493e.

* Revert Xlsx reader

* Fix indentation
guillaume-ro-fr pushed a commit to guillaume-ro-fr/PhpSpreadsheet that referenced this issue Jun 12, 2019
- Add changelog entry for issue PHPOffice#700
@leoburnettbucharest
Copy link

I've noticed that getHighestRow() and getHighestColumn() are influenced by the active cell--as in the cell that you last clicked. I've had only 4 rows and 2 columns, but my active cell was like F18 because that was where I clicked before saving the file. The max values were F and 18 instead of B and 4.

So, I've placed the cursor in A1 and saved the file. That fixed my problem, but I think the 2 methods should take into consideration only cell values and not the empty active cell.

@MarkBaker
Copy link
Member

I've noticed that getHighestRow() and getHighestColumn() are influenced by the active cell--as in the cell that you last clicked. I've had only 4 rows and 2 columns, but my active cell was like F18 because that was where I clicked before saving the file. The max values were F and 18 instead of B and 4.

So, I've placed the cursor in A1 and saved the file. That fixed my problem, but I think the 2 methods should take into consideration only cell values and not the empty active cell.

Consider using the getHighestDataRow() and getHighestDataColumn() methods instead

@oleibman
Copy link
Collaborator

Fixed by PR #944.

@oleibman oleibman removed the stale label Jul 16, 2024
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

6 participants