D22062: Addition of php script output patterns. Initial development.

Friedrich W. H. Kossebau noreply at phabricator.kde.org
Thu Jul 11 15:41:29 BST 2019


kossebau added a comment.


  Hm, seeing all this hardcoded code for optional plugins, this calls out for someone to look into making this something pulled in from the language plugins instead :) Not your fault, just mentioning the obvious. So should be fine to just add here.
  
  Some quick principal API/ABI comments added, otherwise added @pprkut as PHP contributor to give this a look, not anywhere my personal domain :)

INLINE COMMENTS

> outputfilteringstrategies.h:84
>  public:
> -    ScriptErrorFilterStrategy();
> +    ScriptErrorFilterStrategy(const QString& scriptdir = QString());
>  

Make constructor `explicit`, scriptdir -> scriptDir.
To improve over the existing code, please add some API dox what value exactly is expected with th argument scriptDir, not immediately obvious to me (as non-.script code developer).

> outputfilteringstrategies.h:90
> +private:
> +    QString scriptDir;
>  };

For consistency, please add instead a pimpl

  const QScopedPointer<class ScriptErrorFilterStrategyPrivate> d;

similar to CompilerFilterStrategy

REPOSITORY
  R32 KDevelop

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

To: santilin, #kdevelop, pprkut
Cc: kossebau, kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190711/1e4d8be8/attachment.html>


More information about the KDevelop-devel mailing list