Review Request 118718: Add settings dialog to add additional compilers.

[Milian Wolff]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118718/#review60690
-----------------------------------------------------------


I'd like to get rid of the dummy factory - I don't see what we gain with it. The DummyCompiler might make sense for the case where kdevelop is run on a system without any compiler (neither gcc or clang nor msvc) available. In such a case though, we can directly create the compiler and don't have to go through the factory registry in arcane ways.

Furthermore, I'd then propose to rename the DummyCompiler to something like UnavailableCompiler, NoCompiler, or similar. DummyXYZ sounds like a mock class used in tests.


languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
<https://git.reviewboard.kde.org/r/118718/#comment42316>

    override



languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
<https://git.reviewboard.kde.org/r/118718/#comment42317>

    override



languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
<https://git.reviewboard.kde.org/r/118718/#comment42323>

    again, this should either directly create a dummy compiler, or - better - get rid of the dummy compiler thingy in general and fall-back to a system compiler like gcc or clang.



languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
<https://git.reviewboard.kde.org/r/118718/#comment42318>

    why is a dummy compiler created on startup? why is it registered?



languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
<https://git.reviewboard.kde.org/r/118718/#comment42319>

    instead, just directly create a dummy compiler in these cases
    
    setCompiler(it.key(), new DummyCompiler);
    
    this way, you could simplify the other logic where you need to filter based on the factory name.
    
    but generally, what's the dummy compiler there for? shouldn't the project then fall back to one of the system compilers, like GCC or clang? I don't quite get this.



languages/plugins/custom-definesandincludes/kcm_widget/compilersmodel.h
<https://git.reviewboard.kde.org/r/118718/#comment42320>

    lowercase



languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.cpp
<https://git.reviewboard.kde.org/r/118718/#comment42321>

    why that? generally pretty dangerous and makes the code harder to debug. what do you try to achieve?



languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.cpp
<https://git.reviewboard.kde.org/r/118718/#comment42322>

    prefer to use QString() instead of ""
    
    add a comment like "add compiler without any information, wait for the user to fill in the data"


- Milian Wolff


On June 21, 2014, 5:07 p.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118718/
> -----------------------------------------------------------
> 
> (Updated June 21, 2014, 5:07 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> Now there are compilers that auto-detected and those that added manually. The latter exist globally and can be used for any project.
> 
> 
> Diffs
> -----
> 
>   languages/plugins/custom-definesandincludes/CMakeLists.txt 71bc32e 
>   languages/plugins/custom-definesandincludes/compilerprovider/CMakeLists.txt 45cccbc 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.h PRE-CREATION 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.cpp PRE-CREATION 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h cbcab2f 
>   languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp 6483995 
>   languages/plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.h b96df3b 
>   languages/plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp 13e9736 
>   languages/plugins/custom-definesandincludes/compilerprovider/icompiler.h 77ceb75 
>   languages/plugins/custom-definesandincludes/compilerprovider/icompiler.cpp PRE-CREATION 
>   languages/plugins/custom-definesandincludes/compilerprovider/icompilerfactory.h PRE-CREATION 
>   languages/plugins/custom-definesandincludes/compilerprovider/icompilerprovider.h 21bfc05 
>   languages/plugins/custom-definesandincludes/compilerprovider/msvccompiler.h e0ee668 
>   languages/plugins/custom-definesandincludes/compilerprovider/msvccompiler.cpp 1c6b6c5 
>   languages/plugins/custom-definesandincludes/kcm_widget/CMakeLists.txt ea0fe01 
>   languages/plugins/custom-definesandincludes/kcm_widget/compilersmodel.h PRE-CREATION 
>   languages/plugins/custom-definesandincludes/kcm_widget/compilersmodel.cpp PRE-CREATION 
>   languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.h PRE-CREATION 
>   languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.cpp PRE-CREATION 
>   languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.ui PRE-CREATION 
>   languages/plugins/custom-definesandincludes/kcm_widget/customdefinesandincludes.kcfg 9acb2e0 
>   languages/plugins/custom-definesandincludes/kcm_widget/kcm_customdefinesandincludes.cpp c09b00e 
>   languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.h 3618170 
>   languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp c3a0823 
>   languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.ui 9831b94 
>   languages/plugins/custom-definesandincludes/settingsmanager.h f215268 
>   languages/plugins/custom-definesandincludes/settingsmanager.cpp fb7fd0f 
>   projectmanagers/custom-buildsystem/tests/projects/builddirproject/.kdev4/builddirproject.kdev4 176ebae 
>   projectmanagers/custom-buildsystem/tests/projects/multipathproject/.kdev4/multipathproject.kdev4 c968968 
>   projectmanagers/custom-buildsystem/tests/projects/simpleproject/.kdev4/simpleproject.kdev4 b5fb394 
> 
> Diff: https://git.reviewboard.kde.org/r/118718/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> addCompiler.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/06/13/613c5d8c-6425-42ad-b0e4-1ccc2384a838__addCompiler.png
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140622/d2bd9774/attachment-0001.html>