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

On October 5th, 2012, 8:27 p.m., Michel Ludwig wrote:

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.

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

Well, I've tried. Indeed, it leads to many changes. Some of them are trivial, some are not. For example, Info::lastModifiedFile() uses url(). Where can I get doc manger from in that function?

Perhaps, it would be better if I remove introduced by patch changes in Info constructors, and switch to the using of doc manager in the parser thread, then you accept this patch ;) and further clean up can be done independently?

On October 6th, 2012, 8:36 a.m., Michel Ludwig wrote:

Ok, let's do it this way.

On October 6th, 2012, 1:31 p.m., Eugene Shalygin wrote:

Ups... Manager::urlFor(TextInfo* textInfo) calls Manager::itemFor(Info *docinfo, KileProject *project /*=0*/) which in turn calls Manager::itemFor(const KUrl &url, KileProject *project /*=0L*/) with docinfo->url()

Thus, by removing url from Info we destroy this chain.

On October 6th, 2012, 3:46 p.m., Michel Ludwig wrote:

Ok, let's wait with this for now then. The whole thing is a mess anyway and needs to be cleaned up!

Could you just remove the additional URL parameter that you have added from the patch?

On October 6th, 2012, 3:56 p.m., Eugene Shalygin wrote:

Sorry, I can not :) If the parser does not know the url, it can not deliver the results to a proper destination. That is why I've started all this work with adding url to Info objects, because without that, the url exists only for Infos with documents. So the idea was to provide information to parser without loading KateDocument. This includes url and file contents.

On October 6th, 2012, 5:06 p.m., Michel Ludwig wrote:

Calling 'KileDocument::Manager::urlFor' in 'DocumentParserThread::addDocument' should now work for project items as well.
Thank you! But seems like this will take more time to rework the patch then I expected.
After merging your commits, I have a crash with the following backtrace:

#5  0x000000000047f5fc in QList<QString>::QList (this=0x7fff1fc31f20, l=...) at /usr/include/qt4/QtCore/qlist.h:122
#6  0x0000000000476dcf in QStringList::QStringList (this=0x7fff1fc31f20, l=...) at /usr/include/qt4/QtCore/qstringlist.h:71
#7  0x0000000000529131 in KileDocument::Info::labels (this=0x0) at /home/eugene/develop/KDE/kile/src/documentinfo.h:127
#8  0x0000000000527698 in KileInfo::retrieveList (this=0x2970a78, getit=(QStringList (KileDocument::Info::*)(const KileDocument::Info * const)) 0x52910a <KileDocument::Info::labels() const>, docinfo=0x0) at /home/eugene/develop/KDE/kile/src/kileinfo.cpp:244
#9  0x0000000000527977 in KileInfo::allLabels (this=0x2970a78, info=0x0) at /home/eugene/develop/KDE/kile/src/kileinfo.cpp:263
#10 0x00000000005861e5 in KileWidget::StructureView::showReferences (this=0x47cb780, ki=0x2970a78) at /home/eugene/develop/KDE/kile/src/widgets/structurewidget.cpp:568
#11 0x000000000058a356 in KileWidget::StructureWidget::updateAfterParsing (this=0x3aaf9e0, info=0x47c93d0, items=...) at /home/eugene/develop/KDE/kile/src/widgets/structurewidget.cpp:1009
#12 0x00000000005ccbeb in KileDocument::Manager::handleParsingComplete (this=0x29bd820, url=..., output=0x47de0a0) at /home/eugene/develop/KDE/kile/src/kiledocmanager.cpp:2069
#13 0x00000000005d1c22 in KileDocument::Manager::qt_static_metacall (_o=0x29bd820, _c=QMetaObject::InvokeMetaMethod, _id=98, _a=0x7fe913061d20) at /home/eugene/develop/KDE/kile/build/debug/src/kiledocmanager.moc:354
#14 0x00007fe924990c2e in QObject::event(QEvent*) () from /usr/lib64/qt4/libQtCore.so.4
#15 0x00007fe9238c88dc in QApplicationPrivate::notify_helper(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#16 0x00007fe9238ca65a in QApplication::notify(QObject*, QEvent*) () from /usr/lib64/qt4/libQtGui.so.4
#17 0x00007fe925650c16 in KApplication::notify(QObject*, QEvent*) () from /usr/lib64/libkdeui.so.5
#18 0x00007fe92497bfae in QCoreApplication::notifyInternal(QObject*, QEvent*) () from /usr/lib64/qt4/libQtCore.so.4
#19 0x00007fe92497f7d1 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () from /usr/lib64/qt4/libQtCore.so.4
#20 0x00007fe9249a94a3 in postEventSourceDispatch(_GSource*, int (*)(void*), void*) () from /usr/lib64/qt4/libQtCore.so.4
#21 0x00007fe91e387035 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#22 0x00007fe91e387368 in g_main_context_iterate.isra.23 () from /usr/lib64/libglib-2.0.so.0
#23 0x00007fe91e387424 in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#24 0x00007fe9249a9636 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#25 0x00007fe9239637de in QGuiEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtGui.so.4
#26 0x00007fe92497fc7f in QCoreApplication::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from /usr/lib64/qt4/libQtCore.so.4
#27 0x00000000005c91f8 in KileDocument::Manager::projectOpen (this=0x29bd820, url=..., step=0, max=2, openProjectItemViews=false) at /home/eugene/develop/KDE/kile/src/kiledocmanager.cpp:1652
#28 0x00000000004e5b88 in Kile::restoreFilesAndProjects (this=0x2970a30, allowRestore=true) at /home/eugene/develop/KDE/kile/src/kile.cpp:1240
#29 0x00000000004d4534 in Kile::Kile (this=0x2970a30, allowRestore=true, parent=0x0, __in_chrg=<optimized out>, __vtt_parm=<optimized out>) at /home/eugene/develop/KDE/kile/src/kile.cpp:362
#30 0x00000000004fd985 in main (argc=1, argv=0x7fff1fc33f78) at /home/eugene/develop/KDE/kile/src/main.cpp:153

Perhaps, just to set Info for all items before calling projectOpenItem()?

- Eugene


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