Bug 107255 - Merging cells where only one is not empty should be done without confirmation
Summary: Merging cells where only one is not empty should be done without confirmation
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Kohei Yoshida
URL:
Whiteboard: target:5.4.0
Keywords:
Depends on:
Blocks: Calc-Merge-Split
  Show dependency treegraph
 
Reported: 2017-04-19 01:21 UTC by Aron Budea
Modified: 2017-06-03 18:46 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aron Budea 2017-04-19 01:21:52 UTC
In a spreadsheet leave A1 empty, write something in A2, select both cells and click Merge and Center Cells.

=> The usual dialog appears. (Some cells are not empty. etc.)

As long as only a single merged cell is not empty, there's only one meaningful action: to move its content to the first cell (this is the first option in the dialog). No need for the dialog in this case.

This change is somewhere between a bug and an enhancement.
The current behavior is there in 3.3.0 and 5.3.2.2 as well.
Comment 1 Aron Budea 2017-04-19 01:51:10 UTC
Currently the only case when there's no confirmation dialog if the top-left cell is the non-empty one.
Comment 2 Xisco Faulí 2017-04-19 06:57:04 UTC
Confirmed in

Version: 5.4.0.0.alpha0+
Build ID: 7635e0c1c7f821a1081f8e3868f641ae74a172d6
CPU threads: 4; OS: Linux 4.8; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: group
Comment 4 Aron Budea 2017-04-20 01:06:57 UTC
What seems to be tricky here is that checking if a block is empty is straightforward, but checking if it contains at most one cell with data (or getting the number of empty cells in a block), not so much.
Comment 5 Kohei Yoshida 2017-05-01 22:14:19 UTC
I can take care of this.
Comment 6 Kohei Yoshida 2017-05-02 02:09:42 UTC
You are right about one thing though; implementing this is not trivial.
Comment 7 Aron Budea 2017-05-02 04:31:52 UTC
I gave this a bit of thought when Julien posted the code pointer, and the idea I had on how to find if only a single cell has data is to iteratively bisect the cell block and call IsBlockEmpty() on the halves until the block size is exactly 1 cell or the halves both yield false.

Just throwing it out there, if it's unhelpful, don't mind me.
Comment 8 Kohei Yoshida 2017-05-02 13:32:55 UTC
(In reply to Aron Budea from comment #7)
> I gave this a bit of thought when Julien posted the code pointer, and the
> idea I had on how to find if only a single cell has data is to iteratively
> bisect the cell block and call IsBlockEmpty() on the halves until the block
> size is exactly 1 cell or the halves both yield false.
> 
> Just throwing it out there, if it's unhelpful, don't mind me.

FYI, I already have my implementation locally. :-)  The intent of my comment was it became a bit more complex than I had initially anticipated.

The best approach is to touch the underlying mdds structure directly for best performance.  Otherwise the code may function correctly, but it may not scale.
Comment 9 Commit Notification 2017-05-03 00:39:49 UTC
Kohei Yoshida committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=93f5cb55349e6de5003182462bfee434dc51f6ad

tdf#107255: detect whether the range has only one data cell.

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2017-05-03 02:57:03 UTC
Kohei Yoshida committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5233a2c9009533e454a734ac57c764e15f92a229

tdf#107255: add test case for the multi-cell query method.

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Kohei Yoshida 2017-05-03 22:57:46 UTC
Fixed.