Review Request 118194: Make it possible to select compiler at run-tume to deduce standard includes/defines
Milian Wolff
mail at milianw.de
Wed May 21 08:07:14 UTC 2014
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118194/#review58218
-----------------------------------------------------------
Missing the ICompilerProvider specification, I find it hard to review.
This now introduces:
- KDevelop::ICompilerProvider
- BaseProvider
- CompilerProvider
- IADCompilerProvider
- the gcc, clang and msvc providers
This is far too complicated. Also the whole code comparing strings of compiler names like "gcc", "clang", "msvc" is very error-prone and hard to extend.
I suggest instead to make the gcc, clang and msvc plugins implementing the new ICompilerProvider interface. Add a "virtual QString name() const = 0" to the interface (which should return a localized human readable name). Then in the GUI just query the plugins implementing this interface to fill the combobox. Oh and also add a "virtual QString defaultPath() const = 0" and show that in the UI as well to fill the path-lineedit by default. To remember what compiler the user selected, use the plugin ID (not the pretty localized name).
Then the API to actually query includes and defines should take the actual user-selected path i.e.:
virtual QStringList includes(const QString &path) const = 0
virtual QHash<QString, QString> defines(const QString &path) const = 0
This should then be queried from the IADM. Also ensure to cache this data. We don't want to query gcc/clang/msvc for data every time we parse a file.
languages/cpp/tests/test_cppfiles.cpp
<https://git.reviewboard.kde.org/r/118194/#comment40479>
yeah odd, but this change is fine. I'll have to worry about it :)
languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h
<https://git.reviewboard.kde.org/r/118194/#comment40484>
this file is missing, also probably must be installed in the CMakeLists.txt, no?
languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h
<https://git.reviewboard.kde.org/r/118194/#comment40481>
move this to a separate file please
languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h
<https://git.reviewboard.kde.org/r/118194/#comment40475>
pure virtual and constify
languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h
<https://git.reviewboard.kde.org/r/118194/#comment40474>
you don't need to pimpl this class, its not exported after all.
languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
<https://git.reviewboard.kde.org/r/118194/#comment40482>
const
languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp
<https://git.reviewboard.kde.org/r/118194/#comment40483>
return QString() and in the UI replace it with a proper localized error message.
languages/plugins/custom-definesandincludes/definesandincludesmanager.h
<https://git.reviewboard.kde.org/r/118194/#comment40478>
this and ::includes() below: make const and also mark as override, no?
languages/plugins/custom-definesandincludes/settingsmanager.h
<https://git.reviewboard.kde.org/r/118194/#comment40476>
const
languages/plugins/custom-definesandincludes/settingsmanager.h
<https://git.reviewboard.kde.org/r/118194/#comment40477>
const
languages/plugins/custom-definesandincludes/settingsmanager.cpp
<https://git.reviewboard.kde.org/r/118194/#comment40480>
remove this line, or move it one line up:
{
auto grp = ...
grp.writeEntry(...);
grp.writeEntry(...);
}
- Milian Wolff
On May 18, 2014, 5:34 p.m., Sergey Kalinichev wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118194/
> -----------------------------------------------------------
>
> (Updated May 18, 2014, 5:34 p.m.)
>
>
> Review request for KDevelop.
>
>
> Repository: kdevelop
>
>
> Description
> -------
>
> To make it all work I had to turn IADM into global plugin, that way it can be used to retrieve includes/defines for files without project.
> Also I made SettingsManager a shared library to ease access to it from different places.
>
>
> Diffs
> -----
>
> languages/cpp/setuphelpers.cpp 57505fc
> languages/cpp/setuphelpers.h 012dcf4
> languages/cpp/includepathcomputer.cpp 822b0bc
> languages/cpp/msvcdefinehelper.cpp 9b131ed
> languages/cpp/cpputils.cpp 380dd4e
> languages/cpp/cpputils.h c951bd3
> languages/cpp/CMakeLists.txt 377f35e
> languages/cpp/cpplanguagesupport.cpp 0f3bd4a
> languages/cpp/setuphelpers_gcc_like.cpp b261589
> languages/cpp/setuphelpers_msvc.cpp 5576c01
> languages/cpp/tests/CMakeLists.txt d6eb67a
> languages/cpp/tests/test_cppfiles.cpp 76486d0
> languages/plugins/custom-definesandincludes/CMakeLists.txt be7d6e8
> languages/plugins/custom-definesandincludes/compilerprovider/CMakeLists.txt PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/gcclikeprovider.h PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/gcclikeprovider.cpp PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/icompilerprovider.h PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/kdevcompilerprovider.desktop.cmake PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/msvcdefinehelper.cpp PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/msvcprovider.h PRE-CREATION
> languages/plugins/custom-definesandincludes/compilerprovider/msvcprovider.cpp PRE-CREATION
> languages/plugins/custom-definesandincludes/definesandincludesmanager.h 12a7763
> languages/plugins/custom-definesandincludes/definesandincludesmanager.cpp bdce956
> languages/plugins/custom-definesandincludes/kcm_widget/CMakeLists.txt 0d8c830
> languages/plugins/custom-definesandincludes/kcm_widget/customdefinesandincludes.kcfg 3b49940
> languages/plugins/custom-definesandincludes/kcm_widget/customdefinesandincludes.kcfgc 720af41
> languages/plugins/custom-definesandincludes/kcm_widget/kcm_customdefinesandincludes.cpp 75f5063
> languages/plugins/custom-definesandincludes/kcm_widget/kcm_kdevcustomdefinesandincludes.desktop ef63c57
> languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp 1456217
> languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.ui 4e512a9
> languages/plugins/custom-definesandincludes/kdevdefinesandincludesmanager.desktop.cmake ad08867
> languages/plugins/custom-definesandincludes/settingsmanager.h 9ae23d6
> languages/plugins/custom-definesandincludes/settingsmanager.cpp dc7aa3e
> languages/plugins/custom-definesandincludes/tests/plugintest.cpp 1cfa2e4
> projectmanagers/custom-buildsystem/tests/projects/builddirproject/.kdev4/builddirproject.kdev4 33c8568
> projectmanagers/custom-buildsystem/tests/projects/multipathproject/.kdev4/multipathproject.kdev4 fd14aca
> projectmanagers/custom-buildsystem/tests/projects/simpleproject/.kdev4/simpleproject.kdev4 2c4bac5
>
> Diff: https://git.reviewboard.kde.org/r/118194/diff/
>
>
> Testing
> -------
>
> All tests pass (except cppspecialcompletion, which doesn't work for me anyway).
> Also I've tested it with 3 opened projects in one session, works fine for me.
>
>
> File Attachments
> ----------------
>
> kdevplatform_changes
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/18/f8aee22e-deac-41f2-999d-7c321a419147__kdevplatform.diff
> selectCompiler_image.png
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/18/e7a99678-843d-4dd1-a5f2-297662956134__selectCompiler_image.png
>
>
> Thanks,
>
> Sergey Kalinichev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140521/f7d00687/attachment-0001.html>
More information about the KDevelop-devel
mailing list