From 0e760e5f8d06d00e084c14b64875f58bb42bd8f1 Mon Sep 17 00:00:00 2001
From: Stefano Pettini <stefano.pettini@gmail.com>
Date: Sun, 31 Jul 2016 22:32:11 +0200
Subject: [PATCH 27/47] Organize tracks / Guess tags presets persisted
properly.
FilenameLayoutWidget is reworked to fix an old bug regarding the presets
not being saved properly, that I was also able to reproduce.
Logic is not a bit more "linear". Presets are shared between "Organize tracks"
and "Guess tags" dialog. If this is not what is wanted, it can be easily changed,
just let me know. Presets are saved as soon as they are added, updated or deleted.
It is not anymore necessary to accept the dialog to save eventual changes.
The preset used is not saved with the marker "#DELIM#selected" anymore.
Presets are just presets. The one used is matched on the fly using the custom
preset field. If a preset matches the custom preset field, it's considered
selected. Users can of course create custom presets as before.
While presets are shared between the two dialogs, custom presets are not.
Add/Update/Remove work properly. Clicking on a preset name in the combobox
resets the custom pattern to the preset value, this is a new feature,
before it was not possible.
It's a bit difficult to explain, but UX is more natural, just give it a try.
A lot of testing was done (by Stefano), like:
- adding/updating/removing presets.
- if settings are persisted and shared when dialogs are cancelled or accepted.
- if custom patterns are persisted and not shared.
- resetting a modified pattern to its defualt.
- cancelling dialogs, modified presets are kept but custom patterns are not.
REVIEW: 128565
BUG: 226144
---
ChangeLog | 4 +-
src/amarokconfig.kcfg | 2 +-
src/dialogs/OrganizeCollectionDialog.cpp | 5 --
src/dialogs/OrganizeCollectionDialog.h | 2 -
src/widgets/FilenameLayoutWidget.cpp | 102 ++++++++++++++++---------------
src/widgets/FilenameLayoutWidget.h | 10 ++-
6 files changed, 61 insertions(+), 64 deletions(-)
diff --git a/src/amarokconfig.kcfg b/src/amarokconfig.kcfg
index c35140c..b41ca5b 100644
--- a/src/amarokconfig.kcfg
+++ b/src/amarokconfig.kcfg
@@ -183,7 +183,7 @@
<entry key="Format Presets" type="StringList">
<label>Format Presets</label>
<whatsthis>A list of preset formats (token schemas).</whatsthis>
- <default>Default#DELIM#%artist%/%album%/%track%_-_%title%#DELIM#selected</default>
+ <default>Default#DELIM#%artist%/%album%/%track%_-_%title%</default>
</entry>
</group>
diff --git a/src/dialogs/OrganizeCollectionDialog.cpp b/src/dialogs/OrganizeCollectionDialog.cpp
index 973e9cb..ffeabc9 100644
--- a/src/dialogs/OrganizeCollectionDialog.cpp
+++ b/src/dialogs/OrganizeCollectionDialog.cpp
@@ -139,8 +139,6 @@ OrganizeCollectionDialog::OrganizeCollectionDialog( const Meta::TrackList &track
QFlags<KDialog::ButtonCode> buttonMask )
: KDialog( parent )
, ui( new Ui::OrganizeCollectionDialogBase )
- , m_detailed( true )
- , m_schemeModified( false )
, m_conflict( false )
{
Q_UNUSED( name )
@@ -196,7 +194,6 @@ OrganizeCollectionDialog::OrganizeCollectionDialog( const Meta::TrackList &track
// to show the conflict error
connect( ui->overwriteCheck, SIGNAL(stateChanged(int)), SLOT(slotOverwriteModeChanged()) );
- connect( this, SIGNAL(finished(int)), ui->organizeCollectionWidget, SLOT(slotSaveFormatList()) );
connect( this, SIGNAL(accepted()), ui->organizeCollectionWidget, SLOT(onAccept()) );
connect( this, SIGNAL(accepted()), SLOT(slotDialogAccepted()) );
connect( ui->folderCombo, SIGNAL(currentIndexChanged(QString)),
@@ -397,8 +394,6 @@ OrganizeCollectionDialog::slotDialogAccepted()
AmarokConfig::setAsciiOnly( ui->optionsWidget->asciiOnly() );
AmarokConfig::setReplacementRegexp( ui->optionsWidget->regexpText() );
AmarokConfig::setReplacementString( ui->optionsWidget->replaceText() );
-
- ui->organizeCollectionWidget->onAccept();
}
//The Ok button should be disabled when there's no collection root selected, and when there is no .%filetype in format string
diff --git a/src/dialogs/OrganizeCollectionDialog.h b/src/dialogs/OrganizeCollectionDialog.h
index 88fa08d..8d57f4a 100644
--- a/src/dialogs/OrganizeCollectionDialog.h
+++ b/src/dialogs/OrganizeCollectionDialog.h
@@ -116,10 +116,8 @@ class AMAROK_EXPORT OrganizeCollectionDialog : public KDialog
Ui::OrganizeCollectionDialogBase *ui;
TrackOrganizer *m_trackOrganizer;
- bool m_detailed;
Meta::TrackList m_allTracks;
QString m_targetFileExtension;
- bool m_schemeModified;
QStringList m_originals;
QStringList m_previews;
diff --git a/src/widgets/FilenameLayoutWidget.cpp b/src/widgets/FilenameLayoutWidget.cpp
index 8d189cf..6d01743 100644
--- a/src/widgets/FilenameLayoutWidget.cpp
+++ b/src/widgets/FilenameLayoutWidget.cpp
@@ -90,14 +90,15 @@ FilenameLayoutWidget::FilenameLayoutWidget( QWidget *parent )
// - the preset buttons
m_addPresetButton = new QPushButton( i18n("Add preset"), this );
- m_addPresetButton->setToolTip( i18n("Saves the current scheme/format above as a preset.") );
+ m_addPresetButton->setToolTip( i18n("Saves the current scheme/format as new preset.") );
presetLayout1->addWidget( m_addPresetButton, 0 );
m_updatePresetButton = new QPushButton( i18n("Update preset"), this );
+ m_updatePresetButton->setToolTip( i18n("Updates the preset with the current scheme/format.") );
presetLayout1->addWidget( m_updatePresetButton, 0 );
m_removePresetButton = new QPushButton( i18n("Remove preset"), this );
- m_removePresetButton->setToolTip( i18n("Removes the currently selected format preset") );
+ m_removePresetButton->setToolTip( i18n("Removes the currently selected preset.") );
presetLayout1->addWidget( m_removePresetButton, 0 );
schemeGroupLayout->addLayout( presetLayout1 );
@@ -167,7 +168,8 @@ FilenameLayoutWidget::FilenameLayoutWidget( QWidget *parent )
connect( m_filenameLayoutEdit, SIGNAL(textChanged(QString)),
this, SIGNAL(schemeChanged()) );
-debug() << "st3.1";
+ connect( m_filenameLayoutEdit, SIGNAL(textChanged(QString)),
+ this, SLOT(slotUpdatePresetButton()) );
}
Token*
@@ -233,16 +235,17 @@ FilenameLayoutWidget::createStaticToken(qint64 value) const
void
FilenameLayoutWidget::onAccept() //SLOT
{
- slotSaveFormatList();
+ QString custom = getParsableScheme();
+
+ // Custom scheme is stored per dialog
+ debug() << "--- saving custom scheme for" << m_configCategory << custom;
+ Amarok::config( m_configCategory ).writeEntry( "Custom Scheme", custom );
}
QString
FilenameLayoutWidget::getParsableScheme() const
{
- QString scheme = m_advancedMode ? m_filenameLayoutEdit->text() : dropTargetScheme();
-
- Amarok::config( m_configCategory ).writeEntry( "Custom Scheme", scheme );
- return scheme;
+ return m_advancedMode ? m_filenameLayoutEdit->text() : dropTargetScheme();
}
// attempts to set the scheme
@@ -255,8 +258,6 @@ void FilenameLayoutWidget::setScheme(const QString& scheme)
emit schemeChanged();
}
-
-
//Handles the modifications to the dialog to toggle between advanced and basic editing mode.
void
FilenameLayoutWidget::toggleAdvancedMode()
@@ -287,7 +288,6 @@ FilenameLayoutWidget::setAdvancedMode( bool isAdvanced )
Amarok::config( m_configCategory ).writeEntry( "Mode", entryValue );
}
-
QString
FilenameLayoutWidget::dropTargetScheme() const
{
@@ -342,28 +342,34 @@ FilenameLayoutWidget::populateConfiguration()
QString mode = Amarok::config( m_configCategory ).readEntry( "Mode" );
setAdvancedMode( mode == QLatin1String( "Advanced" ) );
- setScheme( Amarok::config( m_configCategory ).readEntryUntranslated( "Custom Scheme" ) );
+ // Custom scheme is stored per dialog
+ QString custom = Amarok::config( m_configCategory ).readEntryUntranslated( "Custom Scheme" );
+ debug() << "--- got custom scheme for" << m_configCategory << custom;
- populateFormatList();
-}
+ populateFormatList( custom );
+ setScheme( custom );
+}
void
-FilenameLayoutWidget::populateFormatList()
+FilenameLayoutWidget::populateFormatList( const QString& custom )
{
DEBUG_BLOCK
+ // Configuration is not symmetric: dialog-specific settings are saved
+ // using m_configCategory, that is different per dialog. The presets are saved
+ // only in one single place, so these can be shared. This place is the "default" one,
+ // that is the configuration for OrganizeCollectionDialog.
+
// items are stored in the config list in the following format:
- // Label#DELIM#format string#DELIM#selected
- // the last item to have the third parameter is the default selected preset
- // the third param isnis optional
+ // Label#DELIM#format string
QStringList presets_raw;
int selected_index = -1;
m_presetCombo->clear();
- presets_raw = AmarokConfig::formatPresets();
+ presets_raw = AmarokConfig::formatPresets(); // Always use the one in OrganizeCollectionDialog
// presets_raw = Amarok::config( m_configCategory ).readEntry( QString::fromLatin1( "Format Presets" ), QStringList() );
- debug() << "--- got preset for" << m_configCategory << presets_raw;
+ debug() << "--- got presets" << presets_raw;
foreach( const QString &str, presets_raw )
{
@@ -372,49 +378,44 @@ FilenameLayoutWidget::populateFormatList()
if( items.size() < 2 )
continue;
m_presetCombo->addItem( items.at( 0 ), items.at( 1 ) ); // Label, format string
- if( items.size() == 3 )
+ if( items.at( 1 ) == custom )
selected_index = m_presetCombo->findData( items.at( 1 ) );
}
- if( selected_index > 0 )
+ if( selected_index >= 0 )
m_presetCombo->setCurrentIndex( selected_index );
- slotFormatPresetSelected( selected_index );
+ connect( m_presetCombo, SIGNAL(activated(int)), this, SLOT(slotFormatPresetSelected(int)) );
connect( m_presetCombo, SIGNAL(currentIndexChanged(int)), this, SLOT(slotFormatPresetSelected(int)) );
}
void
-FilenameLayoutWidget::slotUpdatePresetButton()
-{
- QString comboScheme = m_presetCombo->itemData( m_presetCombo->currentIndex() ). toString();
- m_updatePresetButton->setEnabled( comboScheme != getParsableScheme() );
-}
-
-void
-FilenameLayoutWidget::slotSaveFormatList()
+FilenameLayoutWidget::saveFormatList() const
{
- if( !m_formatListModified )
- return;
+ DEBUG_BLOCK
- QStringList presets;
+ QStringList presets_raw;
int n = m_presetCombo->count();
- int current_idx = m_presetCombo->currentIndex();
for( int i = 0; i < n; ++i )
{
- QString item;
- if( i == current_idx )
- item = "%1#DELIM#%2#DELIM#selected";
- else
- item = "%1#DELIM#%2";
-
+ QString item = "%1#DELIM#%2";
QString scheme = m_presetCombo->itemData( i ).toString();
QString label = m_presetCombo->itemText( i );
item = item.arg( label, scheme );
- presets.append( item );
+ presets_raw.append( item );
}
- Amarok::config( m_configCategory ).writeEntry( QString::fromLatin1( "Format Presets" ), presets );
+ debug() << "--- saving presets" << presets_raw;
+ AmarokConfig::setFormatPresets( presets_raw ); // Always use the one in OrganizeCollectionDialog
+ // Amarok::config( m_configCategory ).writeEntry( QString::fromLatin1( "Format Presets" ), presets_raw );
+}
+
+void
+FilenameLayoutWidget::slotUpdatePresetButton()
+{
+ QString comboScheme = m_presetCombo->itemData( m_presetCombo->currentIndex() ).toString();
+ m_updatePresetButton->setEnabled( comboScheme != getParsableScheme() );
}
void
@@ -428,14 +429,15 @@ void
FilenameLayoutWidget::slotAddFormat()
{
bool ok = false;
- QString name = KInputDialog::getText( i18n( "New Format Preset" ), i18n( "Preset Name" ), i18n( "New Preset" ), &ok, this );
+ QString name = KInputDialog::getText( i18n( "New Preset" ), i18n( "Preset Name" ), i18n( "New Preset" ), &ok, this );
if( !ok )
return; // user canceled.
QString format = getParsableScheme();
- m_presetCombo->insertItem(0, name, format);
- m_presetCombo->setCurrentIndex( 0 );
- m_formatListModified = true;
+ m_presetCombo->addItem( name, format );
+ m_presetCombo->setCurrentIndex( m_presetCombo->count() - 1 );
+
+ saveFormatList();
}
void
@@ -443,7 +445,8 @@ FilenameLayoutWidget::slotRemoveFormat()
{
int idx = m_presetCombo->currentIndex();
m_presetCombo->removeItem( idx );
- m_formatListModified = true;
+
+ saveFormatList();
}
void
@@ -453,5 +456,6 @@ FilenameLayoutWidget::slotUpdateFormat()
QString formatString = getParsableScheme();
m_presetCombo->setItemData( idx, formatString );
m_updatePresetButton->setEnabled( false );
- m_formatListModified = true;
+
+ saveFormatList();
}
diff --git a/src/widgets/FilenameLayoutWidget.h b/src/widgets/FilenameLayoutWidget.h
index 9fb2678..8f6e35a 100644
--- a/src/widgets/FilenameLayoutWidget.h
+++ b/src/widgets/FilenameLayoutWidget.h
@@ -70,7 +70,6 @@ class AMAROK_EXPORT FilenameLayoutWidget : public QWidget
, CollectionRoot
};
-
explicit FilenameLayoutWidget( QWidget *parent = 0 );
virtual ~FilenameLayoutWidget() {}
@@ -78,7 +77,6 @@ class AMAROK_EXPORT FilenameLayoutWidget : public QWidget
void setScheme( const QString &scheme );
-
public slots:
virtual void onAccept();
@@ -91,7 +89,6 @@ class AMAROK_EXPORT FilenameLayoutWidget : public QWidget
/* Updates the update preset button */
void slotUpdatePresetButton();
- void slotSaveFormatList();
void slotFormatPresetSelected( int );
void slotAddFormat();
void slotRemoveFormat();
@@ -109,7 +106,6 @@ class AMAROK_EXPORT FilenameLayoutWidget : public QWidget
/** Fills the m_dropTarget according to the given string scheme. */
void inferScheme( const QString &scheme );
- bool m_formatListModified;
bool m_advancedMode;
protected:
@@ -119,7 +115,10 @@ class AMAROK_EXPORT FilenameLayoutWidget : public QWidget
void populateConfiguration();
/** Populates the preset combo box */
- void populateFormatList();
+ void populateFormatList( const QString &custom );
+
+ /** Saves the preset combo box */
+ void saveFormatList() const;
virtual Token* createToken(qint64 value) const;
@@ -145,7 +144,6 @@ class AMAROK_EXPORT FilenameLayoutWidget : public QWidget
QFrame *m_filenameLayout;
KLineEdit *m_filenameLayoutEdit;
-
/** The name of the category used for storing the configuration */
QString m_configCategory;
};
--
2.9.3
