Review Request: Move KNewMenu from libkonq to kdelibs.

[Aaron Seigo]

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


getting rid of duplicated code is terrific :) i have some questions/comments about the class definitions that will join the public API:


/trunk/KDE/kdelibs/kfile/knameandurlinputdialog.h
<http://reviewboard.kde.org/r/2629/#comment3075>

    should the parent have a default of 0 like other QObject classes to to?
    
    similar for the other members, do they all need values or can they have defaults? since setSuggestedName and setSuggestedUrl already exist to do this, the ctor could even be stripped back to just KNameAndUrlInputDialog(QWidget *parent = 0)?



/trunk/KDE/kdelibs/kfile/knameandurlinputdialog.h
<http://reviewboard.kde.org/r/2629/#comment3076>

    looking at the api i expected a getter; and then i thought maybe name() would be that getter.
    
    perhaps setCurrentName(const QString &name) and QString currentName() const would be clearer API? (similar for setSuggestedUrl below?)



/trunk/KDE/kdelibs/kfile/knewfilemenu.h
<http://reviewboard.kde.org/r/2629/#comment3077>

    parentWidget should be the last parameter? can it be defaulted to null?
    
    personally, i'd change parent to actionCollection and parentWidget to just parent; more in-line with existing API



/trunk/KDE/kdelibs/kfile/knewfilemenu.h
<http://reviewboard.kde.org/r/2629/#comment3079>

    getters?



/trunk/KDE/kdelibs/kfile/knewfilemenu.h
<http://reviewboard.kde.org/r/2629/#comment3078>

    should "slot" be removed from its name?



/trunk/KDE/kdelibs/kfile/knewfilemenu.h
<http://reviewboard.kde.org/r/2629/#comment3080>

    should slot be remove from its name? perhaps jobResult(KJob *job)?


- Aaron


On 2010-01-15 21:09:00, David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2629/
> -----------------------------------------------------------
> 
> (Updated 2010-01-15 21:09:00)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> Move KNewMenu from libkonq to kdelibs/kfile, so that http://reviewboard.kde.org/r/2535/ can use it (in order to add "RMB / New File" in kdiroperator, which benefits KFileDialog and other users of kdiroperator).
> 
> Since libkonq is public API, the "moving" needs a renaming (and the existing classes will use the moved ones), so the new class name is KNewFileMenu. The dialog for entering name+url is moved too, as KNameAndUrlInputDialog [should this go to libkio instead? better decide now or we'll need to change the name again].
> 
> The good thing is that this gets rid of some code duplication for creating directories: the libkonq code moving to libkfile means that KDirOperator::mkdir can now use it, rather than its own fork of it. Done in this patch, so as a side effect this patch actually adds RMB / Create New in kdiroperator - taken from 2535 :) I'll let Abhishek finish it though (adding the support for "only showing the mimetypes that are relevant to the current application").
> 
> 
> This addresses bug 93230.
>     https://bugs.kde.org/show_bug.cgi?id=93230
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kfile/CMakeLists.txt 1071368 
>   /trunk/KDE/kdelibs/kfile/kdiroperator.h 1071368 
>   /trunk/KDE/kdelibs/kfile/kdiroperator.cpp 1071368 
>   /trunk/KDE/kdelibs/kfile/knameandurlinputdialog.h PRE-CREATION 
>   /trunk/KDE/kdelibs/kfile/knameandurlinputdialog.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/kfile/knewfilemenu.h PRE-CREATION 
>   /trunk/KDE/kdelibs/kfile/knewfilemenu.cpp PRE-CREATION 
> 
> Diff: http://reviewboard.kde.org/r/2629/diff
> 
> 
> Testing
> -------
> 
> RMB / Create New in KFileDialog. Creating a dir still enters it like it did before.
> 
> 
> Thanks,
> 
> David
> 
>