Review Request: Use the KVersionControlPlugin for the next highest folder not the first one that finds a folder anywhere up in the filesystem

[Dominik Schmidt]

> On Oct. 27, 2012, 1:38 p.m., Frank Reininghaus wrote:
> > Thanks for the patch! Good catch, I wasn't aware of this problem.
> > 
> > Looks good, I'm just wondering if we could maybe do it in a slightly simpler way, without the new function: Maybe one could move the code starting with "// Version control systems..." out of the foreach-loop, then replace "while (upUrl != dirUrl)" by "if (upUrl != dirUrl)" and have the function call itself recursively if that is the case?

Of course we could squeeze it all into one big function, but I'd find it way harder to read - just matter of taste. 

Also I'm not exactly sure how/what the static locals are meant to work/do and I'd be afraid to break stuff when changing too much and recursively calling that function ;-)


- Dominik


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


On Oct. 26, 2012, 11:52 p.m., Dominik Schmidt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107066/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2012, 11:52 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Description
> -------
> 
> Sorry, for the confusing summary, couldn't describe it better in one sentence, suggestions for a commit message are very much appreciated.
> 
> 
> The current behaviour of dolphin is:
> if for the first plugin in the list of loaded plugins the fileName is not found in the current directory, it looks in the directory above, if it's not found there it looks in the directory above and so on - all with the same plugin.
> This leads to a problematic situation with for example Dropbox. Dropbox-Sync folders are identified by the .dropbox folder in their top-level. but also there is a ~/.dropbox. So when you enable the dolphin-box-plugin, basically no other KVersionControlPlugin will be able to work. It will go up to your home and think everything inside it is tracked by Dropbox.
> 
> This patch makes dolphin check all plugins for the current directory, then go one directory up, check all plugins again, go one up etc.. 
> 
> 
> P.S.: Is there any better/more flexible API to use for sync-plugins? 
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/versioncontrol/versioncontrolobserver.h 501af7d 
>   dolphin/src/views/versioncontrol/versioncontrolobserver.cpp 42e00de 
> 
> Diff: http://git.reviewboard.kde.org/r/107066/diff/
> 
> 
> Testing
> -------
> 
> I can use other plugins simultaneously with the Dropbox one now... 
> 
> 
> Thanks,
> 
> Dominik Schmidt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20121028/ad1c58d9/attachment.htm>