D11934: KDevelop : "Reparse Entire Project" action in the project manager context menu

[Milian Wolff]

mwolff added inline comments.

INLINE COMMENTS

> rjvbb wrote in parseprojectjob.h:36
> In fact, what do you think of making this
> 
>   explicit ParseProjectJob(KDevelop::IProject* project, bool forceUpdate = false, bool forceAll = ICore::self()->projectController()->parseAllProjectSources() );
> 
> You'd need to include 2 icore headers in this file but it'd make the behaviour of the option easier to explain. But we'd probably also need to do the same thing for the corresponding new argument to ProjectController::reparseProject() (but not iProjectController::reparseProjet(), I think)?

please don't do that. rather, reuse `ProjectController::reparseProject` below instead of creating the job manually

> rjvbb wrote in projectcontroller.cpp:247
> Well, the routine always checked if only a single project was selected because opening multiple project config dialogs isn't a very good idea (if not only because they're modal dialogs AFAICT).
> 
> So the answer to your question is "yes" :) We need access to the first, last, one-and-only element.
> 
> To be honest, I'm not convinced by the idea of making this collection a QSet. I doubt a performance argument carries much weight here and a QList seems more logical; elsewhere this information is used to loops over the projects. I for one always expect some influence of the order in which I select targets on the order in which commands are executed. I know that isn't always true but if it's not the execution order is usually the order in which the targets are displayed in the list. With a QSet the execution order will be unspecified.

deref the iterator returned by `QSet::begin()`

your QList won't work when you have multiple folders in a single project selected, you'll get a list with two entries, both pointing to the same project.

> projectcontroller.cpp:256
>              // otherwise base on selection
>              ProjectItemContext* ctx =  dynamic_cast<ProjectItemContext*>(ICore::self()->selectionController()->currentSelection());
>              if (ctx) {

double whitespace

> rjvbb wrote in projectcontroller.cpp:1141
> Is there any other reason why this could not go into the current release branch?

it's not a bug fix, and you add new strings too

> projectcontroller.cpp:1145
> +                }
> +                d->m_parseJobs[project] = new KDevelop::ParseProjectJob(project, true, true);
> +                ICore::self()->runController()->registerJob(d->m_parseJobs[project]);

use `ProjectController::reparseProject`

> projectcontroller.cpp:1264
>  
>      d->m_parseJobs[project] = new KDevelop::ParseProjectJob(project, forceUpdate);
>      ICore::self()->runController()->registerJob(d->m_parseJobs[project]);

expand this here

REPOSITORY
  R32 KDevelop

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

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