Re: Review Request: Make KFileDialog remember settings

2012-10-02 Thread Aurélien Gâteau
On Sept. 30, 2012, 8:26 a.m., David Faure wrote: Looks good to me (feel free to commit if nobody else has comments). But then, what if someone is annoyed by icon views, and wants a details view everywhere? [switch this sentence around if the default is details view] He'll have to

Re: Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

2012-10-02 Thread David Faure
On Sept. 29, 2012, 9:20 a.m., David Faure wrote: konqueror/src/konqsessionmanager.cpp, line 450 http://git.reviewboard.kde.org/r/106503/diff/3/?file=87618#file87618line450 Doesn't this lose ItemIsSelectable? I guess item-flags() | Qt::ItemIsUserCheckable would be better. Dawit

Re: Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

2012-10-02 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106503/#review19731 --- konqueror/src/konqsessionmanager.h

Re: Review Request: Make KFileDialog remember settings

2012-10-02 Thread David Faure
On Sept. 30, 2012, 8:26 a.m., David Faure wrote: Looks good to me (feel free to commit if nobody else has comments). But then, what if someone is annoyed by icon views, and wants a details view everywhere? [switch this sentence around if the default is details view] He'll have to

Re: Review Request: Fix whitespace related bugs when listing directories in kio_ftp

2012-10-02 Thread David Faure
On Oct. 1, 2012, 8:30 a.m., David Faure wrote: kioslave/ftp/ftp.cpp, line 2639 http://git.reviewboard.kde.org/r/106636/diff/2/?file=87785#file87785line2639 Does this really work? This method is being called in the middle of the parsing of list command output, so the output of the

Re: Review Request: Add yet another code generation option for having invokable methods

2012-10-02 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103815/#review19738 --- Looks good, but a corresponding unit test is missing (the

Re: Review Request: Make KFileDialog remember settings

2012-10-02 Thread Aurélien Gâteau
On Sept. 30, 2012, 8:26 a.m., David Faure wrote: Looks good to me (feel free to commit if nobody else has comments). But then, what if someone is annoyed by icon views, and wants a details view everywhere? [switch this sentence around if the default is details view] He'll have to

Re: Review Request: Make KFileDialog remember settings

2012-10-02 Thread Aurélien Gâteau
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106581/ --- (Updated Oct. 2, 2012, 10:12 a.m.) Review request for kdelibs. Changes

Re: Review Request: Make KFileDialog remember settings

2012-10-02 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106581/#review19741 --- kfile/kfilewidget.cpp

Re: Review Request: Make KFileDialog remember settings

2012-10-02 Thread David Faure
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106581/ --- (Updated Oct. 2, 2012, 10:55 a.m.) Review request for kdelibs and Andreas

Re: Review request reminder: kded-appmenu

2012-10-02 Thread Cedric Bellegarde
Le samedi 29 septembre 2012 18:05:45 Alex Fiestas a écrit : I was wondering, can't the kded be loaded on demand? what if somebody doesn't care about appmenu? Hello, kded-appmenu was loaded on demand but with last version, i disable this because with Export/TopMenubar mode, nobody is using

Re: Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

2012-10-02 Thread Thomas Lübking
On Oct. 2, 2012, 8:57 a.m., David Faure wrote: konqueror/src/konqsessionmanager.cpp, line 166 http://git.reviewboard.kde.org/r/106503/diff/4/?file=88056#file88056line166 Does this really make any difference? Widgets are transparent by default, in Qt4... To be more aggressive: DO

Re: Review Request: Make KFileDialog remember settings

2012-10-02 Thread Aurélien Gâteau
On Oct. 2, 2012, 10:55 a.m., David Faure wrote: kfile/kfilewidget.cpp, line 1875 http://git.reviewboard.kde.org/r/106581/diff/3/?file=88080#file88080line1875 OK, I was about to react against the move to a member variable configGroup (which basically means accept() will use the

Re: Review Request: Add Activity Awareness to KFilePlaces* Widget (OnlyInActivity)

2012-10-02 Thread David Faure
On Oct. 25, 2011, 10:05 a.m., Kai Uwe Broulik wrote: Is there any progress on this? I’d propose you enhance the feature to make the individual activities selectable, i.e. you can choose in which particular activities an entry should appear, similar to KWin’s Activity menu in the

Re: Review Request: Add Activity Awareness to KFilePlaces* Widget (OnlyInActivity)

2012-10-02 Thread Stephen Kelly
David Faure wrote: If we (=someone) make a frameworks branch for kactivities, it should be quite simple to port that away from kdecore (I suppose it was mostly using KPluginLoader, which has now moved to the kservice framework). So this might just be a matter of updating a few CMakeLists.txt

Re: Review Request: Add Activity Awareness to KFilePlaces* Widget (OnlyInActivity)

2012-10-02 Thread Aaron J. Seigo
On Oct. 25, 2011, 10:05 a.m., Kai Uwe Broulik wrote: Is there any progress on this? I’d propose you enhance the feature to make the individual activities selectable, i.e. you can choose in which particular activities an entry should appear, similar to KWin’s Activity menu in the

Re: Review Request: Add Activity Awareness to KFilePlaces* Widget (OnlyInActivity)

2012-10-02 Thread Ivan Čukić
making a frameworks branch in the kactivities repo is fine. the library should indeed be easy to port. The core library currently uses only KDebug, i18nc and KUrl. The data models library, the service and the workspace addons use more stuff. So, the core library (that is meant to be used in

Porting KActivities to KF5 (Re: Review Request: Add Activity Awareness to KFilePlaces* Widget (OnlyInActivity))

2012-10-02 Thread David Faure
On Tuesday 02 October 2012 19:47:31 Ivan Čukić wrote: making a frameworks branch in the kactivities repo is fine. the library should indeed be easy to port. The core library currently uses only KDebug, i18nc and KUrl. - qDebug, QUrl, and either tr or the ki18n framework, depending on the

Re: Porting KActivities to KF5 (Re: Review Request: Add Activity Awareness to KFilePlaces* Widget (OnlyInActivity))

2012-10-02 Thread Ivan Čukić
In short, the whole libs / runtime split idea from KDE4, is gone in KF5, replaced with the more modular here is all you need for technology xyz. I know. That is the reason why I decided to keep it in one repository in the first place - to have it as close as possible to the layout it will

Re: Review Request: Fix whitespace related bugs when listing directories in kio_ftp

2012-10-02 Thread Dawit Alemayehu
On Oct. 1, 2012, 8:30 a.m., David Faure wrote: kioslave/ftp/ftp.cpp, line 2639 http://git.reviewboard.kde.org/r/106636/diff/2/?file=87785#file87785line2639 Does this really work? This method is being called in the middle of the parsing of list command output, so the output of the

Re: Review Request: Check if the startDir has an schema before discarting it

2012-10-02 Thread Commit Hook
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106475/#review19779 --- This review has been submitted with commit

Re: Review Request: Use product as text for pluggable/removable devices

2012-10-02 Thread Alex Fiestas
On Sept. 30, 2012, 12:23 a.m., Alex Fiestas wrote: I decided to make a review because I wonder why we were using description before, was it working better with HAL? Kevin Ottens wrote: Nope, but the HAL backend itself used to have a very similar piece of code. It's probably a

Re: Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

2012-10-02 Thread Dawit Alemayehu
On Oct. 2, 2012, 8:57 a.m., David Faure wrote: konqueror/src/konqsessionmanager.cpp, line 166 http://git.reviewboard.kde.org/r/106503/diff/4/?file=88056#file88056line166 Does this really make any difference? Widgets are transparent by default, in Qt4... Thomas Lübking wrote:

Re: Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

2012-10-02 Thread Dawit Alemayehu
On Oct. 2, 2012, 8:57 a.m., David Faure wrote: konqueror/src/konqsessionmanager.cpp, line 257 http://git.reviewboard.kde.org/r/106503/diff/4/?file=88056#file88056line257 in the... ? :-) (unfinished sentence) It's easier to just move the connect to the block

Re: Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

2012-10-02 Thread Dawit Alemayehu
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106503/ --- (Updated Oct. 2, 2012, 9:07 p.m.) Review request for KDE Base Apps and

Re: Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

2012-10-02 Thread Thomas Lübking
On Oct. 2, 2012, 8:57 a.m., David Faure wrote: konqueror/src/konqsessionmanager.cpp, line 166 http://git.reviewboard.kde.org/r/106503/diff/4/?file=88056#file88056line166 Does this really make any difference? Widgets are transparent by default, in Qt4... Thomas Lübking wrote: