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

On October 3rd, 2012, 7:12 p.m., Michel Ludwig wrote:

Ship It!

On October 3rd, 2012, 9:40 p.m., Eugene Shalygin wrote:

Dear Michel,

with commit c2a2fe238f8054206cca63d209e480809196739b full parsing is there, but why did you remove files loading without Kate? Project loading is slow... What was wrong with that?

On October 4th, 2012, 6:25 a.m., Michel Ludwig wrote:

When KatePart loads a file, it additionally performs auto-detection of the encoding of the file. Thus, the only way of ensuring that Kile's loading of files will give the same results as the one of KatePart would actually be to use the file loading mechanism of KatePart, but that's currently not possible.

On October 4th, 2012, 11:05 a.m., Eugene Shalygin wrote:

Thank you for the reply and patch rework (almost complete rework:) If you would point me the problems before, I be happy to help you with that)
I see. Indeed I forgot about encodings. But as for me it is quite a large overhead --- creating of KatePart. I still want to solve this situation in one or another way. Because what do I see in calgrind drives me crazy: to load 19 documents Kate initializes myspell 231 times and (in the same time!) hunspell --- 215 times! It is more than 20 invocations of spellchecker initializations per document. This is close to to the point of absurdity. May Kile create so many views per document? If it is not the case, I believe this has to be solved.

  Right now I see the following possibilities:

1. Use, for instance, KEncodingProber to load document-less files. This can cover majority of encodings. Besides, there, on project loading, where document-less Infos are present, we need to get only structure of includes (or another resources inclusion). Can one expect that user provides localized versions of \include/\input commands? I understand that this way adds some inconsistency between different documents loading pathes, which is not good.
It seems to me that KDevelop is using this approach and I did not see any problems with it. However I do not use other than cyrillic single byte encodings (in addition to UTF8).

2. Change KatePart behaviour in order to add possibility to exclude loading config for the document (which can initialize spellchecking, load terminal and who knows what else...)

3. Sort out things in Kate that reads config.

I would go for the first one, because it is simple for us, and seems to do less harm than benefit.

On October 4th, 2012, 1:48 p.m., Eugene Shalygin wrote:

I would like to add that if one would move towards more sophisticated latex parser, then for sure it must be implemented without KatePart. Kind of vote for number 1 :)

On October 4th, 2012, 7:11 p.m., Michel Ludwig wrote:

Hehe, no, even an extended parser would operate on QStrings only ;)

I had a closer look at KatePart's file loading code and it seems that if an encoding is preset, no auto-detection is attempted unless some kind of encoding problem occurs. As Kile saves the encodings of project items anyway, all that is necessary is to call 'setCodec' on the QTextStream. This should work in 99.999% of the cases. But can you put the file reading code in the doc manager as well? and use KIO::NetAccess for remote documents?

Of course, the ultimate solution would be to add a method to KatePart that can read documents without setting up a full KateDocument...

On October 4th, 2012, 7:22 p.m., Eugene Shalygin wrote:

Yes, but not via KatePart --- one can obtain fil contants without it, can not one?

Regarding loading: brilliant!! I'll prepare the patch against current master.
Do I understand correctly, that you want to call back doc manager for text loading? Maybe it would be better to add parseProjectItem() to parser manager? And we still need to implement encoding guessing for free files? Or to believe that we need to parse fast only project files?
No, the loading has to be done *only* for project items during the opening of the project. For stand-alone files and once a project item is open in the editor parsing should be done as it is now. 

We can add methods likde get/setDocumentContents to TextInfo that would allow to set the contents of the document as QStringList (if no document is present). In 'DocumentParserThread::addDocument' we can call 'getDocumentContents' on the TextInfo then, and 'setDocumentContents' can be called in 'loadItem' in the doc manager once the loading is complete. But yes, all the loading should be done inside the doc manager.

- Michel


On September 26th, 2012, 9:12 p.m., Eugene Shalygin wrote:

Review request for Kile.
By Eugene Shalygin.

Updated Sept. 26, 2012, 9:12 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/parser/latexparser.cpp (ea0ae0b)
  • src/parser/parserthread.h (c41123b)
  • src/parser/parserthread.cpp (ad38819)
  • src/kiledocmanager.cpp (1d652c1)
  • src/documentinfo.cpp (2c3a3bb)
  • src/documentinfo.h (27ad09f)

View Diff