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

Is cell reference parsing wrong? #449

Closed
leuski opened this issue Mar 14, 2020 · 4 comments
Closed

Is cell reference parsing wrong? #449

leuski opened this issue Mar 14, 2020 · 4 comments

Comments

@leuski
Copy link

leuski commented Mar 14, 2020

When cell_reference is parsing a string reference it looks like absolute flags are swapped. From the comments in the code:

Reference parameters absolute_column and absolute_row will be set to true if column part or row part are prefixed by a dollar-sign indicating they are absolute, otherwise false.

but in fact,

    if (column_string[0] == '$')
    {
        absolute_row = true;
        column_string = column_string.substr(1);
    }

    if (row_string[0] == '$')
    {
        absolute_column = true;
        row_string = row_string.substr(1);
    }

am I missing something?

@tfussell
Copy link
Owner

Yes, looks like something that broke in refactoring six years ago that hasn't been noticed since. There are unit tests for cell references, but only for both absolute column and row not one or the other. See https://github.com/tfussell/xlnt/blob/master/tests/cell/cell_test_suite.cpp#L667-L669 A fix with an extra test case or two is welcome otherwise I'll take care of it when I can.

@amiremohamadi
Copy link
Contributor

@tfussell I'd like to work on it if no one is working on.
Just to make sure, if column part or row part are prefixed by a dollar-sign both of absolute_column and absolute_row must set to true?!

@tfussell
Copy link
Owner

@amiremohamadi Great! Contribution are always welcome. It's possible to have an absolute column with a relative row and vice versa. So there are four forms:

  • $A$1 absolute column, absolute row
  • $A1 absolute column, relative row
  • A$1 relative column, absolute row
  • A1 relative column, relative row

Official documentation is here https://support.office.com/en-us/article/switch-between-relative-absolute-and-mixed-references-dfec08cd-ae65-4f56-839e-5f0d8d0baca9

I would recommend turning these four into test cases in https://github.com/tfussell/xlnt/blob/master/tests/cell/cell_test_suite.cpp#L667-L669 and then writing the code that passes the tests (TDD).

@tfussell
Copy link
Owner

This should be resolved by #451.

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

No branches or pull requests

3 participants