[Differential] [Accepted] D3125: Fix ProblemsView tabs order

kfunk (Kevin Funk) noreply at phabricator.kde.org
Fri Oct 21 09:17:53 UTC 2016


kfunk accepted this revision.
kfunk added a comment.
This revision is now accepted and ready to land.


  Rest LGTM.
  
  Please target 5.0 with this patch.

INLINE COMMENTS

> problemsview.cpp:438
>  {
> +    static const QString parserTitle = QStringLiteral("Parser");
> +

Hmm, I've just noticed an issue with the implementation of the whole problem model thingie.

  KDevelop::ProblemModelSet* pms = core()->languageController()->problemModelSet();
  pms->addModel(QStringLiteral("Parser"), m_model);

^ Here, "Parser" should be translated.

In `ProblemModelSet::addModel(const QString &name, ProblemModel *model)` `name` acts both as an identifier + a human-readable string. This is wrong. We'd need to split this up and pass it both an id + the translated string.

Fine for now, though, we can't change that in 5.0.

> problemsview.cpp:448
> +
> +            if (insertIdx == 0 && tabName == parserTitle)
> +                continue;

Please add a comment why we're skipping the "Parser" tab.

> problemsview.cpp:451
> +
> +            if (tabName > data.name)
> +                break;

Please use http://doc.qt.io/qt-5/qstring.html#localeAwareCompare-1 instead

REPOSITORY
  rKDEVPLATFORM KDevPlatform

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

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/20161021/637b24b3/attachment.html>


More information about the KDevelop-devel mailing list