View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0015941 | Scribus | General | public | 2019-11-13 11:49 | 2019-12-08 21:24 |
| Reporter | ale | Assigned To | ale | ||
| Priority | normal | Severity | feature | Reproducibility | N/A |
| Status | closed | Resolution | fixed | ||
| Product Version | 1.5.6.svn | ||||
| Fixed in Version | 1.5.6.svn | ||||
| Summary | 0015941: Use the "name (#)" pattern when duplicating the master pages | ||||
| Description | a former (resolved) ticket explained why it would be good to avoid "Copy of name" for renaming styles. the same applies for master pages... | ||||
| Tags | No tags attached. | ||||
| Attached Files | |||||
| Patch | Yes | ||||
|
|
i suggest to - create functions for checking for standard names and rename - if possible avoid message boxes when something does not work (most of all when it's impossible that something can happen...) |
|
|
- also "simplifies" the checks for new master pages. - needs 0015940 to be applied for getUniqueName(). - it won't show message boxes anymore when conflicts arises but, depending on the context, will use the "name (#)" pattern or fall back to the old name. - adds functions for checking for reserved names. - contains the suggestion to move all masterPages logic from the dialog and scribusdoc to a new masterpage.h|cpp. masterpages-conflicting-names.diff (13,912 bytes)
diff --git a/scribus/ui/pagepalette_masterpages.cpp b/scribus/ui/pagepalette_masterpages.cpp
index 24f2b4569..1ff60ec58 100644
--- a/scribus/ui/pagepalette_masterpages.cpp
+++ b/scribus/ui/pagepalette_masterpages.cpp
@@ -16,6 +16,8 @@ for which a new license (GPL+exception) is in place.
#include <QMessageBox>
#include <QScopedPointer>
#include <QSpacerItem>
+#include <QString>
+#include <QStringList>
#include <QToolButton>
#include <QToolTip>
#include <QVBoxLayout>
@@ -34,6 +36,7 @@ for which a new license (GPL+exception) is in place.
#include "scribusdoc.h"
#include "scribusview.h"
#include "selection.h"
+#include "util.h"
#include "util_os.h"
@@ -151,11 +154,7 @@ void PagePalette_MasterPages::deleteMasterPage()
{
m_currentPage = delItem->text();
- if ((m_currentPage == CommonStrings::masterPageNormal)
- || (m_currentPage == CommonStrings::trMasterPageNormal)
- || (m_currentPage == CommonStrings::trMasterPageNormalLeft)
- || (m_currentPage == CommonStrings::trMasterPageNormalMiddle)
- || (m_currentPage == CommonStrings::trMasterPageNormalRight))
+ if (isReservedName(m_currentPage))
continue;
QString extraWarn = "";
@@ -201,43 +200,27 @@ void PagePalette_MasterPages::deleteMasterPage()
void PagePalette_MasterPages::duplicateMasterPage()
{
- int copyC = 1;
- QString potentialMasterPageName(m_currentPage);
- while (m_doc->MasterNames.contains(potentialMasterPageName))
- potentialMasterPageName = tr("Copy #%1 of %2").arg(copyC++).arg(m_currentPage);
-
- QScopedPointer<NewTm> dia(new NewTm(this, tr("&Name:"), tr("New Master Page"), m_doc, potentialMasterPageName));
+ QString name = getUniqueName(m_currentPage, m_doc->MasterNames);
+ QScopedPointer<NewTm> dia(new NewTm(this, tr("&Name:"), tr("New Master Page"), m_doc, name));
if (!dia->exec())
return;
if (m_doc->appMode == modeEditClip)
m_view->requestMode(submodeEndNodeEdit);
- QString masterPageName = dia->Answer->text();
- bool masterPageNameWrong = m_doc->MasterNames.contains(masterPageName);
- masterPageNameWrong |= (masterPageName == CommonStrings::masterPageNormal);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormal);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalLeft);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalMiddle);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalRight);
- masterPageNameWrong |= masterPageName.isEmpty();
- while (masterPageNameWrong)
- {
- if (!dia->exec())
- return;
- masterPageName = dia->Answer->text();
- masterPageNameWrong = m_doc->MasterNames.contains(masterPageName);
- masterPageNameWrong |= (masterPageName == CommonStrings::masterPageNormal);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormal);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalLeft);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalMiddle);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalRight);
- masterPageNameWrong |= masterPageName.isEmpty();
- }
+
+ name = dia->Answer->text();
+
+ if (name.isEmpty())
+ return;
+
+ name = getNonReservedName(name);
+ name = getUniqueName(name, m_doc->MasterNames);
+
PrefsManager& prefsManager = PrefsManager::instance();
int inde = m_doc->MasterNames[m_currentPage];
int nr = m_doc->Pages->count();
ScPage* from = m_doc->Pages->at(inde);
- ScPage* destination = m_doc->addMasterPage(nr, masterPageName);
+ ScPage* destination = m_doc->addMasterPage(nr, name);
if (m_doc->pagePositioning() != singlePage)
{
int lp = dia->Links->currentIndex();
@@ -279,8 +262,8 @@ void PagePalette_MasterPages::duplicateMasterPage()
}
//#8321 : incorrect selection of master page on new mp creation/duplication
//m_doc->setCurrentPage(destination);
- updateMasterPageList(masterPageName);
- selectMasterPage(masterPageName);
+ updateMasterPageList(name);
+ selectMasterPage(name);
uint oldItems = m_doc->Items->count();
uint end2 = m_doc->MasterItems.count();
int GrMax = m_doc->GroupCounter;
@@ -312,8 +295,8 @@ void PagePalette_MasterPages::duplicateMasterPage()
for (uint a = end2; a < end3; ++a)
{
PageItem *newItem = m_doc->MasterItems.at(a);
- int masterPageIndex = m_doc->MasterNames[masterPageName];
- newItem->setMasterPage(masterPageIndex, masterPageName);
+ int masterPageIndex = m_doc->MasterNames[name];
+ newItem->setMasterPage(masterPageIndex, name);
}
from->guides.copy(&destination->guides);
m_doc->GroupCounter = GrMax + 1;
@@ -325,7 +308,6 @@ void PagePalette_MasterPages::duplicateMasterPage()
void PagePalette_MasterPages::newMasterPage()
{
- QString masterPageName;
int nr = m_doc->Pages->count();
QScopedPointer<NewTm> dia(new NewTm(this, tr("Name:"), tr("New MasterPage"), m_doc, tr("New Master Page %1").arg(nr)));
@@ -334,28 +316,16 @@ void PagePalette_MasterPages::newMasterPage()
if (m_doc->appMode == modeEditClip)
m_view->requestMode(submodeEndNodeEdit);
- masterPageName = dia->Answer->text();
- bool masterPageNameWrong = m_doc->MasterNames.contains(masterPageName);
- masterPageNameWrong |= (masterPageName == CommonStrings::masterPageNormal);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormal);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalLeft);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalMiddle);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalRight);
- masterPageNameWrong |= masterPageName.isEmpty();
- while (masterPageNameWrong)
- {
- if (!dia->exec())
- return;
- masterPageName = dia->Answer->text();
- masterPageNameWrong = m_doc->MasterNames.contains(masterPageName);
- masterPageNameWrong |= (masterPageName == CommonStrings::masterPageNormal);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormal);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalLeft);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalMiddle);
- masterPageNameWrong |= (masterPageName == CommonStrings::trMasterPageNormalRight);
- masterPageNameWrong |= masterPageName.isEmpty();
- }
- m_doc->setCurrentPage(m_doc->addMasterPage(nr, masterPageName));
+
+ auto name = dia->Answer->text();
+
+ if (name.isEmpty())
+ return;
+
+ name = getNonReservedName(name);
+ name = getUniqueName(name, m_doc->MasterNames);
+
+ m_doc->setCurrentPage(m_doc->addMasterPage(nr, name));
if (m_doc->pagePositioning() != singlePage)
{
int lp = dia->Links->currentIndex();
@@ -367,10 +337,10 @@ void PagePalette_MasterPages::newMasterPage()
lp++;
m_doc->Pages->at(nr)->LeftPg = lp;
}
- updateMasterPageList(masterPageName);
+ updateMasterPageList(name);
//#8321 : incorrect selection of master page on new mp creation/duplictation
- //m_view->showMasterPage(m_doc->MasterNames[masterPageName]);
- selectMasterPage(masterPageName);
+ //m_view->showMasterPage(m_doc->MasterNames[name]);
+ selectMasterPage(name);
m_view->reformPages();
}
@@ -477,21 +447,16 @@ void PagePalette_MasterPages::updateMasterPageList(QString masterPageName)
for (QMap<QString,int>::Iterator it = m_doc->MasterNames.begin(); it != m_doc->MasterNames.end(); ++it)
{
QString mpName = it.key();
-// qDebug() << mpName;
+
if (it.key() == CommonStrings::masterPageNormal)
mpName = CommonStrings::trMasterPageNormal;
QListWidgetItem* mpItem = new QListWidgetItem(mpName);
- if ((mpName != CommonStrings::masterPageNormal) && (mpName != CommonStrings::trMasterPageNormal) &&
- (mpName != CommonStrings::masterPageNormalLeft) && (mpName != CommonStrings::trMasterPageNormalLeft) &&
- (mpName != CommonStrings::masterPageNormalMiddle) && (mpName != CommonStrings::trMasterPageNormalMiddle) &&
- (mpName != CommonStrings::masterPageNormalRight) && (mpName != CommonStrings::trMasterPageNormalRight))
- {
+
+ if (!isReservedName(mpName))
mpItem->setFlags(mpItem->flags() |= Qt::ItemIsEditable);
- }
else
- {
mpItem->setFlags(mpItem->flags() &= ~Qt::ItemIsEditable);
- }
+
mpItem->setData(Qt::UserRole, it.key());
masterPageListBox->addItem(mpItem);
}
@@ -508,45 +473,48 @@ void PagePalette_MasterPages::updateMasterPageList(QString masterPageName)
void PagePalette_MasterPages::renameMasterPage(QListWidgetItem * item)
{
- bool success = true;
QString oldName(item->data(Qt::UserRole).toString());
QString newName(item->text());
+
if (oldName == newName)
return;
- bool sigBlocked = masterPageListBox->blockSignals(true);
- if ((newName == CommonStrings::masterPageNormal) || (newName == CommonStrings::trMasterPageNormal) ||
- (newName == CommonStrings::masterPageNormalLeft) || (newName == CommonStrings::trMasterPageNormalLeft) ||
- (newName == CommonStrings::masterPageNormalMiddle) || (newName == CommonStrings::trMasterPageNormalMiddle) ||
- (newName == CommonStrings::masterPageNormalRight) || (newName == CommonStrings::trMasterPageNormalRight))
+ // FIXME: the checks in the old version of this function
+ // and the ones in m_doc->renameMasterPage() did not match.
+ // we should move all MasterPage related code to a new masterpage.h
+ // file and remove code from scribusdoc.cpp and from here.
+ // at the end, the code below should be:
+ //
+ // if (m_doc->m_masterPage->rename(oldName, newName))
+ // {
+ // bool sigBlocked = masterPageListBox->blockSignals(true);
+ // item->setData(Qt::UserRole, newName);
+ // masterPageListBox->blockSignals(sigBlocked);
+ // updateMasterPageList(newName);
+ // }
+ // else
+ // {
+ // bool sigBlocked = masterPageListBox->blockSignals(true);
+ // item->setText(getTranslatedReservedName(oldName));
+ // masterPageListBox->blockSignals(sigBlocked);
+ // }
+
+ if (newName.isEmpty() || isReservedName(newName) ||
+ !m_doc->MasterNames.contains(oldName) ||
+ m_doc->MasterNames.contains(newName))
{
- ScMessageBox::information( this, tr("Unable to Rename Master Page"), tr("The Normal page is not allowed to be renamed."), QMessageBox::Ok );
- success = false;
+ bool sigBlocked = masterPageListBox->blockSignals(true);
+ item->setText(oldName);
+ masterPageListBox->blockSignals(sigBlocked);
}
- if (newName.isEmpty())
- success = false;
-
- if (success)
- success = m_doc->renameMasterPage(oldName, newName);
- if (success)
+ else
{
+ bool sigBlocked = masterPageListBox->blockSignals(true);
+ m_doc->renameMasterPage(oldName, newName);
item->setData(Qt::UserRole, newName);
masterPageListBox->blockSignals(sigBlocked);
updateMasterPageList(newName);
- return;
}
-
- QString masterPageName = oldName;
- if (masterPageName == CommonStrings::masterPageNormal)
- masterPageName = CommonStrings::trMasterPageNormal;
- if (masterPageName == CommonStrings::masterPageNormalLeft)
- masterPageName = CommonStrings::trMasterPageNormalLeft;
- if (masterPageName == CommonStrings::masterPageNormalMiddle)
- masterPageName = CommonStrings::trMasterPageNormalMiddle;
- if (masterPageName == CommonStrings::masterPageNormalRight)
- masterPageName = CommonStrings::trMasterPageNormalRight;
- item->setText(masterPageName);
- masterPageListBox->blockSignals(sigBlocked);
}
@@ -565,3 +533,43 @@ void PagePalette_MasterPages::changeEvent(QEvent *e)
QWidget::changeEvent(e);
}
+
+// FIXME: move to a new masterpages.h
+bool PagePalette_MasterPages::isReservedName(const QString& name) const
+{
+ return name == CommonStrings::masterPageNormal ||
+ name == CommonStrings::trMasterPageNormal ||
+ name == CommonStrings::masterPageNormalLeft ||
+ name == CommonStrings::trMasterPageNormalLeft ||
+ name == CommonStrings::masterPageNormalMiddle ||
+ name == CommonStrings::trMasterPageNormalMiddle ||
+ name == CommonStrings::masterPageNormalRight ||
+ name == CommonStrings::trMasterPageNormalRight;
+}
+
+// FIXME: move to a new masterpages.h
+QString PagePalette_MasterPages::getNonReservedName(const QString& name) const
+{
+ const QStringList reservedNames = {
+ CommonStrings::masterPageNormal,
+ CommonStrings::trMasterPageNormal,
+ CommonStrings::trMasterPageNormalLeft,
+ CommonStrings::trMasterPageNormalMiddle,
+ CommonStrings::trMasterPageNormalRight
+ };
+ return getUniqueName(name, reservedNames);
+}
+
+// FIXME: move to a new masterpages.h
+QString PagePalette_MasterPages::getTranslatedReservedName(const QString& name) const
+{
+ if (name == CommonStrings::masterPageNormal)
+ return CommonStrings::trMasterPageNormal;
+ if (name == CommonStrings::masterPageNormalLeft)
+ return CommonStrings::trMasterPageNormalLeft;
+ if (name == CommonStrings::masterPageNormalMiddle)
+ return CommonStrings::trMasterPageNormalMiddle;
+ if (name == CommonStrings::masterPageNormalRight)
+ return CommonStrings::trMasterPageNormalRight;
+ return name;
+}
diff --git a/scribus/ui/pagepalette_masterpages.h b/scribus/ui/pagepalette_masterpages.h
index 67e20077c..9939e4b60 100644
--- a/scribus/ui/pagepalette_masterpages.h
+++ b/scribus/ui/pagepalette_masterpages.h
@@ -51,7 +51,14 @@ protected:
void connectSignals();
void disconnectSignals();
- virtual void changeEvent(QEvent *e);
+ void changeEvent(QEvent *e) override;
+
+ bool isReservedName(const QString& name) const;
+ QString getNonReservedName(const QString& name) const;
+ /*!
+ * \retval the current name or, if it's a standard one, the translated version
+ */
+ QString getTranslatedReservedName(const QString& name) const;
private slots:
void duplicateMasterPage();
|
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2019-11-13 11:49 | ale | New Issue | |
| 2019-11-13 11:50 | ale | Note Added: 0047064 | |
| 2019-11-13 12:50 | ale | File Added: masterpages-conflicting-names.diff | |
| 2019-11-13 12:50 | ale | Note Added: 0047065 | |
| 2019-11-13 12:50 | ale | Summary | use the "name (#)" pattern when duplicating the master pages => [PATCH] use the "name (#)" pattern when duplicating the master pages |
| 2019-11-13 12:50 | ale | Patch | No => Yes |
| 2019-11-13 12:52 | ale | Assigned To | => ale |
| 2019-11-13 12:52 | ale | Status | new => assigned |
| 2019-11-13 22:24 | cbradney | Status | assigned => resolved |
| 2019-11-13 22:24 | cbradney | Fixed in Version | => 1.5.6.svn |
| 2019-11-13 22:24 | cbradney | Resolution | open => fixed |
| 2019-11-13 22:27 | jghali | Summary | [PATCH] use the "name (#)" pattern when duplicating the master pages => Use the "name (#)" pattern when duplicating the master pages |
| 2019-12-08 21:24 | cbradney | Status | resolved => closed |