Review Request 115590: Allow language plugins to provide custom formatting configuration items

[Milian Wolff]

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

Ship it!


some style nitpicks, otherwise looks good - I assume it works as well and you tested it :)

thanks for this sven!


interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34836>

    mark as movable



interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34835>

    QVector



interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34838>

    don't inline this, its an exported class (yes I know - its done so already but no need to follow that bad practice for new code...)



interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34837>

    bad indentation (using tabs, right? use four spaces instead please)



interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34839>

    dont inline



interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34840>

    dont inline



interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34841>

    const& and dont inline



interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34843>

    dont inline



interfaces/isourceformatter.h
<https://git.reviewboard.kde.org/r/115590/#comment34842>

    indentation



language/interfaces/ilanguagesupport.h
<https://git.reviewboard.kde.org/r/115590/#comment34845>

    newline after /**, whitespace after *
    
    yes - the code above is bad - ignore it



language/interfaces/ilanguagesupport.h
<https://git.reviewboard.kde.org/r/115590/#comment34846>

    make the method const?



shell/settings/sourceformattersettings.cpp
<https://git.reviewboard.kde.org/r/115590/#comment34854>

    join line



shell/settings/sourceformattersettings.cpp
<https://git.reviewboard.kde.org/r/115590/#comment34855>

    join line



shell/sourceformattercontroller.h
<https://git.reviewboard.kde.org/r/115590/#comment34847>

    tabs again?



shell/sourceformattercontroller.cpp
<https://git.reviewboard.kde.org/r/115590/#comment34850>

    static, newline before {



shell/sourceformattercontroller.cpp
<https://git.reviewboard.kde.org/r/115590/#comment34851>

    use QString() instead of "" please



shell/sourceformattercontroller.cpp
<https://git.reviewboard.kde.org/r/115590/#comment34852>

    join line



shell/sourceformattercontroller.cpp
<https://git.reviewboard.kde.org/r/115590/#comment34853>

    join line


- Milian Wolff


On Feb. 9, 2014, 12:14 a.m., Sven Brauch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115590/
> -----------------------------------------------------------
> 
> (Updated Feb. 9, 2014, 12:14 a.m.)
> 
> 
> Review request for KDevelop and Ashwin Rajeev.
> 
> 
> Repository: kdevplatform
> 
> 
> Description
> -------
> 
> This was more difficult than anticipated, but here it comes ;)
> 
> See also discussion here: https://git.reviewboard.kde.org/r/115356/
> 
> The basic is to shift the philosophy of source formatters a bit: instead of providing integration with one fixed tool with many slightly different configurations, a Source Formatter plugin can provide an engine for source formatters which can do very different things depending on the configuration. For many languages (I obviously have python in mind) tools already exist which re-format source code and just need to be called. Writing a new formatter plugin for each of those would basically duplicate the existing functionality (especially from the customscript formatter) for each language.
> 
> Thus, this patch provides a new virtual function in ILanguageSupport which allows language plugins to return any amount of formatting configurations for any source formatter plugin (the formatter plugin choses which of the items it accepts based on a name).
> 
> To make this useful, the mime type and language, and -- optionally -- a text sample different from the default are now stored inside the formatting configuration items. The list of displayed configurations is filtered by language. Only formatting engines which support at least one configuration for a selected language are shown (as before), but this is now "dynamic" and depends on the loaded language plugins (it's no longer hardcoded in the .desktop file).
> 
> I didn't yet evaluate creating custom such items, that remains a TODO for the future (it also requires a bit of thinking UI-wise). One step at a time ;)
> 
> 
> Diffs
> -----
> 
>   interfaces/isourceformatter.h f0ab04c2452d4b9a47311335564c8e932435cde8 
>   language/interfaces/ilanguagesupport.h 74892d5aa95e1dc18aa72a5ec278f98fb8a415f6 
>   language/interfaces/ilanguagesupport.cpp 1d50d9495d73603871983b827a0aa29ac0bfde8a 
>   shell/settings/editstyledialog.cpp 3b019bb4db2f5bdc8ee94fb0f39aec100e26681b 
>   shell/settings/sourceformattersettings.h 4cb8c48703649201f22e577791a965f2dacade70 
>   shell/settings/sourceformattersettings.cpp fda6cde29f55788e7814fd098c58bcf365136f71 
>   shell/sourceformattercontroller.h ea6ef555e5965098b3816ad5ff1b817d65480fe6 
>   shell/sourceformattercontroller.cpp 7b063055b0acfc170eb2025a4253af48154f59ed 
> 
> Diff: https://git.reviewboard.kde.org/r/115590/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> custom script formatter + python
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/02/08/2a1a3c51-9ef7-4de4-be24-15ac4d2532c7__python.png
> custom script formatter with cpp -- works as before
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/02/08/bb2b4080-c712-4cc6-ab36-a0bbbeb1192c__cpp.png
> 
> 
> Thanks,
> 
> Sven Brauch
> 
>

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