D15532: kdev-astyle : improved ObjC support

[Friedrich W. H. Kossebau]

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?

INLINE COMMENTS

> 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:

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

> 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.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D15532

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>