doy / spreadsheet-parsexlsx

parse XLSX files
http://metacpan.org/release/Spreadsheet-ParseXLSX
27 stars 35 forks source link

Information on merged cells is not documented #34

Closed Tux closed 8 years ago

Tux commented 9 years ago

I use the global sheet info for the known merged areas:

    $oWkS->{MergedArea}   and $sheet{merged} = [
        map  {  $_->[0] }
        sort {  $a->[1] cmp $b->[1] }
        map  {[ $_, pack "NNNN", @$_          ]}
        map  {[ map { $_ + 1 } @{$_}[1,0,3,2] ]}
        @{$oWkS->{MergedArea}} ];

And the cell attribute information on merged cells

    $sheet{attr}[$c + 1][$r + 1] = {
        @def_attr,

        type    => lc $oWkC->{Type},
        enc     => $oWkC->{Code},
        merged  => $oWkC->{Merged} || 0,
        hidden  => $FmT->{Hidden}  || 0,
        locked  => $FmT->{Lock}    || 0,

Neither is documented, so I am at risk here.

I'd like to see it documented, so I can use it "safe".

doy commented 9 years ago

I'm hesitant to document internal implementation details of Spreadsheet::ParseExcel, since they might change in the future (and so then my code would also have to change to retain compatibility, which it can't do if I document it). Are the documented methods is_merged and get_merged_areas not sufficient?

Tux commented 9 years ago

I would have used those if I would have found them in the manual.

None of man Spreadsheet::ParseExcel, man Spreadsheet::ParseXLSX, perldoc Spreadsheet::ParseExcel, and perldoc Spreadsheet::ParseXLSX mention any method for merged cells.

Tux commented 9 years ago

See https://github.com/Tux/Spreadsheet-Read/issues/1 for the original report

doy commented 9 years ago

https://metacpan.org/pod/Spreadsheet::ParseExcel::Worksheet#get_merged_areas https://metacpan.org/pod/Spreadsheet::ParseExcel::Cell#is_merged

Tux commented 9 years ago

I would never have found the first without a reference to it in the top-level docs. Maybe a section about merged cells?

https://github.com/Tux/Spreadsheet-Read/commit/4a2683c61f6c28e7a2da3a1fc79368efae124772 works fine.

doy commented 8 years ago

I added references to the Spreadsheet::ParseExcel documentation in 0f73ea7 - I'm not too interested in maintaining a full copy of the Spreadsheet::ParseExcel docs myself, but hopefully this will make them more discoverable.