To comment on the following update, log in, then open the issue: http://www.openoffice.org/issues/show_bug.cgi?id=8302
------- Additional comments from [EMAIL PROTECTED] Thu Aug 28 22:16:05 +0000 2008 ------- ->nn : not a final one, but it's still much better. It was developped against a dev300 code base. It's heavy merged sheet' proof but it's not dumb's proof : that's what the final version is about, isn't it ? ;). It works fine as long as you don't have at the same time a vertical merged range or an horizontal merged range in an insertion box of at 2+ cols & 2+ Rows. I did not find yet time & motivation to finish it, but here are the wise comments of Kohei about this patch (my personal advise is preceded by a '>'): 8<---------------------------------------------------------------------- So, I took a deeper look of your patch & have a few comments. 1) I think it's far more efficient to directly manipulate the merged cell attributes during the cell insertion operation, instead of calling UnmergeCells() and MergeCells() before and after the cell insertion, because these two calls are somewhat expensive & potentially hurt performance. > +1, but not sure how to do this properly 2) Instead of using EnterListAction() and LeaveListAction(), sticking with just the ScUndoInsertCells instance will make the code a little cleaner maintenance-wise. > +1, I have tried but failed on this one, I had very strange behaviour. 3) In terms of behavior, I think the most logical behavior would be the following: * for column insertion case, to take the leftmost column's merged state and copy it right for the inserted column(s) except when the selection overlaps the anchor cell of the merged range (i.e. the topleft cell). When the selection overlaps the anchor cell, the merged range simply gets shifted & there is no need to expand that merged range. * for row insertion case, similar to the column insertion case but copy the topmost row's merged state down. The same rule applies when the selection overlaps the anchor cell of a merged range. 4) Additionally, we probably should enforce the following rules: * check all the merged ranges that the selection overlaps. If there is at least one merged range whose vertical range goes outside the vertical range of the selection, then the users should not be allowed to insert columns or insert cells & shift right. > -1 : The main point of my patch is to get rid of those ridiculous message box. * Similarly, if there is at least one merged range whose horizontal range goes outside the horizontal range of the selection, then the users should not be allowed to insert rows or insert cells & shift down. > -1 : same reason. For example, say C5:E5 is merged, and D3:D7 are selected. In this case, because the selection's horizontal range does not entirely encompass the horizontal range of the merged region, the users are not allowed to insert row(s) or insert cells & shift down. I think this is important, otherwise a very weird behavior would ensue. So, if I were you I would: 1) remove the merged cell checking code entirely from ScDocFunc::InsertCells(), 2) write new attributes to the inserted cells according to the above behavior. The right place for this would probably be ScTable::InsertCol() for column insertion, and ScTable::InsertRow() for row insertion. 3) adjust the code in ScUndoInsertCells to make the undo/redo work. 4) look into disabling certain menu items in the column/row header context menus & the insert cells context menu, based on the above rules. 5) maybe more that I forget ... P.S. Excel doesn't need to worry about handling stuff like this, since Excel's range selection always expands to entirely overlap any merged regions. So, inserting cells while a merged region is partially selected would never happen. I personally like Excel's range selection behavior though. It takes away a lot of worries. Anyway, I digress. > +10 : maybe it's the right answer to this problem. 8<---------------------------------------------------------------------- Niklas, Kohei or who ever, feel free to pursue, improve or break this patch. I am currently on mail merge performance in Writer, and it's enough to busy me all the day. Regards, --------------------------------------------------------------------- Please do not reply to this automatically generated notification from Issue Tracker. Please log onto the website and enter your comments. http://qa.openoffice.org/issue_handling/project_issues.html#notification --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
