Re: [Libreoffice] [PATCH] format clipboard
Hi Maxime, On Tue, 2012-03-27 at 18:57 +0200, Maxime de Roucy wrote: > I hope this clarified a bit my choice to remove the "nSelectionType == > nsSelectionType::SEL_TBL" part from the lcl_CreateEmptyItemSet > function. Thanks for the details, I pushed the patch and the others that were needed (and forgotten). Could you please add a statement on this page: http://wiki.documentfoundation.org/ReleaseNotes/3.6 -- Cedric ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
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-an
Re: [Libreoffice] [PATCH] format clipboard
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 -- Maxime de Roucy Groupe LINAGORA - OSSA 80 rue Roque de Fillol 92800 PUTEAUX Tel. : 0 810 251 251 Le jeudi 15 mars 2012 à 17:06 +0100, Cedric Bosdonnat a écrit : > Hi Maxime, > > On Wed, 2012-03-14 at 16:50 +0100, Maxime de Roucy wrote: > > Here is some new patchs on the format clipboard. The first just add some > > comment in the formatclipboard.hxx file. The second one depend on > > modifications I made in the previous patch > > 0001-add-GetCurParAttr-and-GetPaMParAttr-in-SwEditShell.patch which is > > itself dependant of the patch > > 0004-SwEditShell-use-of-the-STL-swap-function.patch which is waiting for > > moderator approval to be published in the mailing list. > > I pushed the first patch to the master branch... but there is something > weird in the second patch. Why did you remove the case where > nSelectionType is a table selection? > > I'll push the second patch once that question is clarified. > > -- > Cedric > signature.asc Description: This is a digitally signed message part ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
Re: [Libreoffice] [PATCH] format clipboard
Hi Maxime, On Wed, 2012-03-14 at 16:50 +0100, Maxime de Roucy wrote: > Here is some new patchs on the format clipboard. The first just add some > comment in the formatclipboard.hxx file. The second one depend on > modifications I made in the previous patch > 0001-add-GetCurParAttr-and-GetPaMParAttr-in-SwEditShell.patch which is > itself dependant of the patch > 0004-SwEditShell-use-of-the-STL-swap-function.patch which is waiting for > moderator approval to be published in the mailing list. I pushed the first patch to the master branch... but there is something weird in the second patch. Why did you remove the case where nSelectionType is a table selection? I'll push the second patch once that question is clarified. -- Cedric ___ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
[Libreoffice] [PATCH] format clipboard
Hello Here is some new patchs on the format clipboard. The first just add some comment in the formatclipboard.hxx file. The second one depend on modifications I made in the previous patch 0001-add-GetCurParAttr-and-GetPaMParAttr-in-SwEditShell.patch which is itself dependant of the patch 0004-SwEditShell-use-of-the-STL-swap-function.patch which is waiting for moderator approval to be published in the mailing list. This second patch modify the format paintbrush feature, here is a copy of the commit description : With this patch the format paintbrush differentiate automatic character attributes apply to character than those apply to paragraph. Character attributes applied to paragraph are treat as paragraph format when paragraph format are copied (and so apply to paragraph), and treat as character format when paragraph format are not copied. It also change the behavior of the format paintbrush on "restart numbering" and "restart numbering at" attributes. With this patch those attributes are simply copied… as expected. Regards Maxime de Roucy -- Maxime de Roucy Groupe LINAGORA - OSSA 80 rue Roque de Fillol 92800 PUTEAUX Tel. : 0 810 251 251 From 90a80f234f60bd4d5b48cf725f25771fb92975b6 Mon Sep 17 00:00:00 2001 From: Maxime de Roucy Date: Wed, 14 Mar 2012 15:39:31 +0100 Subject: [PATCH 1/2] Add comments in formatclipboard.hxx --- sw/source/ui/inc/formatclipboard.hxx | 30 ++ 1 file changed, 30 insertions(+) diff --git a/sw/source/ui/inc/formatclipboard.hxx b/sw/source/ui/inc/formatclipboard.hxx index 07891a0..49f47dc 100644 --- a/sw/source/ui/inc/formatclipboard.hxx +++ b/sw/source/ui/inc/formatclipboard.hxx @@ -46,24 +46,54 @@ public: SwFormatClipboard(); ~SwFormatClipboard(); +/** + * Test if the object contain text or paragraphe attribut + */ bool HasContent() const; bool HasContentForThisType( int nSelectionType ) const; bool CanCopyThisType( int nSelectionType ) const; +/** + * Store/Backup the text and paragraph attribut of the current selection. + * + * @param bPersistentCopy + * input parameter - specify if the Paste function will erase the current object. + */ void Copy( SwWrtShell& rWrtShell, SfxItemPool& rPool, bool bPersistentCopy=false ); + +/** + * Paste the stored text and paragraph attributs on the current selection and current paragraph. + * + * @param bNoCharacterFormats + * Do not past the charactere formats. + * + * @param bNoParagraphFormats + * Do not past the paragraph formats. + */ void Paste( SwWrtShell& rWrtShell, SfxStyleSheetBasePool* pPool , bool bNoCharacterFormats=false, bool bNoParagraphFormats=false ); + +/** + * Clear the current text and paragraph attributs stored. + */ void Erase(); private: int m_nSelectionType; + +/** automatic/named character and paragraph attribut set */ SfxItemSet* m_pItemSet; + +/** table attribut set */ SfxItemSet* m_pTableItemSet; +/** name of the character format (if it exist) */ String m_aCharStyle; +/** name of the paragraph format (if it exist) */ String m_aParaStyle; //no frame style because it contains position information +/** specify if the Paste function have to clear the current object */ bool m_bPersistentCopy; }; -- 1.7.9.4 From 0af328d505bbe16bca32ec27b05e8666eaa6075c Mon Sep 17 00:00:00 2001 From: Maxime de Roucy Date: Wed, 14 Mar 2012 16:07:57 +0100 Subject: [PATCH 2/2] Rewrite of the format paintbrush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this patch the format paintbrush differentiate automatic character attributes apply to character than those apply to paragraph. Character attributes applied to paragraph are treat as paragraph format when paragraph format are copied (and so apply to paragraph), and treat as character format when paragraph format are not copied. It also change the behavior of the format paintbrush on "restart numbering" and "restart numbering at" attributes. With this patch those attributes are simply copied… as expected. --- sw/source/ui/inc/formatclipboard.hxx|7 +- sw/source/ui/uiview/formatclipboard.cxx | 270 ++- 2 files changed, 165 insertions(+), 112 deletions(-) diff --git a/sw/source/ui/inc/formatclipboard.hxx b/sw/source/ui/inc/formatclipboard.hxx index 49f47dc..e227622 100644 --- a/sw/source/ui/inc/formatclipboard.hxx +++ b/sw/source/ui/inc/formatclipboard.hxx @@ -81,8 +81,11 @@ public: private: int m_nSelectionType; -/** automatic/named character and paragraph attribut set */ -SfxItemSet* m_pItemSet; +/** automatic/named character attribut set */ +SfxItemSet* m_pItemSet_TxtAttr; +/** automatic/named paragraph attribut set + * (it can be car