This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106567/

Ok, this looks very good already. I just have have a few suggestions for improving the patch even more.

Sorry, but I don't see how 'cleanStruct()' could help avoid any race conditions. Could you elaborate on that?

src/documentinfo.cpp (Diff revision 4)
const long* TextInfo::getStatistics(KTextEditor::View *view)
542
		return KUrl();
551
		return Info::url();
I don't know how much work this will be but could you try to remove the 'Info::url()' method completely? It just causes too many problems, for example when files are renamed.

It should be sufficient to use 'urlFor' from the doc manager instead.

src/kiledocmanager.cpp (Diff revision 4)
void Manager::projectOpenItem(KileProjectItem *item, bool openProjectItemViews)
1549
	//oops, doc apparently was open while the project settings wants it closed, don't trash it, update openState instead
It looks like this is still needed somewhere...

src/kiledocmanager.cpp (Diff revision 4)
QStringList Manager::loadFileContents(const KUrl& url, const QString& encoding)
2559
			KIO::Job* getJob = KIO::file_copy(url, KUrl(tmpFile.fileName()), -1, KIO::Overwrite | KIO::HideProgressInfo);
Could you use 'KIO::NetAccess::download' here? And maybe do everything in one method?

It should make the code easier to understand.

- Michel


On October 5th, 2012, 5:33 p.m., Eugene Shalygin wrote:

Review request for Kile.
By Eugene Shalygin.

Updated Oct. 5, 2012, 5:33 p.m.

Description

Kile shedules parsing of all projet items when it opens a project. But because Manager::m_projects does not contain currently loading project at that time, Manager can not resolve projet items to documents and parsing does not happen.

This patch adds curently parsing project to m_projects collection before invoking projectOpenItem() and thus parsing goes. Then we crate Info* object for each project item (if it does not exist) and sneding it to parsing.
In the next step, in Manager::handleParsingComplete(), TextInfo object is determined via opened documents list, as I understood. To install results for all project items, the patch adds a second branch that determines TextInfo object via KileProjectItem::getInfo() and the item is accessed via Manager::itemFor() (that is why project must be added to m_projects before parsing)

Also the patch changes Info* classes in a way that makes possible to parse files without loading them into KatePart, i.e. without Document object creation. This speeds up project loading. At least on my machine before most of the time was spent in spell checker initialization. We do not need a spell checker for parsing :). To parse a file we use a tiny helper function that loads file contents (getFileContents() in anonymous namespace, parserthread.cpp).

Then it tries to fix a race between parsing and cleaning of Info object by separating the cleaning code into cleanInfo() method and calling it from installParserResults()

With these changes completion works for all bib items and labels in project even if these files are not opened.

Testing

Manual testing

Diffs

  • src/documentinfo.h (16d6e49)
  • src/documentinfo.cpp (923d099)
  • src/kiledocmanager.h (5edc14e)
  • src/kiledocmanager.cpp (aa8a1d8)
  • src/parser/parserthread.cpp (4e80c41)

View Diff