cui/source/options/cfgchart.cxx | 12 --------
cui/source/options/cfgchart.hxx | 9 +++---
cui/source/options/optchart.cxx | 60 +++++++++++++++++++---------------------
cui/source/options/optchart.hxx | 7 +++-
4 files changed, 39 insertions(+), 49 deletions(-)
New commits:
commit e53f380cbae0741ee4b39483a6422898bb580d28
Author: Armin Le Grand <armin.le.gr...@me.com>
AuthorDate: Thu Mar 21 10:41:06 2019 +0100
Commit: Armin Le Grand <armin.le.gr...@me.com>
CommitDate: Thu Mar 21 13:06:43 2019 +0100
Make SvxChartColorTableItem more const
SvxChartColorTableItem has non-const members (aka modifiers)
which is bad for SfxItems in general, see comments in change.
Adapt SvxChartColorTableItem and it's usage in
SvxDefaultColorOptPage as needed.
This is also preparation for possible SfxItem refactoring
Change-Id: Ia7982b4e7bbfa736229223e55ce63e02143b8cf7
Reviewed-on: https://gerrit.libreoffice.org/69499
Tested-by: Jenkins
Reviewed-by: Armin Le Grand <armin.le.gr...@me.com>
diff --git a/cui/source/options/cfgchart.cxx b/cui/source/options/cfgchart.cxx
index 685b6f9db6f2..52ddde097f4c 100644
--- a/cui/source/options/cfgchart.cxx
+++ b/cui/source/options/cfgchart.cxx
@@ -285,16 +285,4 @@ bool SvxChartColorTableItem::operator==( const
SfxPoolItem& rAttr ) const
return false;
}
-void SvxChartColorTableItem::SetOptions( SvxChartOptions* pOpts ) const
-{
- if ( pOpts )
- pOpts->SetDefaultColors( m_aColorTable );
-}
-
-
-void SvxChartColorTableItem::ReplaceColorByIndex( size_t _nIndex, const
XColorEntry & _rEntry )
-{
- m_aColorTable.replace( _nIndex, _rEntry );
-}
-
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/cui/source/options/cfgchart.hxx b/cui/source/options/cfgchart.hxx
index 91adbf17cda3..debb2372d81c 100644
--- a/cui/source/options/cfgchart.hxx
+++ b/cui/source/options/cfgchart.hxx
@@ -79,7 +79,11 @@ public:
// items
-
+// Make Item read-only (no non-const access methods). Two reasons:
+// (1) Preparation for Item refactor
+// (2) Dangerous due to SfxItem may not be what you expect (e.g. when
+// ::Set in SfxItemSet, not your instance may be used there, no control
+// about what will happen wothout deep knowledge about
SfxItems/SfxItemSets)
class SvxChartColorTableItem : public SfxPoolItem
{
public:
@@ -87,11 +91,8 @@ public:
virtual SfxPoolItem* Clone( SfxItemPool *pPool = nullptr ) const
override;
virtual bool operator==( const SfxPoolItem& ) const override;
- void SetOptions( SvxChartOptions* pOpts ) const;
const SvxChartColorTable & GetColorList() const { return m_aColorTable;}
- SvxChartColorTable & GetColorList() { return m_aColorTable;}
- void ReplaceColorByIndex( size_t _nIndex, const
XColorEntry & _rEntry );
private:
SvxChartColorTable m_aColorTable;
diff --git a/cui/source/options/optchart.cxx b/cui/source/options/optchart.cxx
index 4645f4ea10d0..0ddd786c71c6 100644
--- a/cui/source/options/optchart.cxx
+++ b/cui/source/options/optchart.cxx
@@ -82,15 +82,14 @@ void SvxDefaultColorOptPage::ModifyColorEntry(const
XColorEntry& rEntry, sal_Int
void SvxDefaultColorOptPage::FillBoxChartColorLB()
{
- if (!pColorConfig)
+ if (!m_SvxChartColorTableUniquePtr)
return;
- const SvxChartColorTable & rTab = pColorConfig->GetColorList();
m_pLbChartColors->SetUpdateMode(false);
ClearColorEntries();
- long nCount = rTab.size();
+ const long nCount(m_SvxChartColorTableUniquePtr->size());
for (long i = 0; i < nCount; ++i)
- InsertColorEntry(rTab[i]);
+ InsertColorEntry((*m_SvxChartColorTableUniquePtr)[i]);
m_pLbChartColors->SetUpdateMode(true);
}
@@ -114,19 +113,19 @@
SvxDefaultColorOptPage::SvxDefaultColorOptPage(vcl::Window* pParent, const SfxIt
m_pValSetColorBox->SetStyle( m_pValSetColorBox->GetStyle()
| WB_ITEMBORDER | WB_NAMEFIELD |
WB_VSCROLL );
- pChartOptions.reset(new SvxChartOptions);
+ m_SvxChartOptionsUniquePtr.reset(new SvxChartOptions);
const SfxPoolItem* pItem = nullptr;
if ( rInAttrs.GetItemState( SID_SCH_EDITOPTIONS, false, &pItem ) ==
SfxItemState::SET )
{
- pColorConfig.reset(static_cast< SvxChartColorTableItem*
>(pItem->Clone()));
+ m_SvxChartColorTableUniquePtr = std::make_unique<SvxChartColorTable>(
+ static_cast<const SvxChartColorTableItem*>(pItem)->GetColorList());
}
else
{
- SvxChartColorTable aTable;
- aTable.useDefault();
- pColorConfig.reset(new SvxChartColorTableItem( SID_SCH_EDITOPTIONS,
aTable ));
- pColorConfig->SetOptions( pChartOptions.get() );
+ m_SvxChartColorTableUniquePtr = std::make_unique<SvxChartColorTable>();
+ m_SvxChartColorTableUniquePtr->useDefault();
+
m_SvxChartOptionsUniquePtr->SetDefaultColors(*m_SvxChartColorTableUniquePtr.get());
}
Construct();
@@ -139,11 +138,8 @@ SvxDefaultColorOptPage::~SvxDefaultColorOptPage()
void SvxDefaultColorOptPage::dispose()
{
- if (pChartOptions)
- {
- pColorConfig.reset();
- pChartOptions.reset();
- }
+ m_SvxChartColorTableUniquePtr.reset();
+ m_SvxChartOptionsUniquePtr.reset();
m_pLbChartColors.clear();
m_pValSetColorBox.clear();
m_pPBDefault.clear();
@@ -169,8 +165,10 @@ VclPtr<SfxTabPage> SvxDefaultColorOptPage::Create(
TabPageParent pParent, const
bool SvxDefaultColorOptPage::FillItemSet( SfxItemSet* rOutAttrs )
{
- if( pColorConfig )
- rOutAttrs->Put( *pColorConfig );
+ if( m_SvxChartColorTableUniquePtr )
+ {
+ rOutAttrs->Put(SvxChartColorTableItem(SID_SCH_EDITOPTIONS,
*m_SvxChartColorTableUniquePtr.get()));
+ }
return true;
}
@@ -198,10 +196,10 @@ void SvxDefaultColorOptPage::FillPaletteLB()
void SvxDefaultColorOptPage::SaveChartOptions()
{
- if (pChartOptions)
+ if (m_SvxChartOptionsUniquePtr && m_SvxChartColorTableUniquePtr)
{
- pChartOptions->SetDefaultColors( pColorConfig->GetColorList() );
- pChartOptions->Commit();
+
m_SvxChartOptionsUniquePtr->SetDefaultColors(*m_SvxChartColorTableUniquePtr.get());
+ m_SvxChartOptionsUniquePtr->Commit();
}
}
@@ -213,9 +211,9 @@ void SvxDefaultColorOptPage::SaveChartOptions()
IMPL_LINK_NOARG(SvxDefaultColorOptPage, ResetToDefaults, Button*, void)
{
- if( pColorConfig )
+ if( m_SvxChartColorTableUniquePtr )
{
- pColorConfig->GetColorList().useDefault();
+ m_SvxChartColorTableUniquePtr->useDefault();
FillBoxChartColorLB();
@@ -230,16 +228,16 @@ IMPL_LINK_NOARG(SvxDefaultColorOptPage, ResetToDefaults,
Button*, void)
IMPL_LINK_NOARG(SvxDefaultColorOptPage, AddChartColor, Button*, void)
{
- if( pColorConfig )
+ if( m_SvxChartColorTableUniquePtr )
{
Color const black( 0x00, 0x00, 0x00 );
- pColorConfig->GetColorList().append (XColorEntry ( black,
SvxChartColorTable::getDefaultName(pColorConfig->GetColorList().size())));
+ m_SvxChartColorTableUniquePtr->append(
+ XColorEntry(black,
SvxChartColorTable::getDefaultName(m_SvxChartColorTableUniquePtr->size())));
FillBoxChartColorLB();
-
m_pLbChartColors->GetFocus();
- m_pLbChartColors->SelectEntryPos( pColorConfig->GetColorList().size()
- 1 );
+ m_pLbChartColors->SelectEntryPos(m_SvxChartColorTableUniquePtr->size()
- 1);
m_pPBRemove->Enable();
}
}
@@ -254,23 +252,23 @@ IMPL_LINK_NOARG( SvxDefaultColorOptPage,
RemoveChartColor, Button*, void )
if (m_pLbChartColors->GetSelectedEntryCount() == 0)
return;
- if( pColorConfig )
+ if( m_SvxChartColorTableUniquePtr )
{
- OSL_ENSURE(pColorConfig->GetColorList().size() > 1, "don't delete the
last chart color");
+ OSL_ENSURE(m_SvxChartColorTableUniquePtr->size() > 1, "don't delete
the last chart color");
std::unique_ptr<weld::Builder>
xBuilder(Application::CreateBuilder(GetFrameWeld(),
"cui/ui/querydeletechartcolordialog.ui"));
std::unique_ptr<weld::MessageDialog>
xQuery(xBuilder->weld_message_dialog("QueryDeleteChartColorDialog"));
if (RET_YES == xQuery->run())
{
- pColorConfig->GetColorList().remove( nIndex );
+ m_SvxChartColorTableUniquePtr->remove(nIndex);
FillBoxChartColorLB();
m_pLbChartColors->GetFocus();
if (nIndex == m_pLbChartColors->GetEntryCount() &&
m_pLbChartColors->GetEntryCount() > 0)
- m_pLbChartColors->SelectEntryPos(
pColorConfig->GetColorList().size() - 1 );
+
m_pLbChartColors->SelectEntryPos(m_SvxChartColorTableUniquePtr->size() - 1);
else if (m_pLbChartColors->GetEntryCount() > 0)
m_pLbChartColors->SelectEntryPos( nIndex );
else
@@ -295,7 +293,7 @@ IMPL_LINK_NOARG(SvxDefaultColorOptPage, BoxClickedHdl,
ValueSet*, void)
const XColorEntry aEntry( m_pValSetColorBox->GetItemColor(
m_pValSetColorBox->GetSelectedItemId() ), m_pLbChartColors->GetSelectedEntry()
);
ModifyColorEntry(aEntry, nIdx);
- pColorConfig->ReplaceColorByIndex( nIdx, aEntry );
+ m_SvxChartColorTableUniquePtr->replace(nIdx, aEntry);
m_pLbChartColors->SelectEntryPos( nIdx ); // reselect entry
}
diff --git a/cui/source/options/optchart.hxx b/cui/source/options/optchart.hxx
index f99f0798cf79..321fe0ee4256 100644
--- a/cui/source/options/optchart.hxx
+++ b/cui/source/options/optchart.hxx
@@ -42,8 +42,11 @@ private:
VclPtr<PushButton> m_pPBAdd;
VclPtr<PushButton> m_pPBRemove;
- std::unique_ptr<SvxChartOptions> pChartOptions;
- std::unique_ptr<SvxChartColorTableItem> pColorConfig;
+ std::unique_ptr<SvxChartOptions> m_SvxChartOptionsUniquePtr;
+ // no reason to use a cloned SfxItem here (SvxChartColorTableItem)
+ // that just leads to non-const SfxItem and potential trouble
+ std::unique_ptr<SvxChartColorTable> m_SvxChartColorTableUniquePtr;
+
ImpColorList aColorList;
PaletteManager aPaletteManager;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
