D15532: [Astyle] Add Objective C to list of languages with formatters

[Friedrich W. H. Kossebau]

kossebau accepted this revision.
kossebau added a comment.


  In D15532#328544 <https://phabricator.kde.org/D15532#328544>, @kossebau wrote:
  
  > In D15532#328524 <https://phabricator.kde.org/D15532#328524>, @kossebau wrote:
  >
  > > 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.
  >
  >
  > Nope, this is actually some bug in KDevelop which should be fixed first,  somewhere in`SourceFormatterStyle::modeForMimetype(...)`  the wrong mode is chosen. Still digging.
  
  
  Seems that this needs some fixing in SourceFormatterStyle::modeForMimetype(), which is too eager picking any first style where `mime.inherits(item.mimeType)`, which is then the "C" style given it comes first and shared-mime-info has this questionable inheritage for C++ and Objective-C from C. So nothing this patch here is guily for (now that it also uses ids matching the KSyntaxHighlighting mode ids).
  
  In D15532#328664 <https://phabricator.kde.org/D15532#328664>, @kfunk wrote:
  
  > Please update the Diff according to our requests, then I think this can still go into 5.3 branch. Friedrich, you still agree this could go into 5.3? :)
  
  
  Not tested again the latest patch, but from code reading looks good. Can be considered a bug fix, so 5.3 okay with me. Also no string freeze break. So let's have it in 5.3, making more potential users happy earlier :)

INLINE COMMENTS

> rjvbb wrote in astyle_plugin.cpp:36
> And there I was thinking the days were gone where foo[] and *foo were ever so subtly different... :)

For what I know, the days are still on-going due to this:

  static const char * foo = "foo";

is a non-const pointer to a const char array. You can do (accidentally)

  foo = "bar";

and the compiler will not see this runs against the (usual) intent.

It is also one indirection more (unless the compiler is sure enough to be able to optimize this out), as `foo` is a symbol for a data that is a pointer, which holds the address of the char array.
Where with

  static const char foo[] = "foo";

foo is a symbol for a data that is the char array.

> rjvbb wrote in astyle_plugin.cpp:284
> Thanks, that's a nice reminder there a few more places where changes are required and for which I have D15530 <https://phabricator.kde.org/D15530> up!

And as I meanwhile found, using "Objective-C" and "Objective-C++" here is also required, as this is by chance also the id used by the syntax highlighting mode for the respective highlighting rules.

> rjvbb wrote in astyle_plugin.cpp:327
> I take it you meant fixing the whitespace for the entire method(?)

Actually I only meant the lines you are touching, sorry for being unclear.

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, kossebau, kfunk
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/20180920/e09ac640/attachment-0001.html>