> On Sept. 1, 2012, 12:03 p.m., Frank Reininghaus wrote:
> > First of all, thanks for the patch!
> > 
> > I was not aware of any "constant breakage of the Konqueror directory 
> > filtering plugin once Konqueror was switched to use Dolphin's part for 
> > filemanagement", and I think neither was Peter.
> > 
> > I checked it now, and can confirm that mime type filtering does not work 
> > (to be honest, I never noticed this menu entry in Konqueror before), but 
> > did this actually ever work in the DolphinPart? A quick look at the 
> > DolphinView code from KDE 4.7 didn't show me anything that filters mime 
> > types. What else is broken about the filtering functionality at the moment? 
> > Filtering using ~/*.jpg in Konqueror's location bar works for me.
> > 
> > Moreover, what are the new signals itemsAdded() and itemsDeleted() needed 
> > for? They could be confused easily with the existing signals 
> > itemsInserted() and itemsRemoved(). Therefore, I think we should not add 
> > these new signals unless there is a *very* good reason to have them.

The breakage is not really the fault of the DolphinPart, but rather the fact 
that access to crucial signals from KDirLister, the equivalent of the 
"itemsAdded" and "itemsDeleted", was no longer available as it was in the 
original Konqueror filemanagement module. It is also true that the plugin never 
worked correctly once Konqueror was ported to use DolphinPart for 
filemanagement. However, that situation was sort of addressed by myself (due to 
a bug report) using a very ugly hack a while back. The hack simply attempts to 
walk through the a Part's child classes and find an instance of a KDirLister 
and connect to its newItems and itemsDeleted signals. As is the case with most 
hacks, any change in the code is bound to break any assumptions made by the 
hack and that is exactly what transpired once Dolphin was re-written for its 
2.0 release.

As far as the two new signals I added to KFileItemModel and DolphinView, 
itemsAdded and itemsDeleted, this whole patch will be useless without them. 
They are very important because these new signals are not about what is 
happening to the current DolphinView. Rather they are about what is going on in 
the actual directory the view is displaying. Simply put, the new signals are 
ONLY emitted when the user adds or deletes an item (files or sub directories) 
in current directory. The old signals on the other hand are emitted when 
anything changes in the view itself, e.g. the view is filtered by a name.


- Dawit


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


On Aug. 31, 2012, 7:30 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106289/
> -----------------------------------------------------------
> 
> (Updated Aug. 31, 2012, 7:30 p.m.)
> 
> 
> Review request for Dolphin and KDE Base Apps.
> 
> 
> Description
> -------
> 
> The attached patch provides an implementation of KParts' 
> ListingFilterExtension for Dolphin. The extension allows the Dolphin KPart to 
> provide directory/file filtering services without requiring direct linking 
> against Dolphin itself.
> 
> The review for the new KPart listing filter extension 
> (ListingFilterExtension) can be found at 
> https://git.reviewboard.kde.org/r/106288/
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinpart.h e5693b3 
>   dolphin/src/dolphinpart.cpp fff7dc0 
>   dolphin/src/kitemviews/kfileitemmodel.h d9bebdf 
>   dolphin/src/kitemviews/kfileitemmodel.cpp 6936af4 
>   dolphin/src/kitemviews/private/kfileitemmodelfilter.h 9bdf1fd 
>   dolphin/src/kitemviews/private/kfileitemmodelfilter.cpp 816d356 
>   dolphin/src/views/dolphinview.h 10f63c5 
>   dolphin/src/views/dolphinview.cpp 8050415 
> 
> Diff: http://git.reviewboard.kde.org/r/106289/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dawit Alemayehu
> 
>

Reply via email to