Review Request: Save workingset state per area

[Olivier Jean de Gaalon]

> On June 17, 2011, 11:19 p.m., Milian Wolff wrote:
> > shell/workingsets/workingset.cpp, line 80
> > <http://git.reviewboard.kde.org/r/101589/diff/1/?file=24319#file24319line80>
> >
> >     where has the deleteGroup gone to? is this intended?

They just got recursively deleted in the caller, so that shouldn't be needed. (I think, KConfig is a little opaque to me, and I know it had a nasty bug around deleting groups. Whoever wrote the recursivelyDelete function clearly found it).


> On June 17, 2011, 11:19 p.m., Milian Wolff wrote:
> > shell/workingsets/workingset.cpp, line 86
> > <http://git.reviewboard.kde.org/r/101589/diff/1/?file=24319#file24319line86>
> >
> >     where has the deleteGroup gone to? is this intended?

as above.


> On June 17, 2011, 11:19 p.m., Milian Wolff wrote:
> > shell/workingsets/workingset.cpp, line 206
> > <http://git.reviewboard.kde.org/r/101589/diff/1/?file=24319#file24319line206>
> >
> >     a unique separator between id and title would maybe be good to make sure? something like '|' ?! or even a subgroup?

I'll put a seperator, since the ws group should be deleted recursively on every change without touching the area specific config.


> On June 17, 2011, 11:19 p.m., Milian Wolff wrote:
> > shell/workingsets/workingset.cpp, line 212
> > <http://git.reviewboard.kde.org/r/101589/diff/1/?file=24319#file24319line212>
> >
> >     good cleanup :)

It also didn't work (as it was) if the area wasn't active. I bet it has an interesting history.


> On June 17, 2011, 11:19 p.m., Milian Wolff wrote:
> > shell/workingsets/workingset.cpp, line 251
> > <http://git.reviewboard.kde.org/r/101589/diff/1/?file=24319#file24319line251>
> >
> >     this looks unrelated?

Fixes an annoying bug, as in the desc.


> On June 17, 2011, 11:19 p.m., Milian Wolff wrote:
> > shell/workingsets/workingset.cpp, line 256
> > <http://git.reviewboard.kde.org/r/101589/diff/1/?file=24319#file24319line256>
> >
> >     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 ;-)

I'll add some comments in the code then.


- Olivier Jean de


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


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/3c573de0/attachment.html>