Review Request: VcsEventWidget: Fetch only the last n events from the history, request earlier ones as needed

[Andreas Pakulat]

> On 2010-01-17 14:07:56, Andreas Pakulat wrote:
> > /trunk/extragear/sdk/kdevplatform/vcs/vcsevent.h, line 103
> > <http://reviewboard.kde.org/r/2638/diff/2/?file=17252#file17252line103>
> >
> >     This seems to be an unrelated change, would be good to have that in a separate commit.
> 
> Fabian Wiesel wrote:
>     Sure, will do that.

I actually didn't mean to add a new review request for it, just to commit it separately from the rest :)


> On 2010-01-17 14:07:56, Andreas Pakulat wrote:
> > /trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.cpp, line 124
> > <http://reviewboard.kde.org/r/2638/diff/2/?file=17251#file17251line124>
> >
> >     This looks strange, I don't see any elements being inserted here, so why does the model tell the view it got new data?
> 
> Fabian Wiesel wrote:
>     I add there some place holders, which I return in line 177: ("Loading Data...",  "Loading Data...", ...)
>     This gives the user some feedback, that there are elements, but they aren't simply not loaded yet.
>     Granted, the model is probably not the right place, to do that.

Ah, ok, missed that apparently. Well its not too bad in the model, as adding it to the view is relatively hard and outside of it takes away precious vertical space...


> On 2010-01-17 14:07:56, Andreas Pakulat wrote:
> > /trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.cpp, line 122
> > <http://reviewboard.kde.org/r/2638/diff/2/?file=17251#file17251line122>
> >
> >     This would look nice as initialization list.
> 
> Fabian Wiesel wrote:
>     Only the this-pointer or all the elements?
>

All member initializations, is mostly just a matter of style, but these days I like the initializer list (unless initialization depends on some computations)


- Andreas


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


On 2010-01-17 13:16:28, Fabian Wiesel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2638/
> -----------------------------------------------------------
> 
> (Updated 2010-01-17 13:16:28)
> 
> 
> Review request for KDevelop and Andreas Pakulat.
> 
> 
> Summary
> -------
> 
> VcsEventModel now has a IBasicVersionControl, so that it can fetch the events as they are requested by the view.
> While it now supports the canFetchMore()/fetchMore() interface of QAbstractItemModel, further elements would
> only be requested, if the bottom has been reached.
> To hide the latency of fetching further events, they are internally prefetched, when a low watermark is reached.
> The fields of the elements, which are currently fetched are returned by the model as "Loading...", so that the user
> gets a feedback, that those fields are not yet available.
> 
> It relies on the following properties, which are not specified by the IBasicVersionControl::log() in that way,
> but happens to be the case with the svn-plugin and (after that patch to DVcsJob) the hg-plugin.
> - log() returns _at least_ "limit" events, or there are fewer events left than requested. This is required,
>    so that the model can answer canFetchMore() correctly, requires only an update of the specification of log().
> - finished() is fired _after_ resultsReady() (patch toDVcsJob)
> 
> VcsEventWidget also works directly with IBasicVersionControl, instead indirectly by getting it from the job.
> 
> Finally, only indirectly related, some const correctess in VcsEvent.
> 
> 
> Diffs
> -----
> 
>   /trunk/extragear/sdk/kdevplatform/vcs/dvcs/dvcsjob.cpp 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.h 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/models/vcseventmodel.cpp 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/vcsevent.h 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/vcsevent.cpp 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/vcspluginhelper.cpp 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/widgets/vcseventwidget.h 1075986 
>   /trunk/extragear/sdk/kdevplatform/vcs/widgets/vcseventwidget.cpp 1075986 
> 
> Diff: http://reviewboard.kde.org/r/2638/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Fabian
> 
>