D7699: Add support for custom search paths for application-specific syntax and theme definitions

[Dominik Haumann]

dhaumann added a comment.


  Ok, I guess with your reasoning given this patch indeed makes sense.
  
  To me, this patch along with the unit test already looks very good. I only have minor comments, then this is good to go.
  
  @vkrause Any comments from your side?

INLINE COMMENTS

> syntaxrepository_test.cpp:18
>  
> +#include "test-config.h"
> +

Is this include required?

> syntaxrepository_test.cpp:190
> +        repo.addCustomSearchPath(testInputPath);
> +        QVERIFY(!repo.customSearchPaths().empty());
> +        QCOMPARE(repo.customSearchPaths()[0], testInputPath);

Suggestion:
QCOMPARE(!repo.customSearchPaths().size(), 1);

> repository.h:157
> +    /**
> +     * Add a custom search path to the repo.
> +     * This path will be searched in addition to the usual locations for

Please no abbreviations: repository

REPOSITORY
  R216 Syntax Highlighting

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

To: zrax, #kate, #framework_syntax_hightlighting, dhaumann
Cc: dhaumann, vkrause, cullmann, #framework_syntax_hightlighting, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170906/91f4717e/attachment.html>