D7742: KDevelop projects: optional use of KDirWatch and defer it and parser start to after project loading

[René J.V. Bertin]

rjvbb marked 3 inline comments as done.
rjvbb added inline comments.

INLINE COMMENTS

> mwolff wrote in abstractfilemanagerplugin.cpp:120
> if the setting has changed, you still may have a watcher from before which you want to delete here. I don't think you should handle the setting, but rather make the code fail gracefully if no watcher exists for the given project:
> 
>   delete m_watchers.value(project, nullptr);
>   m_watchers.remove(project);

Your code snippet looks like it achieves the same thing as my code. I take it your main suggestion was to remove the check for watchAllProjectDirectories()?

> mwolff wrote in abstractfilemanagerplugin.cpp:396
> if the setting has changed, you still may have a watcher from before which you want to stop here, see above

Question is, should the watcher be deleted here if dirwatching is turned off?

> mwolff wrote in abstractfilemanagerplugin.cpp:410
> dito, but slightly changed - if the setting changed, did you recreate the watchers?

The answer to that is "no, not unless the project was reloaded". Dirwatcher creation remains coupled to project import, and the checkbox callback doesn't trigger one. Just like the "parse all project files" it only affects future project imports.

So this check must stay.

> mwolff wrote in projectcontroller.cpp:916
> this should be a separate patch, it has nothing to do with dirwatches

Exactly what should be a separate patch, in fact? The whole timing logic and KDEV_DONT_DEFER_PROJECT_PARSING option is not intended for committing. Unless someone wants it to go in, in that case we can make it a separate commit of course.

> mwolff wrote in projectcontroller.cpp:1288
> this looks wrong, add the API to IProject and call the virtual directly, obsoleting this whole method

Adding it to IProject would mean accessing the higher level IProjectFileManager class from there. That seems wrong too, so I've made a `virtual bool IProjectFileManager createWatcher(IProject*)` method.

Should I check the return value from `IProject::projectFileManager()`?

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, mwolff, brauch
Cc: mwolff, kossebau, arrowdodger, brauch, zhigalin, kdevelop-devel, geetamc, Pilzschaf, akshaydeo, surgenight
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170918/637bddb0/attachment.html>