Re: [Libreoffice] [PATCH] format clipboard

Hello,

Following my discussion with Cedric on IRC here is some other
explanations on why I chose to remove the "nSelectionType ==
nsSelectionType::SEL_TBL" case from the lcl_CreateEmptyItemSet function
in formatclipboard.cxx.

Without the patch : lcl_CreateEmptyItemSet is called only in the
formatclipboard.cxx file, at line 326, 399 and 511. At line 399, the
function is called with the nSelectionType parameter equal to
nsSelectionType::SEL_TBL. At the other place the function is called
with the nSelectionType parameter equal to the output of
"rWrtShell.GetSelectionType();". rWrtShell is a SwWrtShell. If you look
in the SwWrtShell::GetSelectionType function (line 1412 file
sw/source/ui/wrtsh/wrtsh1.cxx) you will see that when the program take
the only path where the function output can hold an
nsSelectionType::SEL_TBL attribut, the "shorter" function output is equal
to : "GetCntType() | nsSelectionType::SEL_TBL".
Then if you look in the SwEditShell::GetCntType function
(sw/source/core/edit/edws.cxx line 163) you will see this function HAVE
TO return something different from 0 : "OSL_ASSERT( nRet );"

Then we can say that the "rWrtShell.GetSelectionType();" instruction
can't return exactly "nsSelectionType::SEL_TBL". So the "nSelectionType
== nsSelectionType::SEL_TBL" part of the lcl_CreateEmptyItemSet
function is only call by the "lcl_CreateEmptyItemSet(
nsSelectionType::SEL_TBL, rPool );" instruction (line 399).

I think the "nSelectionType == nsSelectionType::SEL_TBL" part of the
lcl_CreateEmptyItemSet function is confusing so I moved it line 399
(just to recall : line 399, the only place where it is currently executed).

I hope this clarified a bit my choice to remove the "nSelectionType ==
nsSelectionType::SEL_TBL" part from the lcl_CreateEmptyItemSet
function.

Regards

Maxime de Roucy

-- 
Maxime de Roucy
Groupe LINAGORA - OSSA
80 rue Roque de Fillol
92800 PUTEAUX
Tel. : 0 810 251 251


Le vendredi 16 mars 2012 à 10:16 +0100, Maxime de Roucy a écrit :
> Here a copy of the mail I send to Cedric Bosdonnat… forgot to add the
> list as "Cc:".
>
> Hello
>
> Thanks you for asking me…
>
> When you select a cell in Writer the selection type is a mix of
> nsSelectionType::SEL_TBL_CELLS, nsSelectionType::SEL_TBL and
> nsSelectionType::SEL_TXT.
> So when the lcl_CreateEmptyItemSet function is called directly with
> the nSelectionType (from rWrtShell.GetSelectionType()) the if
> statement "nSelectionType == nsSelectionType::SEL_TBL" is false.
>
> So in the SwFormatClipboard::Copy function there is another
> statement : 
>
>
>
> ______________________________________________________________________
>
>
>
>
>       if( nSelectionType & nsSelectionType::SEL_TBL_CELLS )//only copy table attributes if really 
> cells are selected (not only text in tables)
>       {
>               m_pTableItemSet = lcl_CreateEmptyItemSet( nsSelectionType::SEL_TBL, rPool );
>               lcl_getTableAttributes( *m_pTableItemSet, rWrtShell );
>       }
>
>
>
> ______________________________________________________________________
>
>
>
>
> That call lcl_CreateEmptyItemSet with just nsSelectionType::SEL_TBL if
> the real selection type contain nsSelectionType::SEL_TBL_CELLS.
>
> I found this way of doing a bit confusing and as tables parameters are
> handle separately in the whole format paintbrush code I thought that
> replacing : 
>
>
>
> ______________________________________________________________________
>
>
>
>
>       m_pTableItemSet = lcl_CreateEmptyItemSet( nsSelectionType::SEL_TBL, rPool );
>
>
>
> ______________________________________________________________________
>
>
>
>
> by : 
>
>
>
> ______________________________________________________________________
>
>
>
>
>       m_pTableItemSet = new SfxItemSet(rPool,
>               SID_ATTR_BORDER_INNER,  SID_ATTR_BORDER_SHADOW, //SID_ATTR_BORDER_OUTER is inbetween
>               RES_BACKGROUND,         RES_SHADOW, //RES_BOX is inbetween
>               SID_ATTR_BRUSH_ROW,     SID_ATTR_BRUSH_TABLE,
>               RES_BREAK,              RES_BREAK,
>               RES_PAGEDESC,           RES_PAGEDESC,
>               RES_LAYOUT_SPLIT,       RES_LAYOUT_SPLIT,
>               RES_ROW_SPLIT,          RES_ROW_SPLIT,
>               RES_KEEP,               RES_KEEP,
>               RES_FRAMEDIR,           RES_FRAMEDIR,
>               FN_PARAM_TABLE_HEADLINE, FN_PARAM_TABLE_HEADLINE,
>               FN_TABLE_BOX_TEXTORIENTATION, FN_TABLE_BOX_TEXTORIENTATION,
>               FN_TABLE_SET_VERT_ALIGN, FN_TABLE_SET_VERT_ALIGN,
>               0);
>
>
>
> ______________________________________________________________________
>
>
>
>
> was not irrelevant.
>
> After this replacement was made there where no reasons keeping the
> "nSelectionType == nsSelectionType::SEL_TBL" block in the
> lcl_CreateEmptyItemSet function.
>
> I hope I answered your question.
>
> BEWARE : this patch need the first patch
> 0001-add-GetCurParAttr-and-GetPaMParAttr-in-SwEditShell from my
> preview mail
> http://lists.freedesktop.org/archives/libreoffice/2012-March/028157.html
>
> Regards
>
> Maxime de Roucy
>
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice

Attachment: signature.asc
Description: This is a digitally signed message part