Integrate file name format options into RipCDDialog

for consistency with OrganiseDialog and reducing code duplication

src/core/organiseformat.cpp

@@ -31,6 +31,7 @@
 #include "core/arraysize.h"
 #include "core/timeconstants.h"
 #include "core/utilities.h"
+#include "transcoder/transcoder.h"
 
 const char* OrganiseFormat::kTagPattern = "\\%([a-zA-Z]*)";
 const char* OrganiseFormat::kBlockPattern = "\\{([^{}]+)\\}";
@@ -97,7 +98,8 @@ bool OrganiseFormat::IsValid() const {
   return v.validate(format_copy, pos) == QValidator::Acceptable;
 }
 
-QString OrganiseFormat::GetFilenameForSong(const Song& song) const {
+QString OrganiseFormat::GetFilenameForSong(const Song& song,
+                                           QString prefix_path) const {
   QString filename = ParseBlock(format_, song);
 
   if (QFileInfo(filename).completeBaseName().isEmpty()) {
@@ -141,9 +143,20 @@ QString OrganiseFormat::GetFilenameForSong(const Song& song) const {
     }
   }
 
+  if (!prefix_path.isEmpty()) parts.insert(0, prefix_path);
+
   return parts.join("/");
 }
 
+QString OrganiseFormat::GetFilenameForSong(
+    const Song& song, const TranscoderPreset& transcoder_preset,
+    QString prefix_path) const {
+  OrganiseFormat format(*this);
+  format.add_tag_override("extension", transcoder_preset.extension_);
+
+  return format.GetFilenameForSong(song, prefix_path);
+}
+
 QStringList OrganiseFormat::GetFilenamesForSongs(const SongList& songs) const {
   // Check if we will have multiple files with the same name.
   // If so, they will erase each other if the overwrite flag is set.

src/core/organiseformat.h

@@ -27,9 +27,12 @@
 
 #include "core/song.h"
 
+class TranscoderPreset;
+
 class OrganiseFormat {
  public:
   explicit OrganiseFormat(const QString& format = QString());
+  OrganiseFormat(const OrganiseFormat& format) = default;
 
   static const char* kTagPattern;
   static const char* kBlockPattern;
@@ -55,7 +58,10 @@ class OrganiseFormat {
   void reset_tag_overrides() { tag_overrides_.clear(); }
 
   bool IsValid() const;
-  QString GetFilenameForSong(const Song& song) const;
+  QString GetFilenameForSong(const Song& song, QString prefix_path = "") const;
+  QString GetFilenameForSong(const Song& song,
+                             const TranscoderPreset& transcoder_preset,
+                             QString prefix_path = "") const;
   QStringList GetFilenamesForSongs(const SongList& songs) const;
 
   class Validator : public QValidator {

src/ripper/ripcddialog.cpp

@@ -30,6 +30,7 @@
 
 #include "config.h"
 #include "core/logging.h"
+#include "core/organiseformat.h"
 #include "core/tagreaderclient.h"
 #include "devices/cddadevice.h"
 #include "devices/cddasongloader.h"
@@ -85,8 +86,9 @@ RipCDDialog::RipCDDialog(DeviceManager* device_manager, QWidget* parent)
   cancel_button_->hide();
   ui_->progress_group->hide();
 
-  rip_button_->setEnabled(false);  // will be enabled by DeviceSelected if a
+  rip_button_->setEnabled(
-                                   // valid device is selected
+      false);  // will be enabled by signal handlers if a valid device is
+               // selected by user and a list of tracks is loaded
 
   InitializeDevices();
 
@@ -100,6 +102,9 @@ RipCDDialog::RipCDDialog(DeviceManager* device_manager, QWidget* parent)
   connect(ui_->options, SIGNAL(clicked()), SLOT(Options()));
   connect(ui_->select, SIGNAL(clicked()), SLOT(AddDestination()));
 
+  connect(ui_->naming_group, SIGNAL(FormatStringChanged()),
+          SLOT(FormatStringUpdated()));
+
   setWindowTitle(tr("Rip CD"));
   AddDestinationDirectory(QDir::homePath());
 
@@ -176,6 +181,14 @@ void RipCDDialog::InitializeDevices() {
 void RipCDDialog::ClickedRipButton() {
   Q_ASSERT(cdda_device_);
 
+  OrganiseFormat format = ui_->naming_group->format();
+  Q_ASSERT(format.IsValid());
+
+  ui_->naming_group->StoreSettings();
+
+  QFileInfo path(
+      ui_->destination->itemData(ui_->destination->currentIndex()).toString());
+
   // create and connect Ripper instance for this task
   Ripper* ripper = new Ripper(cdda_device_->raw_cdio(), this);
   connect(cancel_button_, SIGNAL(clicked()), ripper, SLOT(Cancel()));
@@ -196,11 +209,13 @@ void RipCDDialog::ClickedRipButton() {
     if (!checkboxes_.value(i - 1)->isChecked()) {
       continue;
     }
-    QString transcoded_filename = GetOutputFileName(
+    Song& song = songs_[i - 1];
-        ParseFileFormatString(ui_->format_filename->text(), i));
+    QString transcoded_filename = format.GetFilenameForSong(
-    QString title = track_names_.value(i - 1)->text();
+        song, preset, /*prefix_path=*/path.filePath());
-    ripper->AddTrack(i, title, transcoded_filename, preset);
+    ripper->AddTrack(i, song.title(), transcoded_filename, preset,
+                     ui_->naming_group->overwrite_existing());
   }
+
   ripper->SetAlbumInformation(
       ui_->albumLineEdit->text(), ui_->artistLineEdit->text(),
       ui_->genreLineEdit->text(), ui_->yearLineEdit->text().toInt(),
@@ -275,6 +290,7 @@ void RipCDDialog::DeviceSelected(int device_index) {
   if (cdda_device_) disconnect(cdda_device_.get(), nullptr, this, nullptr);
 
   ResetDialog();
+  EnableIfPossible();
   if (device_index < 0)
     return;  // Invalid selection, probably no devices around
 
@@ -299,15 +315,14 @@ void RipCDDialog::DeviceSelected(int device_index) {
   Q_ASSERT(loader_);
 
   connect(loader_, SIGNAL(SongsDurationLoaded(SongList)),
-          SLOT(BuildTrackListTable(SongList)));
+          SLOT(UpdateTrackList(SongList)));
   connect(loader_, SIGNAL(SongsMetadataLoaded(SongList)),
-          SLOT(UpdateTrackListTable(SongList)));
+          SLOT(UpdateTrackList(SongList)));
   connect(loader_, SIGNAL(SongsMetadataLoaded(SongList)),
           SLOT(AddAlbumMetadataFromMusicBrainz(SongList)));
 
   // load songs from new SongLoader
   loader_->LoadSongs();
-  rip_button_->setEnabled(true);
 }
 
 void RipCDDialog::Finished(Ripper* ripper) {
@@ -328,13 +343,24 @@ void RipCDDialog::UpdateProgressBar(int progress) {
   ui_->progress_bar->setValue(progress);
 }
 
-void RipCDDialog::BuildTrackListTable(const SongList& songs) {
+void RipCDDialog::UpdateTrackList(const SongList& songs) {
-  checkboxes_.clear();
+  if (songs_.isEmpty() || songs_.length() == songs.length()) {
-  track_names_.clear();
+    songs_ = songs;
+    UpdateTrackListTable();
+  } else {
+    qLog(Error) << "Number of tracks in metadata does not match number of "
+                   "songs on disc!";
+  }
+  EnableIfPossible();
+}
 
-  ui_->tableWidget->setRowCount(songs.length());
+void RipCDDialog::UpdateTrackListTable() {
+  checkboxes_.clear();
+
+  ui_->tableWidget->clear();
+  ui_->tableWidget->setRowCount(songs_.length());
   int current_row = 0;
-  for (const Song& song : songs) {
+  for (const Song& song : songs_) {
     QCheckBox* checkbox = new QCheckBox(ui_->tableWidget);
     checkbox->setCheckState(Qt::Checked);
     checkboxes_.append(checkbox);
@@ -343,7 +369,10 @@ void RipCDDialog::BuildTrackListTable(const SongList& songs) {
                                     new QLabel(QString::number(song.track())));
     QLineEdit* line_edit_track_title =
         new QLineEdit(song.title(), ui_->tableWidget);
-    track_names_.append(line_edit_track_title);
+    connect(line_edit_track_title, &QLineEdit::textChanged,
+            [this, current_row](const QString& text) {
+              songs_[current_row].set_title(text);
+            });
     ui_->tableWidget->setCellWidget(current_row, kTrackTitleColumn,
                                     line_edit_track_title);
     ui_->tableWidget->setCellWidget(current_row, kTrackDurationColumn,
@@ -352,15 +381,6 @@ void RipCDDialog::BuildTrackListTable(const SongList& songs) {
   }
 }
 
-void RipCDDialog::UpdateTrackListTable(const SongList& songs) {
-  if (track_names_.length() == songs.length()) {
-    BuildTrackListTable(songs);
-  } else {
-    qLog(Error) << "Number of tracks in metadata does not match number of "
-                   "songs on disc!";
-  }
-}
-
 void RipCDDialog::AddAlbumMetadataFromMusicBrainz(const SongList& songs) {
   Q_ASSERT(songs.length() > 0);
 
@@ -382,31 +402,8 @@ void RipCDDialog::SetWorking(bool working) {
   ui_->progress_group->setVisible(true);
 }
 
-QString RipCDDialog::GetOutputFileName(const QString& basename) const {
-  QFileInfo path(
-      ui_->destination->itemData(ui_->destination->currentIndex()).toString());
-  QString extension = ui_->format->itemData(ui_->format->currentIndex())
-                          .value<TranscoderPreset>()
-                          .extension_;
-  return path.filePath() + '/' + basename + '.' + extension;
-}
-
-QString RipCDDialog::ParseFileFormatString(const QString& file_format,
-                                           int track_no) const {
-  QString to_return = file_format;
-  to_return.replace(QString("%artist"), ui_->artistLineEdit->text());
-  to_return.replace(QString("%album"), ui_->albumLineEdit->text());
-  to_return.replace(QString("%disc"), ui_->discLineEdit->text());
-  to_return.replace(QString("%genre"), ui_->genreLineEdit->text());
-  to_return.replace(QString("%year"), ui_->yearLineEdit->text());
-  to_return.replace(QString("%title"),
-                    track_names_.value(track_no - 1)->text());
-  to_return.replace(QString("%track"), QString::number(track_no));
-
-  return to_return;
-}
-
 void RipCDDialog::ResetDialog() {
+  songs_.clear();
   ui_->tableWidget->setRowCount(0);
   ui_->albumLineEdit->clear();
   ui_->artistLineEdit->clear();
@@ -414,3 +411,10 @@ void RipCDDialog::ResetDialog() {
   ui_->yearLineEdit->clear();
   ui_->discLineEdit->clear();
 }
+
+void RipCDDialog::FormatStringUpdated() { EnableIfPossible(); }
+
+void RipCDDialog::EnableIfPossible() {
+  rip_button_->setEnabled(!songs_.isEmpty() &&
+                          ui_->naming_group->format().IsValid());
+}

src/ripper/ripcddialog.h

@@ -60,31 +60,24 @@ class RipCDDialog : public QDialog {
   void Cancelled(Ripper* ripper);
   void SetupProgressBarLimits(int min, int max);
   void UpdateProgressBar(int progress);
-  // Initializes track list table based on preliminary song list with durations
+  void UpdateTrackList(const SongList& songs);
-  // but without metadata.
-  void BuildTrackListTable(const SongList& songs);
-  // Update track list based on metadata.
-  void UpdateTrackListTable(const SongList& songs);
   // Update album information with metadata.
   void AddAlbumMetadataFromMusicBrainz(const SongList& songs);
   void DiscChanged();
+  void FormatStringUpdated();
 
  private:
   static const char* kSettingsGroup;
   static const int kMaxDestinationItems;
 
-  // Constructs a filename from the given base name with a path taken
-  // from the ui dialog and an extension that corresponds to the audio
-  // format chosen in the ui.
   void AddDestinationDirectory(QString dir);
-  QString GetOutputFileName(const QString& basename) const;
-  QString ParseFileFormatString(const QString& file_format, int track_no) const;
   void SetWorking(bool working);
   void ResetDialog();
   void InitializeDevices();
+  void EnableIfPossible();
+  void UpdateTrackListTable();
 
   QList<QCheckBox*> checkboxes_;
-  QList<QLineEdit*> track_names_;
   QString last_add_dir_;
   QPushButton* cancel_button_;
   QPushButton* close_button_;
@@ -95,5 +88,6 @@ class RipCDDialog : public QDialog {
   bool working_;
   std::shared_ptr<CddaDevice> cdda_device_;
   CddaSongLoader* loader_;
+  SongList songs_;
 };
 #endif  // SRC_RIPPER_RIPCDDIALOG_H_

src/ripper/ripcddialog.ui

@@ -10,7 +10,7 @@
     <x>0</x>
     <y>0</y>
     <width>522</width>
-    <height>575</height>
+    <height>800</height>
    </rect>
   </property>
   <property name="windowTitle">
@@ -122,7 +122,7 @@
            <number>4</number>
           </property>
           <attribute name="horizontalHeaderVisible">
-           <bool>true</bool>
+           <bool>false</bool>
           </attribute>
           <attribute name="horizontalHeaderMinimumSectionSize">
            <number>10</number>
@@ -224,51 +224,30 @@
      </layout>
     </widget>
    </item>
+   <item>
+    <widget class="FileNameFormatWidget" name="naming_group" native="true"/>
+   </item>
    <item>
     <widget class="QGroupBox" name="output_group">
      <property name="title">
       <string>Output options</string>
      </property>
      <layout class="QGridLayout" name="gridLayout">
-      <item row="2" column="2">
+      <item row="1" column="0">
-       <widget class="QPushButton" name="select">
-        <property name="text">
-         <string>Select...</string>
-        </property>
-       </widget>
-      </item>
-      <item row="0" column="0">
-       <widget class="QLabel" name="label_3">
-        <property name="text">
-         <string>File Format</string>
-        </property>
-       </widget>
-      </item>
-      <item row="0" column="1" colspan="2">
-       <widget class="QLineEdit" name="format_filename">
-        <property name="text">
-         <string notr="true">%track - %artist - %title</string>
-        </property>
-       </widget>
-      </item>
-      <item row="2" column="0">
        <widget class="QLabel" name="label_2">
         <property name="text">
          <string>Destination</string>
         </property>
        </widget>
       </item>
-      <item row="1" column="1">
+      <item row="0" column="0">
-       <widget class="QComboBox" name="format">
+       <widget class="QLabel" name="label">
-        <property name="sizePolicy">
+        <property name="text">
-         <sizepolicy hsizetype="Expanding" vsizetype="Fixed">
+         <string>Audio format</string>
-          <horstretch>0</horstretch>
-          <verstretch>0</verstretch>
-         </sizepolicy>
         </property>
        </widget>
       </item>
-      <item row="2" column="1">
+      <item row="1" column="1">
        <widget class="QComboBox" name="destination">
         <property name="enabled">
          <bool>true</bool>
@@ -281,17 +260,27 @@
         </property>
        </widget>
       </item>
-      <item row="1" column="2">
+      <item row="0" column="1">
-       <widget class="QPushButton" name="options">
+       <widget class="QComboBox" name="format">
-        <property name="text">
+        <property name="sizePolicy">
-         <string>Options...</string>
+         <sizepolicy hsizetype="Expanding" vsizetype="Fixed">
+          <horstretch>0</horstretch>
+          <verstretch>0</verstretch>
+         </sizepolicy>
         </property>
        </widget>
       </item>
-      <item row="1" column="0">
+      <item row="1" column="2">
-       <widget class="QLabel" name="label">
+       <widget class="QPushButton" name="select">
         <property name="text">
-         <string>Audio format</string>
+         <string>Select...</string>
+        </property>
+       </widget>
+      </item>
+      <item row="0" column="2">
+       <widget class="QPushButton" name="options">
+        <property name="text">
+         <string>Options...</string>
         </property>
        </widget>
       </item>
@@ -322,6 +311,14 @@
    </item>
   </layout>
  </widget>
+ <customwidgets>
+  <customwidget>
+   <class>FileNameFormatWidget</class>
+   <extends>QWidget</extends>
+   <header>widgets/filenameformatwidget.h</header>
+   <container>1</container>
+  </customwidget>
+ </customwidgets>
  <tabstops>
   <tabstop>tableWidget</tabstop>
   <tabstop>select_all_button</tabstop>
@@ -332,7 +329,6 @@
   <tabstop>genreLineEdit</tabstop>
   <tabstop>yearLineEdit</tabstop>
   <tabstop>discLineEdit</tabstop>
-  <tabstop>format_filename</tabstop>
   <tabstop>format</tabstop>
   <tabstop>options</tabstop>
   <tabstop>destination</tabstop>

src/ripper/ripper.cpp

@@ -58,12 +58,13 @@ Ripper::~Ripper() {}
 
 void Ripper::AddTrack(int track_number, const QString& title,
                       const QString& transcoded_filename,
-                      const TranscoderPreset& preset) {
+                      const TranscoderPreset& preset, bool overwrite_existing) {
   if (track_number < 1 || track_number > TracksOnDisc()) {
     qLog(Warning) << "Invalid track number:" << track_number << "Ignoring";
     return;
   }
-  TrackInformation track(track_number, title, transcoded_filename, preset);
+  TrackInformation track(track_number, title, transcoded_filename, preset,
+                         overwrite_existing);
   tracks_.append(track);
 }
 
@@ -232,7 +233,7 @@ void Ripper::Rip() {
 
     it->temporary_filename = filename;
     transcoder_->AddJob(it->temporary_filename, it->preset,
-                        it->transcoded_filename);
+                        it->transcoded_filename, it->overwrite_existing);
   }
   transcoder_->Start();
   emit RippingComplete();

src/ripper/ripper.h

@@ -48,7 +48,7 @@ class Ripper : public QObject {
   // chosen TranscoderPreset.
   void AddTrack(int track_number, const QString& title,
                 const QString& transcoded_filename,
-                const TranscoderPreset& preset);
+                const TranscoderPreset& preset, bool overwrite_existing);
   // Sets album metadata. This information is used when tagging the
   // final files.
   void SetAlbumInformation(const QString& album, const QString& artist,
@@ -83,17 +83,19 @@ class Ripper : public QObject {
   struct TrackInformation {
     TrackInformation(int track_number, const QString& title,
                      const QString& transcoded_filename,
-                     const TranscoderPreset& preset)
+                     const TranscoderPreset& preset, bool overwrite_existing)
         : track_number(track_number),
           title(title),
           transcoded_filename(transcoded_filename),
-          preset(preset) {}
+          preset(preset),
+          overwrite_existing(overwrite_existing) {}
 
     int track_number;
     QString title;
     QString transcoded_filename;
     TranscoderPreset preset;
     QString temporary_filename;
+    bool overwrite_existing;
   };
 
   struct AlbumInformation {

src/widgets/filenameformatwidget.ui

@@ -21,6 +21,12 @@
        <layout class="QHBoxLayout" name="innerHorizontalLayout">
         <item>
          <widget class="LineTextEdit" name="naming">
+          <property name="sizePolicy">
+           <sizepolicy hsizetype="Expanding" vsizetype="Fixed">
+            <horstretch>0</horstretch>
+            <verstretch>0</verstretch>
+           </sizepolicy>
+          </property>
           <property name="toolTip">
            <string>&lt;p&gt;Tokens start with %, for example: %artist %album %title &lt;/p&gt;