[Minor visual bug] Syntax and comments

[Oblivion]

Hello.

- Code::Blocks information
Release: 12.11 rev 8629
SDK Version: 1.13.14
Build: Nov 25, 2012, 09:32:38 - wx2.8.12 (Windows, UNICODE) - 32 bit

- The bug information / description
When you use a C++ comment, and have a left parenthesis followed directly by an asterisk: All the code below that line is shown as being 'commented out' (from a C-style comment).

I found this out moments ago while working on a project; Here is a screen shot explaining what I mean:


This is strictly a visual bug. It has no effect on the compilation or anything else of the sort that I am aware of.
I did not bother to search around to see if others have already reported this bug or not; I figure it's harmless by nature.

The offensive code:

Code

//(*itHit).move(TOP);

The same offensive code, without the bug being reproduced (a single space):

Code

// (*itHit).move(TOP);

That is all. 

[Jenna]

This is not a bug, it's a feature  .

And yes, I mean what I said.

Code

//(*

[/b] and //*) are used as start and end-tag of the wxSmith-created code.
The code between the tags can (and will) be changed (or resetted) in projects, that use wxSmith every time anything is changed by the wxSmith-plugin.

You can switch this behaviour off by unchecking "Settings -> Editor -> General settings -> C/C++ Editor settings -> Highlight wxSmith sections differently".

wxSmith is the wxWidgets rad-development plugin distributed with the Code::Blocks contrib-plugins.

[oBFusCATed]

Jens:
It is a bug in fact and I'm annoyed by this problem, too.
Can we make it a bit more strict that if the //(* is followed by any other characters it won't trigger the change in style?

[Jenna]

It's not that easy, because wxSmith also adds text directly behind the asterisk, e.g. //(*AppInitialize .

[Jenna]

Jens:
It is a bug in fact and I'm annoyed by this problem, too.

Why is it a bug ?
The only misbehaviour might be (I have to test it), is that this opetion is turned on as default.
I think it's very useful to see which parts of the code are safe to change and which parts might be automatically changed by wxSmith.
And wxSmith has to use start and end tags to find it's code.

I did not see very much reports about this issue (if any).

Anyway it should be tested if such rare comments can conflict with wxSmith and (most worth) whether it can happen that wxSmith overrides code that is not owned by it.

[oBFusCATed]

Why is it a bug ?

Because it triggers for non-wxsmith comments. And when it triggers for non-wxsmith comment, most of the times there is no //*)
(or whatever is the proper closing comment), so the whole file ends up being marked as one big wxsmith block.

[Alpha]

For reference purposes, here are the comments that list out the names of all blocks (to my knowledge) that wxSmith generates.  If we want to make the wxSmith code highlighting more restrictive, we could explicitly enumerate the allowed block names.
src/plugins/contrib/wxSmith/wxscoder.h 163

Code

/** \page Auto-Code Code automatically generated by wxSmith
 *
 * Here's list of automatically generated code:
 *
 * \li \c //(*EventTable($CLASSNAME) - generated in class source file, contains
 *                                  entries for event table
 * \li \c //(*Initialize($CLASSNAME) - generated in class source file, this code
 *                                  does all resource initialization (loading
 *                                  XRC, adding widgets etc.)
 * \li \c //(*Headers($CLASSNAME) - generated in class header file, this block
 *                                  contains set of #includes including required
 *                                  header files for this resource
 * \li \c //(*Identifiers($CLASSNAME) - generated in class header file, this
 *                                  code generates identifiers for window
 *                                  items (usually enum), in case of XRC
 *                                  resource (with it's own identifier
 *                                  handling system), it will be empty
 * \li \c //(*Handlers($CLASSNAME) - generated in class header. It contains
 *                                  declarations of event handler functions,
 *                                  inside this code, user may put it's own
 *                                  event handler declarations but they must
 *                                  be in form:
 *                                     void HandlerName(eventType& event).
 *                                  This block is parsed when generating list
 *                                  of event handlers which may be used with
 *                                  given event type.
 * \li \c //(*Declarations($CLASSNAME) - declarations of window items. This
 *                                  block will contain declarations of all
 *                                  window items which have "Is Member"
 *                                  property set to true.
 *
 * Blocks which will be added in future:
 *
 * \li \c //(*AppHeaders - declared in main application's source file.
 *                                 This block will contain set of #includes
 *                                 required by application.
 * \li \c //(*AppInitialize - declared in main application's source file.
 *                                 This block will automatically load resources
 *                                 and show main application's window
 */

src/plugins/contrib/wxSmith/wxsversionconverter.cpp 198

Code

    // Need to add two new sections: //(*InternalHeaders and //(*IdInit
    // to do this //(*InternalHeaders will be added before any source code
    // but after all  #xxx directives
    // //(*IdInit will be added just before //(*EventTable nearrest line
    // before that section containing BEGIN_EVENT_TABLE
    //
    // This may not be tricky enough but I hope that not much people mess
    // with code generated by wxSmith from templates ;)
    //
    // BTW we do not use wxsCodeMarks::Beg and wxsCodeMarks::End because that could
    // cause some problems with future conversion of these marks
    // (upgrade of old-wxSmith project will be done in two steps then,
    // first - convertion to version 1 of new wxsmith and then upgrading to
    // higher version so if convention of code marks will change, it will
    // break the conversion chain)

[Alpha]

For example:

Code

Index: src/sdk/wxscintilla/src/scintilla/lexers/LexCPP.cxx
===================================================================
--- src/sdk/wxscintilla/src/scintilla/lexers/LexCPP.cxx    (revision 8909)
+++ src/sdk/wxscintilla/src/scintilla/lexers/LexCPP.cxx    (working copy)
@@ -854,12 +854,33 @@
 /* C::B begin */
                 else if (sc.Match("//(*") && options.highlightWxSmith) {
                     // Support for wxSmith auto-generated code
-                    sc.SetState(SCE_C_WXSMITH|activitySet);
-                    sc.Forward(4);
-                    sc.SetState(SCE_C_COMMENTLINE|activitySet);
-                    while (!sc.atLineEnd)
-                        sc.Forward();
-                    sc.SetState(SCE_C_WXSMITH|activitySet);
+                    char* blockIds[] = {"//(*AppHeaders",
+                                        "//(*AppInitialize",
+                                        "//(*Declarations",
+                                        "//(*Destroy",
+                                        "//(*EventTable",
+                                        "//(*Handlers",
+                                        "//(*Headers",
+                                        "//(*IdInit",
+                                        "//(*Identifiers",
+                                        "//(*Initialize",
+                                        "//(*InternalHeaders",
+                                        0};
+                    int i = 0;
+                    for (; blockIds[i]; ++i) {
+                        if (sc.Match(blockIds[i]))
+                            break;
+                    }
+                    if (blockIds[i]) {
+                        sc.SetState(SCE_C_WXSMITH|activitySet);
+                        sc.Forward(4);
+                        sc.SetState(SCE_C_COMMENTLINE|activitySet);
+                        while (!sc.atLineEnd)
+                            sc.Forward();
+                        sc.SetState(SCE_C_WXSMITH|activitySet);
+                    }
+                    else
+                        sc.SetState(SCE_C_COMMENTLINE|activitySet);
                 }
 /* C::B end */
                 else

(Disclaimer: this code was quickly written, and is suboptimal.)

[MortenMacFly]

we want to make the wxSmith code highlighting more restrictive,

Thats an option. But honestly I think most user will be happy to know that another space will "fix" the highlighting issue. I think the proposed solution with having the option disabled by default might be the easiest thing to do and we will never forget to change the wxScintilla portion if we change wxSmith.

Although I personally would do exactly nothing about it.

[oBFusCATed]

I think most user will be happy to know that another space will "fix" the highlighting issue.

No they won't, at least the ones using it for work projects and where the source is stored in a VCS.
Committing such thing will look ridiculous

[Alpha]

Attached is the way I would recommend approaching this problem.  The list of wxSmith identifiers is stored in a keyword set, so it can be modified without recompilation, or even by the end user, while in use.

[Alpha]

Any reason against this?