D15532: kdev-astyle : improved ObjC support

Friedrich W. H. Kossebau noreply at phabricator.kde.org
Wed Sep 19 21:02:02 BST 2018

kossebau accepted this revision.
kossebau added a subscriber: kfunk.
kossebau added a comment.

  Code looks fine to me, modulo all the comments made, and works where I tested it. Please also update the summary text and title of the patch.
  For some reason the preview uses the "C" highlighting mode for me when ObjC is selected, so the preview has some highlighting issues. That seems a bug with KTextEditor I have to explore more.
  Not sure if this is 5.3 or master material. No new string inside, so I would be tempted to have this still with 5.3, given we are still before final release and it's a small innocent addition at the outer spheres, so should not introduce regressions. @kfunk Your take as maintainer?


> astyle_plugin.cpp:327
>          lang = AStylePreferences::CSharp;
> +    else if(mime.inherits(QStringLiteral("text/x-objcsrc")) || mime.inherits(QStringLiteral("text/x-objc++src"))) {
> +        // x-objc++src *should* inherit x-objcsrc but that is not always the case in practice

While adding this line, already add a space between if and (.

> astyle_plugin.cpp:344
> +    // TODO: add previews for the other supported languages
> +    preview = indentingSample(lang);
> +    preview += QLatin1String("\t// Formatting\n");

Please keep this one single string calculation:

      QLatin1String("// Indentation\n") +
      indentingSample(lang) +
      QLatin1String("\t// Formatting\n") +

> astyle_plugin.h:28
> +#include "astyle_preferences.h"
> +

Make first include, so plugin-specific includes come first

> astyle_preferences.h:62
>      bool m_enableWidgetSignals;
> +    Language m_currentLanguage;
>  };

`const  Language m_currentLanguage;`, to point out this is not going to change.

  R32 KDevelop


To: rjvbb, #kdevelop, kossebau
Cc: kfunk, pino, kossebau, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180919/82213f20/attachment-0001.html>

More information about the KDevelop-devel mailing list