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

Add support for ignored errors for sheets to now display errors over selected ranges #2723

Closed

Conversation

jacques
Copy link

@jacques jacques commented Apr 1, 2022

This is:

- [ ] a bugfix
- [x] a new feature

Checklist:

Why this change is needed?

Adds the ability to specify that specific cell ranges should have errors ignored (i.e. when using formulas that reference cells and the formulas next door don't match the one to the left). This removes the green triangles from being displayed as Excel ignores the error.

@oleibman oleibman mentioned this pull request Apr 7, 2022
@jacques
Copy link
Author

jacques commented Apr 8, 2022

@MarkBaker please advise what is required to push this pull request over the line.

public function ignoreError(string $range, string $errorType)
{
if (
!in_array(
Copy link
Member

Choose a reason for hiding this comment

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

Can we use constants for this list of error types, and perhaps define that list in a separate ErrorType class in the way that we define constants for style settings, datatypes, etc?

That's perhaps useful, given that the list entries are case-sensitive.

$range = self::pregReplace('/^(\\d+):(\\d+)$/', 'A${1}:XFD${2}', $range);

if (preg_match('/^([A-Z]+)(\\d+):([A-Z]+)(\\d+)$/', $range, $matches) === 1) {
if (!\array_key_exists($errorType, $this->ignoredErrors)) {
Copy link
Member

Choose a reason for hiding this comment

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

We don't generally use the namespace prefix \ for core functions

if (!\array_key_exists($errorType, $this->ignoredErrors)) {
$this->ignoredErrors[$errorType] = [];
}
$this->ignoredErrors[$errorType][$range] = $range;
Copy link
Member

@MarkBaker MarkBaker Apr 14, 2022

Choose a reason for hiding this comment

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

What happens if we delete or insert row/columns that are part of the ignore error range? Should the entries in the ignoredErrors array be updated to reflect that change?

Do you need to store the range as both key and value? Or would it be sufficient to store the range as the key, and a simple boolean true as the value; then use array_keys() in the save routine?

@MarkBaker
Copy link
Member

Can you add some unit tests to show that the details of errors to ignore are being written as expected?

There's some examples on how to approach testing in /tests/PhpSpreadsheetTests/Writer/Xlsx/* where we use the writer calls to return the XML to the test rather than actually saving to a file; and can then assert that the XML contains the markup that's expected.
e.g. ConditionalTest.php and the testWriteSimpleCellConditionalFromWizard() method.

Something like:

namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;

use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;

class IgnoreErrorsTest extends AbstractFunctional
{
    public function testWriteIgnoreErrors(): void
    {
        $spreadsheet = new Spreadsheet();
        $worksheet = $spreadsheet->getActiveSheet();

        // Code to ignore a set of errors for a range or ranges goes here

        $writer = new Xlsx($spreadsheet);
        $writerWorksheet = new Xlsx\Worksheet($writer);
        $data = $writerWorksheet->writeWorksheet($worksheet, []);

        $expected = <<<XML
//xml for the expected ignoredErrors element markup goes here
XML;
        self::assertStringContainsString($expected, $data);
    }
}

Copy link
Member

@MarkBaker MarkBaker left a comment

Choose a reason for hiding this comment

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

Please take a look at my comments

@MarkBaker MarkBaker added the writer/xlsx Writer for MS OfficeOpenXML-format (xlsx) spreadsheet files label Apr 23, 2022
@oleibman
Copy link
Collaborator

I believe that PR #3508, which was merged in April and should be part of the most recent release, accomplishes what you were requesting.

@oleibman oleibman marked this pull request as draft June 21, 2023 21:33
@oleibman
Copy link
Collaborator

Superseded by 3508, closing.

@oleibman oleibman closed this Jun 28, 2023
@jacques
Copy link
Author

jacques commented Jun 28, 2023

Thank you will check it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
writer/xlsx Writer for MS OfficeOpenXML-format (xlsx) spreadsheet files
Development

Successfully merging this pull request may close these issues.

3 participants