Worksheet::mergeCell should work also in the trivial case

[skypjack]

I faced a curious issue in a project of mine that boils down to the way Worksheet::mergeCell works. Currently, this function runs internally the test reported below:

if (range.rowCount() < 2 && range.columnCount() < 2)
    return false;

I see the reasons for which you check these counters but returning false by default seems all the way wrong.

In other worlds, consider the case in which the column count is 1. This is in fact a valid request. It translates to: merge a range that is composed by a single cell. The result it the cell itself, you've nothing to do here but returning false is an error since the range is in fact valid. The same applies to the row count.

At a first glance, probably this is solved by replacing the 2 with a 1 as:

if (range.rowCount() < 1 && range.columnCount() < 1)
    return false;

I've never tried it yet though. Not sure the code that follows the check works fine already in this case but I don't see any reason for which it shouldn't.

Please, let me know your thoughts. In the meantime, I can easily intercept the single cell range_ before calling mergeCells.

[j2doll]

Dear @skypjack

I am not sure if the following information will help.

• NOTE:
  • First cell index of tableWidget is 0(zero).
  • But, first cell of Qxlsx document is 1(one).

// merged cell
if ( wsheet->workbook()->activeSheet()->sheetType() == AbstractSheet::ST_WorkSheet )
{
    QList<CellRange> mergeCellRangeList = wsheet->mergedCells();
    for ( int mc = 0; mc < mergeCellRangeList.size(); ++mc ) // mc : from 0 to (mergeCellRangeList.size()-1)
    {
        CellRange mergedCellRange = mergeCellRangeList.at(mc);

        CellReference crTopLeft = mergedCellRange.topLeft();
        int tlCol = crTopLeft.column();
        int tlRow = crTopLeft.row();

        CellReference crBottomRight = mergedCellRange.bottomRight();
        int brCol = crBottomRight.column();
        int brRow = crBottomRight.row();

        // row : tlRow ~ brRow
        // col : tlCol ~ brCol

        /// NOTE: First cell index of tableWidget is 0(zero).
        ///       But, first cell of Qxlsx document is 1(one).

        int rowSpan = brRow - tlRow + 1;
        int colSpan = brCol - tlCol + 1;

        setMergedSpan( (tlRow - 1), (tlCol - 1), rowSpan, colSpan ); // signal

    } // for ( int mc = 0; mc < mergeCellRangeList.size(); ++mc ) ...
} 

[skypjack]

Hi, not sure what it is unfortunately. I'm not getting the indexes from a widget though. The excel is generated automatically from some data on a db. I've a vector of elements and I merge a number of cells of the same size. When the range has size 1, I ask to merge cells from column N to column N, that is, to do literally nothing. Since merging applies on ranges, I expected this to work for ranges of any size. Unfortunately, I see in the code that you expect a range of 2 or more elements (see the line in the codebase that I linked with the previous comment).

How does this snippet help me instead? I don't see it unfortunately, I'm sorry. The error (it it's an error, I think so but you could argue that it is not) is pretty clear and the reason is so as well, since there is a < 2 in the code of the function I linked!!

[j2doll]

Thanks for reporting. 🐵 I'll register this issue to a bug.

[skypjack]

I can make the change and test it if you need. Then I can open a PR. Is there a guide or such to know how to run all tests and be sure that nothing breaks?

[j2doll]

There are no unit test projects except TestExcel. I created a new test project if necessary, or modified TestExcel to proceed.