D11082: [WIP] Display all StreamRestore entries in it's own config Tab

Chris Holland noreply at phabricator.kde.org
Tue Mar 6 18:03:53 UTC 2018


Zren added a comment.


  > You forgot to add StreamRestoreTab.qml to review.
  
  Oh woops. Sorry, did a `git diff` rather than staging stuff.
  
  >> I duplicated BaseMap in map.h to use a QString key focused on the info.name property. Code duplication sucks. :/
  > 
  > Can't you just qHash() the name and use it as integer index for normal MapBase?
  
  God, I've been using Python/JS so long I forgot about hashing map keys at the lower level.
  
  Hmmm, that means I could break updateEntry into 3 functions (abstract code/ update by index / update by name).
  
    diff --git a/src/maps.h b/src/maps.h
    index 0051153..7a440e5 100644
    --- a/src/maps.h
    +++ b/src/maps.h
    @@ -120,31 +120,44 @@ public:
         // Context is passed in as parent because context needs to include the maps
         // so we'd cause a circular dep if we were to try to use the instance here.
         // Plus that's weird separation anyway.
    -    void updateEntry(const PAInfo *info, QObject *parent)
    +    void updateEntry(const PAInfo *info, QObject *parent, quint32 key)
         {
             Q_ASSERT(info);
     
    -        if (m_pendingRemovals.remove(info->index)) {
    +        if (m_pendingRemovals.remove(key)) {
                 // Was already removed again.
                 return;
             }
     
    -        const bool isNew = !m_data.contains(info->index);
    +        const bool isNew = !m_data.contains(key);
     
    -        auto *obj = m_data.value(info->index, nullptr);
    +        auto *obj = m_data.value(key, nullptr);
             if (!obj) {
                 obj = new Type(parent);
             }
             obj->update(info);
    -        m_data.insert(info->index, obj);
    +        m_data.insert(key, obj);
     
             if (isNew) {
    -            const int modelIndex = m_data.keys().indexOf(info->index);
    +            const int modelIndex = m_data.keys().indexOf(key);
                 Q_ASSERT(modelIndex >= 0);
                 emit added(modelIndex);
             }
         }
     
    +    void updateEntryByIndex(const PAInfo *info, QObject *parent)
    +    {
    +        Q_ASSERT(info);
    +        updateEntry(info, parent, info->index);
    +    }
    +    void updateEntryByName(const PAInfo *info, QObject *parent)
    +    {
    +        Q_ASSERT(info);
    +        QString infoName = QString::fromUtf8(info->name);
    +        quint32 nameHash = qHash(...);
    +        updateEntry(info, parent, nameHash);
    +    }
    +
         void removeEntry(quint32 index)
         {
             if (!m_data.contains(index)) {
  
  There's one other reference to `object->index()` in `insert(Type *object)`, but I think that was only used by StreamRestore to populate the notification stream in `Context::streamRestoreCallback`. If that's the only use, we could remove that function.
  
  >> Every stream uses the system notifications icon. Would need to play around with that.
  > 
  > I don't think you can get icon name for this from pa.
  
  Yeah I think so too. I meant more like hiding the icon under the "Stream Restore" tab or using a different icon for everything other than the "notification" stream.

REPOSITORY
  R115 Plasma Audio Volume Applet

REVISION DETAIL
  https://phabricator.kde.org/D11082

To: Zren, #plasma
Cc: drosca, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180306/5f8ad193/attachment.html>


More information about the Plasma-devel mailing list