Re: [Libreoffice] [review for 3.4] fix for bnc#707486

Hi Noel,

On Wed, 2011-07-27 at 20:08 +0100, Noel Power wrote:
> Hi Kohei,
>
> On 27/07/11 19:23, Kohei Yoshida wrote:
> > Hi Noel,
> >
> > On Wed, 2011-07-27 at 11:50 +0100, Noel Power wrote:
> > > I would really like to get this patch into 3.4,
> [...]
> > Hmm... did you forget to attach your patch by any chance?
> yes, ouch I did forget  :-) ( although it is attached to the bug )
> >    I tried to
> > cherry-pick the commits you referenced from master, but they don't apply
> > cleanly...
> well, I didn't want the master fix going into 3.4 ( slightly nervous as 
> it affects the core CopyToClip )

Ok. So, I did verify that that particular ScViewFunc::CopyToClip()
method (with the second parameter taking a ScRangeList object) affects
only the VBA code.  Other than that, aside from the duplication which
you already explained the rationale of, I have no problem with this
change.  And I trust that you did your due diligence of testing this
code and make sure it works from the VBA side.

One thing that strikes me is the following change

@@ -2616,11 +2614,22 @@ ScVbaRange::Copy(const ::uno::Any& Destination) throw 
(uno::RuntimeException)
     }
     else
     {
-        ScRange aRange;
-        RangeHelper thisRange( mxRange );
-        ScUnoConversion::FillScRange( aRange, 
thisRange.getCellRangeAddressable()->getRangeAddress() );
-        uno::Reference< frame::XModel > xModel = excel::GetModelFromRange( mxRange );
-        excel::implnCopyRange( xModel, aRange );
+        if ( m_Areas->getCount() > 1 )
+        {
+            uno::Reference< frame::XModel > xModel = excel::GetModelFromRange( mxRanges );
+            ScCellRangesBase* pUnoRangesBase = getCellRangesBase();
+            ScRangeList aList =  pUnoRangesBase->GetRangeList();
+            if ( !excel::implnCopyRanges( xModel, aList ) )
+                throw uno::RuntimeException( rtl::OUString( RTL_CONSTASCII_USTRINGPARAM("!!! That 
command cannot be used on multiple selections" ) ), uno::Reference< uno::XInterface >() );
+        }
+        else
+        {
+            ScRange aRange;
+            RangeHelper thisRange( mxRange );
+            uno::Reference< frame::XModel > xModel = excel::GetModelFromRange( mxRange );
+            ScUnoConversion::FillScRange( aRange, 
thisRange.getCellRangeAddressable()->getRangeAddress() );
+            excel::implnCopyRange( xModel, aRange );
+        }
     }
 }

where these lines

ScUnoConversion::FillScRange( aRange, thisRange.getCellRangeAddressable()->getRangeAddress() );
uno::Reference< frame::XModel > xModel = excel::GetModelFromRange( mxRange );and the line

are flipped before and after in the 'else' scope.  Unless that's
intentional, I would flip these lines back to match the original code.

Let me know what you think of this, and I'll then push to the 3-4 branch
with the above lines flipped back.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc
<kyoshida@novell.com>