Review Request 113584: Ask git if it is safe to reload a document

[Sven Brauch]

> On Nov. 4, 2013, 11:57 p.m., Milian Wolff wrote:
> > plugins/git/gitplugin.h, line 144
> > <http://git.reviewboard.kde.org/r/113584/diff/3/?file=208930#file208930line144>
> >
> >     I know it's a bit strange, but I rather would like to have this API take a QStringList of lines.
> >     
> >     That way, we can efficiently transport the document contents to the background process without copying the full document into a byte array first.

Hm, there's a reason I did it like this. We're looking for bytes on disk, not for strings. Passing a string or string list would mean we need to ask the editor for the encoding, BOM, line endings etc. in each plugin which wants to support that, and encode the text there. That sounds far less logical than asking the plugin to look for some bytes right away. You probably know that though, so if you still think the optimization is worth it, tell me, I'll change it.

The current solution is only an approximation anyways. The proper way to do it is to have an interface in kate which gives us the actual text it would write to disk -- with proper line endings and BOM (which we can only guess currently).
That would also solve this problem since we could just pass the document.

I'm also not sure why this is an optimization -- if we pass it as a string list, and encode it just before writing it to the process, we will still write it to a byte array, no? It'll just live a few nanoseconds shorter. Of course we could do it line-by-line then, but I can hardly believe this would be faster.


- Sven


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


On Nov. 3, 2013, 9:52 p.m., Sven Brauch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113584/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2013, 9:52 p.m.)
> 
> 
> Review request for KDevelop, Aleix Pol Gonzalez and Milian Wolff.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> When a file is changed on disk, we can look in the git object repository
> and see whether the contents currently in the editor are stored somewhere
> there. In that case, there is no risk of data loss when we just silently
> reload the document; the user can always retrieve the old version from
> git.
> 
> For this purpose I added a new VCS interface which version control plugins
> can choose to implement if they are able and willing to provide this
> functionality. If a plugin does not implement the interface, the old
> behaviour is unaffected.
> 
> I implemented the interface for the git plugin and it seems to be working fine.
> There's two issues we might want to talk about:
>  - We can not retrieve the line ending mode from kate. Thus, currently it will
>    only work for files with \n line endings (old behaviour is used instead for
>    different line endings).
>  - It's not blazingly fast. It's not exactly slow, but if you have like 35
>    documents open, the freeze on switching to a completely different branch
>    is noticeable. Test it yourself and give your opinion on whether this
>    is acceptable or not.
> 
> 
> Diffs
> -----
> 
>   plugins/git/gitplugin.h 2f60c24b9d223a815eda6627d1328ce2404e11af 
>   plugins/git/gitplugin.cpp 27f4eaeea46afdfccb88d64caf161cc84a1e5bad 
>   shell/textdocument.h 8bb29fea8395d4cac956778fae85a4a9f57c2cce 
>   shell/textdocument.cpp 187a071d78c3e2d00092bb2adde533486be11eee 
>   vcs/interfaces/icontentawareversioncontrol.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/113584/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sven Brauch
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20131105/7f344bb9/attachment-0001.html>