broulik added a comment.

  Cool!

INLINE COMMENTS

> extension.js:495
> +        var sendTabsIfComplete = function() {
> +            if (--total > 0)
> +                return;

Braces even for single line statements:

  if (...) {
      ...
  }

> extension.js:505
> +
> +        for (let tabIndex in tabs) {
> +            let currentIndex = tabIndex; // Not shared

Does this need a `tabs.hasOwnProperty(...)` check? (cf for in being horrible in 
JS)

> tabsrunner.cpp:188
> +
> +            QString favIconData = 
> tab.value(QStringLiteral("favIconData")).toString();
> +            int b64start = favIconData.indexOf(',');

const

> tabsrunner.cpp:191
> +            if (b64start != -1) {
> +                QByteArray b64 = favIconData.rightRef(favIconData.size() - 
> b64start - 1).toLatin1();
> +                QByteArray data = QByteArray::fromBase64(b64);

+1 for usage of `ref` :)

> tabsrunner.cpp:224
>  
> -            match.setIconName(iconName);
> +            if (!iconName.isEmpty())
> +                match.setIconName(iconName);

Braces

REPOSITORY
  R856 Plasma Browser Integration

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

To: fvogt, #plasma, broulik, davidedmundson
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart, lukas

Reply via email to