Review Request: Save workingset state per area

Milian Wolff mail at milianw.de
Fri Jun 17 23:19:21 UTC 2011


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


generally looks good to me, but there are a few questions (see below). Esp. some comments on the new code would help me tremendously :)


shell/workingsets/workingset.cpp
<http://git.reviewboard.kde.org/r/101589/#comment3256>

    maybe use setGroup instead of wsGroup? just a personal style question though, ignore me if you don't like it :)



shell/workingsets/workingset.cpp
<http://git.reviewboard.kde.org/r/101589/#comment3258>

    where has the deleteGroup gone to? is this intended?



shell/workingsets/workingset.cpp
<http://git.reviewboard.kde.org/r/101589/#comment3257>

    where has the deleteGroup gone to? is this intended?



shell/workingsets/workingset.cpp
<http://git.reviewboard.kde.org/r/101589/#comment3259>

    a unique separator between id and title would maybe be good to make sure? something like '|' ?! or even a subgroup?



shell/workingsets/workingset.cpp
<http://git.reviewboard.kde.org/r/101589/#comment3260>

    good cleanup :)



shell/workingsets/workingset.cpp
<http://git.reviewboard.kde.org/r/101589/#comment3261>

    this looks unrelated?



shell/workingsets/workingset.cpp
<http://git.reviewboard.kde.org/r/101589/#comment3262>

    some documents would be cool on your new code here, I know the old code is not documented either, but you now seem to have understood it to some degree. please help us other poor souls ;-)


- Milian


On June 12, 2011, 3:01 a.m., Olivier Jean de Gaalon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101589/
> -----------------------------------------------------------
> 
> (Updated June 12, 2011, 3:01 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> This patch saves and loads certain area-specific states for each working set, in particular, it saves
> *The document state (cursor position, or selection)
> *The currently active document
> *The order of the documents
> For each area, so that when you reopen KDevelop, all is as you left it, in each area.
> 
> Also, this fixes a bug where when you split a view and then unsplit it (close the split off view), when you restarted KDevelop it would come back (unless you closed the entire working set or closed the split view in *all* the areas).
> 
> If this fixes any reported bugs, let me know what they are.
> 
> It's possible this isn't the best way to do this as I'm not familiar with this area at all, but it's unlikely to break horribly, and is even backwards compatible (ie, you won't lose your current workingsets).
> 
> 
> Diffs
> -----
> 
>   shell/workingsets/workingset.h 512c5cb 
>   shell/workingsets/workingset.cpp c494e51 
>   sublime/areaindex.h 7dfc13f 
> 
> Diff: http://git.reviewboard.kde.org/r/101589/diff
> 
> 
> Testing
> -------
> 
> Starting and stopping kdevelop, splitting and unsplitting, restarting kdevelop, reordering documents, changing cursor positions, restarting kdevelop...
> 
> 
> Thanks,
> 
> Olivier Jean de
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20110617/646a8ee4/attachment.html>


More information about the KDevelop-devel mailing list