Review Request: Save/Restore the ProjectTreeView. This enables a more seamless transition when returning to a project that was closed.

Milian Wolff mail at milianw.de
Sat Jan 7 18:15:35 UTC 2012



> On Jan. 5, 2012, 4:17 p.m., Milian Wolff wrote:
> > plugins/projectmanagerview/projecttreeview.cpp, line 329
> > <http://git.reviewboard.kde.org/r/103619/diff/1/?file=45327#file45327line329>
> >
> >     I wonder about this, as it is a ABI/source-incompatible change. How could this happen with kdelibs code?
> >     
> >     Also: I don't see setTreeView here:
> >     
> >     http://api.kde.org/4.5-api/kdelibs-apidocs/kdeui/html/kviewstatesaver_8h_source.html
> >     
> >     while the apidox in here:
> >     
> >     http://api.kde.org/4.5-api/kdelibs-apidocs/kdeui/html/classKViewStateSaver.html#8ea1426a563a88733f26f55d837946ee
> >     
> >     do refer to a "setTreeView", it is not actually listed as a method there either.
> 
> Andreas Pakulat wrote:
>     It seems like Andrew got confused by the api docs and didn't really compile against a 4.5 kdelibs. Both 4.5 and 4.6 have the setView function in the header so apparently no breakage of ABI/API or source-compatibility, just a typo/oversight in the documentation (note the typo is also in both versions of the docs)
> 
> Andrew Fuller wrote:
>     I read the API documentation for 4.5 here: http://www.purinchu.net/kdelibs-apidocs/kdeui/html/classKViewStateSaver.html which now I see has a different interface than the one at api.kde.org.
>     
>     This means the #if/#else is not needed even for 4.5.  It also means I should never rely on a mirror and always use api.kde.org.

and it also means you don't have to bump the cmake build-time requirement to 4.5


> On Jan. 5, 2012, 4:17 p.m., Milian Wolff wrote:
> > project/projectmodelsaver.cpp, line 38
> > <http://git.reviewboard.kde.org/r/103619/diff/1/?file=45330#file45330line38>
> >
> >     I'd prefer {} even for one-liner conditionals.
> >     
> >     furthermore please stay consistent with spaces, here you do:
> >     
> >     if( a )
> >     
> >     further down you do
> >     
> >     if (a)
> 
> Andrew Fuller wrote:
>     A man after my own heart!
>     
>     For the future, are {} also preferred when updating an existing file that does not use them for single-line conditionals?  (ie: which is more important, consistency within a file, or transitioning to always use {}'s?)
> 
> Andreas Pakulat wrote:
>     That depends :) If the file is consistently using a specific style you should keep to that IMHO, since consitency trumps here. But if the style is inconsistent already then I think its fine to use braces for new code or when touch an existing if-line.

yes, consistency trumps all, and if there is no consistency, use the best-practice (i.e. always {} ).


- Milian


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


On Jan. 7, 2012, 2:45 a.m., Andrew Fuller wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103619/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2012, 2:45 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> Save/Restore the ProjectTreeView.  This enables a more seamless transition when returning to a project that was closed.
> 
> 
> Diffs
> -----
> 
>   plugins/projectmanagerview/CMakeLists.txt 7b58e8e5489819b0370f5d55e0218f063da90af2 
>   plugins/projectmanagerview/projecttreeview.h 16893f0713b83a78a5b025a55fcfa082265d4419 
>   plugins/projectmanagerview/projecttreeview.cpp 8237af513627c0459d571de2bccc275e8a4b0ae9 
>   project/CMakeLists.txt bac4b22b8a9966f4efe4080a6d24e0de17c48e12 
>   project/projectmodelsaver.h PRE-CREATION 
>   project/projectmodelsaver.cpp PRE-CREATION 
>   shell/sessioncontroller.h 551894d4131218b9ab2640e28f2c354ce319a105 
>   shell/sessioncontroller.cpp d2ee8732710e0e4bded56e547d83a3906b20b018 
> 
> Diff: http://git.reviewboard.kde.org/r/103619/diff/diff
> 
> 
> Testing
> -------
> 
> Load a session, Exit, and return.  Voilà!  The tree is right where you left it.
> 
> 
> Thanks,
> 
> Andrew Fuller
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120107/422608f3/attachment.html>


More information about the KDevelop-devel mailing list