[Differential] [Updated] D3291: Fix problems tooltip

[kfunk (Kevin Funk)]

kfunk added a comment.


  Rest LGTM, nice idea! Please update the patch, then I'll have another look.

INLINE COMMENTS

> problemnavigationcontext.cpp:85
> +
> +    if (a->severity() < b->severity())
> +      return true;

When sorting by multiple sort criterias I favor this pattern here: http://stackoverflow.com/a/3574724

Or, let's be fancy and use C++11 `std::tie`: http://stackoverflow.com/a/17080762

> problemnavigationcontext.cpp:115
> +/// therefore we should make check for this case.
> +QString ProblemNavigationContext::html(const QString& text) const
>  {

Shouldn't this method be called `escapedHtml` or something?

> problemnavigationcontext.cpp:263
>  
> -  auto action = assistant->actions().value(index);
> +  auto action = m_assistantsActions[index];
>    if (action) {

`.at(...)`

And you should assert that `action != nullptr` in that case then. No need for the else-branch.

REPOSITORY
  rKDEVPLATFORM KDevPlatform

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: antonanikin, #kdevelop, kfunk
Cc: kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20161116/bf610e05/attachment.html>