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

Fabian Wiesel fabian.wiesel at googlemail.com
Sun Jan 17 14:19:10 UTC 2010



> 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.

Only the this-pointer or all the elements?


> 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.

Sure, will do that.


> 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?

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.


- Fabian


-----------------------------------------------------------
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
> 
>





More information about the KDevelop-devel mailing list