Re: [PATCH] 2/2 List cleanup in SD

On Thursday 12 of April 2012, Rafael Dominguez wrote:
> Last patches, to test the Bookmark related functions, you need to insert
> another presentation or parts of a presentation by going into Insert->File.
>
> Thanks for checking all the patches!!!

 I have pushed all the remaining patches in this thread, I didn't see any 
problems in any of them. Thank you for all this work.

 It would be still nice to have the list returning cleaned up to return it 
normally instead of using an out argument, as already said before.

 Another somewhat stylistic nitpick is this:

+            std::vector<rtl::OUString> aBookmarkList, aExchangeList;
             const SolarMutexGuard aGuard;
 
             bMergeMasterPages = (pDataDoc != rModel.GetDocument());
             nInsertPageCount = pDataDoc->GetSdPageCount( PK_STANDARD );
             rModel.GetDocument()->InsertBookmarkAsPage(
-                NULL,
-                NULL,
+                aBookmarkList,
+                aExchangeList,

 There's no need to create a named temporary variable for 
this, "std::vector<rtl::OUString>()" in place of "aBookmarkList" would do and 
would make it more obvious that it is really just passing in an empty list 
that will not be used for anything. That would not work for the second 
argument, since the reference there is non-const, but if the argument was a 
pointer instead of a reference, that would both make it more visible that it 
is an optional argument (and NULL could be used there) and more visible that 
it is an out argument ("&aExchangeList" makes it obvious that the variable 
will be changed by the function).

 Could you still please provide patches for these? Thanks.

-- 
 Lubos Lunak
 l.lunak@suse.cz