Review Request 118194: Make it possible to select compiler at run-tume to deduce standard includes/defines

Fri May 23 12:27:05 UTC 2014


> On May 21, 2014, 12:07 p.m., Milian Wolff wrote:
> > 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.

>Missing the ICompilerProvider specification
Nope, It was here all the time, seems like you read too fast and didn't notice it...

>Then in the GUI just query the plugins implementing this interface to fill the combobox.
Why bother if KConfig framework does it automatically for us?

>To remember what compiler the user selected, use the plugin ID (not the pretty localized name).
What is the plugin ID? Also compiler names not localized and shouldn't be, so I'm not sure if it makes sense at all...

>Also ensure to cache this data.
Yeap, already done that, see e.g. GccLikeProvider::includes. On the first line it checks whether data already parsed, if so it returns it without quering the compiler.


> On May 21, 2014, 12:07 p.m., Milian Wolff wrote:
> > languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp, line 126
> > <https://git.reviewboard.kde.org/r/118194/diff/1/?file=273406#file273406line126>
> >
> >     return QString() and in the UI replace it with a proper localized error message.

>in the UI replace it with a proper localized error message.
After some thinking I've decided not to show an error message, IMO issuing a warning is more than enough.


- Sergey


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


On May 18, 2014, 9: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, 9: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/20140523/7c0c4804/attachment.html>