[Differential] [Updated] D4430: WIP: Port ClangClassHelper to also fill the include_files var

[Milian Wolff]

mwolff added a comment.


  lgtm in principle

INLINE COMMENTS

> clangclasshelper.cpp:151
>          sourceUrl = urls.constBegin().value();
>      }
>      else {

while at it: join line

> unknowndeclarationproblem.cpp:320
>   */
> -ClangFixit directiveForFile( const QString& includefile, const KDevelop::Path::List& includepaths, const KDevelop::Path& source )
> +ClangFixit directiveForFileFitIt(const QString& includefile, const KDevelop::Path::List& includepaths, const KDevelop::Path& source)
>  {

fitIt?! what? :P

here and below: remove excess KDevelop:: qualification (see above, namespace is used)

> unknowndeclarationproblem.cpp:565
> +// TODO: method an experimental hook to get access to all the logic in this file, where to put this best?
> +QString UnknownDeclarationProblem::includeDirectiveArgumentFromPath(const KDevelop::Path& file,
> +                                                                    const KDevelop::DeclarationPointer& declaration)

needs a unit test, missing so far if I'm not mistaken

util stuff is usually put into duchainutils

> unknowndeclarationproblem.cpp:571
> +    on the file passed. but those files might not exist yet, are only planned to be created by the generator.
> +    What happens exactly in this call then and what to do?
> +*/

look it up yourself :) potentially there is a fallback, alternatively do the fallback yourself. afaik the project managers will usually return $something for a folder as well.

> unknowndeclarationproblem.cpp:584
> +        // TODO: check also scanIncludePaths(identifier, includepaths);?
> +        // cmp. includeFiles(), but no clue where to take identifier from or if it makes sense
> +

that is for finding an include from the name of a class (i.e. an identifier) - doesn't seem to be relevant here

> unknowndeclarationproblem.cpp:591
> +
> +    /* create fixits for candidates */
> +    for (const auto& includeFile : includefiles) {

wrong comment

REPOSITORY
  R32 KDevelop

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

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

To: kossebau, #kdevelop, mwolff
Cc: kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170203/98713c06/attachment-0001.html>