Commit 0e760e5f authored by Stefano Pettini's avatar Stefano Pettini Committed by Matěj Laitl
Browse files

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
parent bdbb3a27
Amarok ChangeLog
================
(C) 2002-2015 the Amarok authors.
(C) 2002-2016 the Amarok authors.
VERSION 2.9.0
FEATURES:
......@@ -13,6 +13,8 @@ VERSION 2.9.0
* Fix integer fields, like length, always showing up as zero in filter creation
dialog; thanks to Stefano Pettini. (BR 341661)
* Fix background color of the lyrics applet, thanks to Stefano Pettini. (BR 314854)
* Fix Organize tracks / Guess tags presets not persisted properly;
thanks to Stefano Pettini. (BR 226144)
VERSION 2.8.90
......
......@@ -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>
......
......@@ -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
......
......@@ -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;
......
......@@ -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();
}
......@@ -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;
};
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment