Generic highlighter: Improving user interface.

- Manage definitions button is enabled only when path is valid.
- Definitions displayed in the dialog are the ones from the path being managed, not the ones currently registered.
- Mime types are registered whenever a download was attempted or one of the locations changed.

src/plugins/texteditor/generichighlighter/highlightdefinitionmetadata.cpp

@@ -66,6 +66,12 @@ void HighlightDefinitionMetaData::setVersion(const QString &version)
 const QString &HighlightDefinitionMetaData::version() const
 { return m_version; }
 
+void HighlightDefinitionMetaData::setFileName(const QString &fileName)
+{ m_fileName = fileName; }
+
+const QString &HighlightDefinitionMetaData::fileName() const
+{ return m_fileName; }
+
 void HighlightDefinitionMetaData::setPatterns(const QStringList &patterns)
 { m_patterns = patterns; }
 

src/plugins/texteditor/generichighlighter/highlightdefinitionmetadata.h

@@ -55,6 +55,9 @@ public:
     void setVersion(const QString &version);
     const QString &version() const;
 
+    void setFileName(const QString &fileName);
+    const QString &fileName() const;
+
     void setPatterns(const QStringList &patterns);
     const QStringList &patterns() const;
 
@@ -76,6 +79,7 @@ private:
     QString m_id;
     QString m_name;
     QString m_version;
+    QString m_fileName;
     QStringList m_patterns;
     QStringList m_mimeTypes;
     QUrl m_url;

src/plugins/texteditor/generichighlighter/highlightersettingspage.cpp

@@ -65,13 +65,11 @@ HighlighterSettingsPagePrivate(const QString &id) :
 
 HighlighterSettingsPage::HighlighterSettingsPage(const QString &id, QObject *parent) :
     TextEditorOptionsPage(parent),
+    m_requestMimeTypeRegistration(false),
     m_d(new HighlighterSettingsPagePrivate(id))
 {
     if (QSettings *s = Core::ICore::instance()->settings())
         m_d->m_settings.fromSettings(m_d->m_settingsPrefix, s);
-
-    connect(this, SIGNAL(definitionsLocationChanged()),
-            Manager::instance(), SLOT(registerMimeTypes()));
 }
 
 HighlighterSettingsPage::~HighlighterSettingsPage()
@@ -111,7 +109,9 @@ QWidget *HighlighterSettingsPage::createPage(QWidget *parent)
     }
 
     connect(m_d->m_page.useFallbackLocation, SIGNAL(clicked(bool)),
-            this, SLOT(useFallbackLocation(bool)));
+            this, SLOT(setFallbackLocationState(bool)));
+    connect(m_d->m_page.definitionFilesPath, SIGNAL(validChanged(bool)),
+            this, SLOT(setManageDefinitionsState(bool)));
     connect(w, SIGNAL(destroyed()), this, SLOT(ignoreDownloadReply()));
 
     return w;
@@ -121,6 +121,11 @@ void HighlighterSettingsPage::apply()
 {
     if (settingsChanged())
         settingsFromUI();
+
+    if (m_requestMimeTypeRegistration) {
+        Manager::instance()->registerMimeTypes();
+        m_requestMimeTypeRegistration = false;
+    }
 }
 
 bool HighlighterSettingsPage::matches(const QString &s) const
@@ -135,12 +140,12 @@ const HighlighterSettings &HighlighterSettingsPage::highlighterSettings() const
 
 void HighlighterSettingsPage::settingsFromUI()
 {    
-    bool locationChanged = false;
+    if (!m_requestMimeTypeRegistration && (
-    if (m_d->m_settings.definitionFilesPath() != m_d->m_page.definitionFilesPath->path() ||
+        m_d->m_settings.definitionFilesPath() != m_d->m_page.definitionFilesPath->path() ||
         m_d->m_settings.fallbackDefinitionFilesPath() !=
             m_d->m_page.fallbackDefinitionFilesPath->path() ||
-        m_d->m_settings.useFallbackLocation() != m_d->m_page.useFallbackLocation->isChecked()) {
+        m_d->m_settings.useFallbackLocation() != m_d->m_page.useFallbackLocation->isChecked())) {
-        locationChanged = true;
+        m_requestMimeTypeRegistration = true;
     }
 
     m_d->m_settings.setDefinitionFilesPath(m_d->m_page.definitionFilesPath->path());
@@ -150,9 +155,6 @@ void HighlighterSettingsPage::settingsFromUI()
     m_d->m_settings.setIgnoredFilesPatterns(m_d->m_page.ignoreEdit->text());
     if (QSettings *s = Core::ICore::instance()->settings())
         m_d->m_settings.toSettings(m_d->m_settingsPrefix, s);
-
-    if (locationChanged)
-        emit definitionsLocationChanged();
 }
 
 void HighlighterSettingsPage::settingsToUI()
@@ -163,7 +165,8 @@ void HighlighterSettingsPage::settingsToUI()
     m_d->m_page.useFallbackLocation->setChecked(m_d->m_settings.useFallbackLocation());
     m_d->m_page.ignoreEdit->setText(m_d->m_settings.ignoredFilesPatterns());
 
-    useFallbackLocation(m_d->m_settings.useFallbackLocation());
+    setFallbackLocationState(m_d->m_page.useFallbackLocation->isChecked());
+    setManageDefinitionsState(m_d->m_page.definitionFilesPath->isValid());
 }
 
 void HighlighterSettingsPage::resetDefinitionsLocation()
@@ -178,9 +181,8 @@ void HighlighterSettingsPage::resetDefinitionsLocation()
 
 void HighlighterSettingsPage::requestAvailableDefinitionsMetaData()
 {
-    m_d->m_page.definitionFilesPath->buttonAtIndex(1)->setEnabled(false);
+    setManageDefinitionsState(false);
 
-    Manager::instance()->downloadAvailableDefinitionsMetaData();
     connect(Manager::instance(),
             SIGNAL(definitionsMetaDataReady(QList<Internal::HighlightDefinitionMetaData>)),
             this,
@@ -188,6 +190,7 @@ void HighlighterSettingsPage::requestAvailableDefinitionsMetaData()
             Qt::UniqueConnection);
     connect(Manager::instance(), SIGNAL(errorDownloadingDefinitionsMetaData()),
             this, SLOT(showError()), Qt::UniqueConnection);
+    Manager::instance()->downloadAvailableDefinitionsMetaData();
 }
 
 void HighlighterSettingsPage::ignoreDownloadReply()
@@ -203,9 +206,11 @@ void HighlighterSettingsPage::ignoreDownloadReply()
 void HighlighterSettingsPage::manageDefinitions(const QList<HighlightDefinitionMetaData> &metaData)
 {
     ManageDefinitionsDialog dialog(metaData,
+                                   m_d->m_page.definitionFilesPath->path() + QLatin1Char('/'),
                                    m_d->m_page.definitionFilesPath->buttonAtIndex(1)->window());
-    dialog.exec();
+    if (dialog.exec() && !m_requestMimeTypeRegistration)
-    m_d->m_page.definitionFilesPath->buttonAtIndex(1)->setEnabled(true);
+        m_requestMimeTypeRegistration = true;
+    setManageDefinitionsState(m_d->m_page.definitionFilesPath->isValid());
 }
 
 void HighlighterSettingsPage::showError()
@@ -213,14 +218,19 @@ void HighlighterSettingsPage::showError()
     QMessageBox::critical(m_d->m_page.definitionFilesPath->buttonAtIndex(1)->window(),
                           tr("Error connecting to server."),
                           tr("Not possible to retrieve data."));
-    m_d->m_page.definitionFilesPath->buttonAtIndex(1)->setEnabled(true);
+    setManageDefinitionsState(m_d->m_page.definitionFilesPath->isValid());
 }
 
-void HighlighterSettingsPage::useFallbackLocation(bool checked)
+void HighlighterSettingsPage::setFallbackLocationState(bool checked)
 {
     m_d->m_page.fallbackDefinitionFilesPath->setEnabled(checked);
 }
 
+void HighlighterSettingsPage::setManageDefinitionsState(bool valid)
+{
+    m_d->m_page.definitionFilesPath->buttonAtIndex(1)->setEnabled(valid);
+}
+
 bool HighlighterSettingsPage::settingsChanged() const
 {    
     if (m_d->m_settings.definitionFilesPath() != m_d->m_page.definitionFilesPath->path())

src/plugins/texteditor/generichighlighter/highlightersettingspage.h

@@ -61,16 +61,14 @@ public:
 
     const HighlighterSettings &highlighterSettings() const;
 
-signals:
-    void definitionsLocationChanged();
-
 private slots:
     void resetDefinitionsLocation();
     void requestAvailableDefinitionsMetaData();
     void manageDefinitions(const QList<Internal::HighlightDefinitionMetaData> &metaData);
     void showError();
     void ignoreDownloadReply();
-    void useFallbackLocation(bool checked);
+    void setFallbackLocationState(bool checked);
+    void setManageDefinitionsState(bool valid);
 
 private:
     void settingsFromUI();
@@ -78,6 +76,8 @@ private:
 
     bool settingsChanged() const;
 
+    bool m_requestMimeTypeRegistration;
+
     struct HighlighterSettingsPagePrivate;
     HighlighterSettingsPagePrivate *m_d;
 };

src/plugins/texteditor/generichighlighter/managedefinitionsdialog.cpp

@@ -38,6 +38,8 @@
 #include <QtCore/QFuture>
 #include <QtCore/QFutureWatcher>
 #include <QtCore/QtConcurrentMap>
+#include <QtCore/QFile>
+#include <QtCore/QFileInfo>
 #include <QtGui/QMessageBox>
 
 #include <QDebug>
@@ -46,8 +48,12 @@ using namespace TextEditor;
 using namespace Internal;
 
 ManageDefinitionsDialog::ManageDefinitionsDialog(
-    const QList<HighlightDefinitionMetaData> &metaDataList, QWidget *parent) :
+        const QList<HighlightDefinitionMetaData> &metaDataList,
-    QDialog(parent), m_definitionsMetaData(metaDataList)
+        const QString &path,
+        QWidget *parent) :
+    QDialog(parent),
+    m_definitionsMetaData(metaDataList),
+    m_path(path)
 {
     ui.setupUi(this);    
     ui.definitionsTable->setHorizontalHeaderLabels(
@@ -68,19 +74,23 @@ void ManageDefinitionsDialog::populateDefinitionsWidget()
     for (int i = 0; i < size; ++i) {
         const HighlightDefinitionMetaData &downloadData = m_definitionsMetaData.at(i);
 
-        QString installedVersion;
+        // Look for this definition in the current path specified by the user, not the one
-        const QString &id = Manager::instance()->definitionIdByName(downloadData.name());
+        // stored in the settings. So the manager should not be queried for this information.
-        const QSharedPointer<HighlightDefinitionMetaData> &metaData =
+        QString dirVersion;
-            Manager::instance()->definitionMetaData(id);
+        QFileInfo fi(m_path + downloadData.fileName());
-        if (!metaData.isNull())
+        QFile definitionFile(fi.absoluteFilePath());
-            installedVersion = metaData->version();
+        if (definitionFile.open(QIODevice::ReadOnly | QIODevice::Text)) {
+            const QSharedPointer<HighlightDefinitionMetaData> &data = Manager::parseMetadata(fi);
+            if (!data.isNull())
+                dirVersion = data->version();
+        }
 
         for (int j = 0; j < 3; ++j) {
             QTableWidgetItem *item = new QTableWidgetItem;
             if (j == 0)
                 item->setText(downloadData.name());
             else if (j == 1) {
-                item->setText(installedVersion);
+                item->setText(dirVersion);
                 item->setTextAlignment(Qt::AlignCenter);
             } else if (j == 2) {
                 item->setText(downloadData.version());
@@ -104,8 +114,8 @@ void ManageDefinitionsDialog::downloadDefinitions()
     QList<QUrl> urls;
     foreach (const QModelIndex &index, ui.definitionsTable->selectionModel()->selectedRows())
         urls.append(m_definitionsMetaData.at(index.row()).url());
-    Manager::instance()->downloadDefinitions(urls);
+    Manager::instance()->downloadDefinitions(urls, m_path);
-    close();
+    accept();
 }
 
 void ManageDefinitionsDialog::changeEvent(QEvent *e)

src/plugins/texteditor/generichighlighter/managedefinitionsdialog.h

@@ -43,6 +43,7 @@ class ManageDefinitionsDialog : public QDialog
     Q_OBJECT
 public:
     explicit ManageDefinitionsDialog(const QList<HighlightDefinitionMetaData> &metaDataList,
+                                     const QString &path,
                                      QWidget *parent = 0);
 
 protected:
@@ -55,6 +56,7 @@ private:
     void populateDefinitionsWidget();
 
     QList<HighlightDefinitionMetaData> m_definitionsMetaData;
+    QString m_path;
     Ui::ManageDefinitionsDialog ui;
 };
 

src/plugins/texteditor/generichighlighter/manager.cpp

@@ -259,6 +259,7 @@ QSharedPointer<HighlightDefinitionMetaData> Manager::parseMetadata(const QFileIn
         if (reader.readNext() == QXmlStreamReader::StartElement && reader.name() == kLanguage) {
             const QXmlStreamAttributes &atts = reader.attributes();
 
+            metaData->setFileName(fileInfo.fileName());
             metaData->setId(fileInfo.absoluteFilePath());
             metaData->setName(atts.value(HighlightDefinitionMetaData::kName).toString());
             metaData->setVersion(atts.value(HighlightDefinitionMetaData::kVersion).toString());
@@ -289,6 +290,7 @@ QSharedPointer<HighlightDefinitionMetaData> Manager::parseMetadata(const QFileIn
 
 QList<HighlightDefinitionMetaData> Manager::parseAvailableDefinitionsList(QIODevice *device) const
 {
+    static const QLatin1Char kSlash('/');
     static const QLatin1String kDefinition("Definition");
 
     QList<HighlightDefinitionMetaData> metaDataList;
@@ -300,8 +302,12 @@ QList<HighlightDefinitionMetaData> Manager::parseAvailableDefinitionsList(QIODev
 
             HighlightDefinitionMetaData metaData;
             metaData.setName(atts.value(HighlightDefinitionMetaData::kName).toString());
-            metaData.setUrl(QUrl(atts.value(HighlightDefinitionMetaData::kUrl).toString()));
             metaData.setVersion(atts.value(HighlightDefinitionMetaData::kVersion).toString());
+            QString url(atts.value(HighlightDefinitionMetaData::kUrl).toString());
+            metaData.setUrl(QUrl(url));
+            const int slash = url.lastIndexOf(kSlash);
+            if (slash != -1)
+                metaData.setFileName(url.right(url.length() - slash - 1));
 
             metaDataList.append(metaData);
         }
@@ -331,22 +337,8 @@ void Manager::downloadAvailableDefinitionsListFinished()
     }
 }
 
-void Manager::downloadDefinitions(const QList<QUrl> &urls)
+void Manager::downloadDefinitions(const QList<QUrl> &urls, const QString &savePath)
 {
-    QString savePath = TextEditorSettings::instance()->highlighterSettings().definitionFilesPath();
-    if (savePath.isEmpty()) {
-        QMessageBox::critical(0, tr("Error"), tr("Please configure the destination directory."));
-        return;
-    }
-
-    savePath.append(QLatin1Char('/'));
-    QDir saveDir(savePath);
-    if (!saveDir.exists()) {
-        QMessageBox::critical(0, tr("Error"),
-                              tr("Please make sure the destination directory exists."));
-        return;
-    }
-
     m_downloaders.clear();
     foreach (const QUrl &url, urls)
         m_downloaders.append(new DefinitionDownloader(url, savePath));
@@ -384,7 +376,6 @@ void Manager::downloadDefinitionsFinished()
         QMessageBox::critical(0, tr("Download Error"), text);
     }
 
-    registerMimeTypes();
     m_downloadingDefinitions = false;
 }
 

src/plugins/texteditor/generichighlighter/manager.h

@@ -75,9 +75,11 @@ public:
     QSharedPointer<HighlightDefinitionMetaData> definitionMetaData(const QString &id) const;
 
     void downloadAvailableDefinitionsMetaData();
-    void downloadDefinitions(const QList<QUrl> &urls);
+    void downloadDefinitions(const QList<QUrl> &urls, const QString &savePath);
     bool isDownloadingDefinitions() const;
 
+    static QSharedPointer<HighlightDefinitionMetaData> parseMetadata(const QFileInfo &fileInfo);
+
 public slots:
     void registerMimeTypes();
     void showGenericHighlighterOptions() const;
@@ -96,7 +98,6 @@ private:
     Q_DISABLE_COPY(Manager)
 
     void gatherDefinitionsMimeTypes(QFutureInterface<Core::MimeType> &future);
-    QSharedPointer<HighlightDefinitionMetaData> parseMetadata(const QFileInfo &fileInfo);
     QList<HighlightDefinitionMetaData> parseAvailableDefinitionsList(QIODevice *device) const;
     void clear();