<table><tr><td style="">Zren added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D11082">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>You forgot to add StreamRestoreTab.qml to review.</p></blockquote>
<p>Oh woops. Sorry, did a <tt style="background: #ebebeb; font-size: 13px;">git diff</tt> rather than staging stuff.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>I duplicated BaseMap in map.h to use a QString key focused on the info.name property. Code duplication sucks. :/</p></blockquote>
<p>Can't you just qHash() the name and use it as integer index for normal MapBase?</p></blockquote>
<p>God, I've been using Python/JS so long I forgot about hashing map keys at the lower level.</p>
<p>Hmmm, that means I could break updateEntry into 3 functions (abstract code/ update by index / update by name).</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="diff" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);"><span style="color: #000080">diff --git a/src/maps.h b/src/maps.h</span>
<span style="color: #000080">index 0051153..7a440e5 100644</span>
<span style="color: #a00000">--- a/src/maps.h</span>
<span style="color: #00a000">+++ b/src/maps.h</span>
<span style="color: #800080">@@ -120,31 +120,44 @@ public:</span>
// 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.
<span style="color: #a00000">- void updateEntry(const PAInfo *info, QObject *parent)</span>
<span style="color: #00a000">+ void updateEntry(const PAInfo *info, QObject *parent, quint32 key)</span>
{
Q_ASSERT(info);
<span style="color: #a00000">- if (m_pendingRemovals.remove(info->index)) {</span>
<span style="color: #00a000">+ if (m_pendingRemovals.remove(key)) {</span>
// Was already removed again.
return;
}
<span style="color: #a00000">- const bool isNew = !m_data.contains(info->index);</span>
<span style="color: #00a000">+ const bool isNew = !m_data.contains(key);</span>
<span style="color: #a00000">- auto *obj = m_data.value(info->index, nullptr);</span>
<span style="color: #00a000">+ auto *obj = m_data.value(key, nullptr);</span>
if (!obj) {
obj = new Type(parent);
}
obj->update(info);
<span style="color: #a00000">- m_data.insert(info->index, obj);</span>
<span style="color: #00a000">+ m_data.insert(key, obj);</span>
if (isNew) {
<span style="color: #a00000">- const int modelIndex = m_data.keys().indexOf(info->index);</span>
<span style="color: #00a000">+ const int modelIndex = m_data.keys().indexOf(key);</span>
Q_ASSERT(modelIndex >= 0);
emit added(modelIndex);
}
}
<span style="color: #00a000">+ void updateEntryByIndex(const PAInfo *info, QObject *parent)</span>
<span style="color: #00a000">+ {</span>
<span style="color: #00a000">+ Q_ASSERT(info);</span>
<span style="color: #00a000">+ updateEntry(info, parent, info->index);</span>
<span style="color: #00a000">+ }</span>
<span style="color: #00a000">+ void updateEntryByName(const PAInfo *info, QObject *parent)</span>
<span style="color: #00a000">+ {</span>
<span style="color: #00a000">+ Q_ASSERT(info);</span>
<span style="color: #00a000">+ QString infoName = QString::fromUtf8(info->name);</span>
<span style="color: #00a000">+ quint32 nameHash = qHash(...);</span>
<span style="color: #00a000">+ updateEntry(info, parent, nameHash);</span>
<span style="color: #00a000">+ }</span>
<span style="color: #00a000">+</span>
void removeEntry(quint32 index)
{
if (!m_data.contains(index)) {</pre></div>
<p>There's one other reference to <tt style="background: #ebebeb; font-size: 13px;">object->index()</tt> in <tt style="background: #ebebeb; font-size: 13px;">insert(Type *object)</tt>, but I think that was only used by StreamRestore to populate the notification stream in <tt style="background: #ebebeb; font-size: 13px;">Context::streamRestoreCallback</tt>. If that's the only use, we could remove that function.</p>
<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>Every stream uses the system notifications icon. Would need to play around with that.</p></blockquote>
<p>I don't think you can get icon name for this from pa.</p></blockquote>
<p>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.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R115 Plasma Audio Volume Applet</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D11082">https://phabricator.kde.org/D11082</a></div></div><br /><div><strong>To: </strong>Zren, Plasma<br /><strong>Cc: </strong>drosca, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>