Problems building Scintilla from source

[Mike Lischke]

Hey Neil,

we are currently upgrading to Scintilla 3.7.0 and got a few problems when compiling on Linux. This seems mostly related to the C++11 regex support which fails to compile. There is this switch NO_CXX11_REGEX in the make file. What is this for? Does it require additional prerequisites other than C++11? Have you seen/heard about problems compiling that part with clang? Note: it compiles fine with clang on OSX using the provided xcode project.

Thanks,

Mike
--
www.soft-gems.net

[Neil Hodgson]

Mike Lischke:

> we are currently upgrading to Scintilla 3.7.0 and got a few problems when compiling on Linux. This seems mostly related to the C++11 regex support which fails to compile.

See the commits from the last week. It appears that recent Linux distributions configure Clang to use the GNU standard C++ library instead of Clang’s libc++. This requires that the iterator constructors be marked ‘noexcept’. Of course, ‘noexcept’ isn’t available on all compilers so it can’t be added indiscriminately.

> There is this switch NO_CXX11_REGEX in the make file. What is this for?

It turns off the C++11 regex feature. It should make the file compile for you.

Neil

[Mike Lischke]

>> we are currently upgrading to Scintilla 3.7.0 and got a few problems when compiling on Linux. This seems mostly related to the C++11 regex support which fails to compile.
>
> See the commits from the last week. It appears that recent Linux distributions configure Clang to use the GNU standard C++ library instead of Clang’s libc++. This requires that the iterator constructors be marked ‘noexcept’. Of course, ‘noexcept’ isn’t available on all compilers so it can’t be added indiscriminately.

Ah, I see. Thanks.

Btw, could you please consider adding the usual

#ifdef SCI_NAMESPACE
namespace Scintilla {
...

wrapper to ScintillaWidget.h?

Thanks,

Mike
--
www.soft-gems.net

[Neil Hodgson]

Mike Lischke:

> Btw, could you please consider adding the usual
>
> #ifdef SCI_NAMESPACE
> namespace Scintilla {
> ...
>
> wrapper to ScintillaWidget.h?

What effect would that have? The whole file is inside an extern “C” block.

Neil

[Mike Lischke]

Well, it's actually about SCNotification which is in the Scintilla namespace if enabled and this file won't compile then. Currently we manually change this to Scintilla::SCNotification, but that's not how it should be.

Mike
--
www.soft-gems.net

[Neil Hodgson]

Mike Lischke:

> Well, it's actually about SCNotification which is in the Scintilla namespace if enabled and this file won't compile then. Currently we manually change this to Scintilla::SCNotification, but that's not how it should be.

Adding the ‘ namespace Scintilla { ‘ to the header leads to ambiguity errors.

From both Clang and g++:

clang++ -Wno-deprecated-register -pthread -I/usr/include/gtk-3.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/gtk-3.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/mirclient -I/usr/include/mircommon -I/usr/include/mircookie -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libpng16 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include --std=gnu++0x -DNDEBUG -Os -Wall -pedantic -fPIC -DGTK -DSCI_LEXER -DSCI_NAMESPACE -I ./../include -I ./../src -I ./../lexlib -c ScintillaGTK.cxx
ScintillaGTK.cxx:2921:9: error: call to 'scintilla_send_message' is ambiguous
return scintilla_send_message(sci, iMessage, wParam, lParam);
^~~~~~~~~~~~~~~~~~~~~~
./../include/ScintillaWidget.h:64:9: note: candidate function
sptr_t scintilla_send_message (ScintillaObject *sci,unsigned int iMessage, uptr_t wParam, sptr_t lParam);
^
ScintillaGTK.cxx:2915:8: note: candidate function
sptr_t scintilla_send_message(ScintillaObject *sci, unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
^
ScintillaGTK.cxx:2961:9: error: call to 'scintilla_get_type' is ambiguous
return scintilla_get_type();
^~~~~~~~~~~~~~~~~~
./../include/ScintillaWidget.h:61:8: note: candidate function
GType scintilla_get_type (void);
^
ScintillaGTK.cxx:2931:7: note: candidate function
GType scintilla_get_type() {
^
ScintillaGTK.cxx:3069:46: error: call to 'scintilla_get_type' is ambiguous
GtkWidget *widget = GTK_WIDGET(g_object_new(scintilla_get_type(), NULL));
^~~~~~~~~~~~~~~~~~
/usr/include/gtk-3.0/gtk/gtkwidget.h:58:61: note: expanded from macro 'GTK_WIDGET'
#define GTK_WIDGET(widget) (G_TYPE_CHECK_INSTANCE_CAST ((widget), GTK_TYPE_WIDGET, GtkWidget))
^~~~~~
/usr/include/glib-2.0/gobject/gtype.h:479:80: note: expanded from macro 'G_TYPE_CHECK_INSTANCE_CAST'
#define G_TYPE_CHECK_INSTANCE_CAST(instance, g_type, c_type) (_G_TYPE_CIC ((instance), (g_type), c_type))
^~~~~~~~
/usr/include/glib-2.0/gobject/gtype.h:2207:57: note: expanded from macro '_G_TYPE_CIC'
((ct*) g_type_check_instance_cast ((GTypeInstance*) ip, gt))
^~
./../include/ScintillaWidget.h:61:8: note: candidate function
GType scintilla_get_type (void);
^
ScintillaGTK.cxx:2931:7: note: candidate function
GType scintilla_get_type() {
^
ScintillaGTK.cxx:3076:9: error: call to 'scintilla_new' is ambiguous
return scintilla_new();
^~~~~~~~~~~~~
./../include/ScintillaWidget.h:62:12: note: candidate function
GtkWidget* scintilla_new (void);
^
ScintillaGTK.cxx:3068:12: note: candidate function
GtkWidget* scintilla_new() {
^
4 errors generated.
makefile:87: recipe for target 'ScintillaGTK.o' failed

g++ -pthread -I/usr/include/gtk-3.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/gtk-3.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/mirclient -I/usr/include/mircommon -I/usr/include/mircookie -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libpng16 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include --std=gnu++0x -DNDEBUG -Os -Wall -pedantic -fPIC -DGTK -DSCI_LEXER -DSCI_NAMESPACE -I ./../include -I ./../src -I ./../lexlib -c ScintillaGTK.cxx
ScintillaGTK.cxx: In function 'gintptr scintilla_object_send_message(Scintilla::ScintillaObject*, unsigned int, uptr_t, sptr_t)':
ScintillaGTK.cxx:2921:61: error: call of overloaded 'scintilla_send_message(Scintilla::ScintillaObject*&, unsigned int&, uptr_t&, sptr_t&)' is ambiguous
return scintilla_send_message(sci, iMessage, wParam, lParam);
^
ScintillaGTK.cxx:2915:8: note: candidate: sptr_t scintilla_send_message(Scintilla::ScintillaObject*, unsigned int, uptr_t, sptr_t)
sptr_t scintilla_send_message(ScintillaObject *sci, unsigned int iMessage, uptr_t wParam, sptr_t lParam) {
^~~~~~~~~~~~~~~~~~~~~~
In file included from ScintillaGTK.cxx:34:0:
./../include/ScintillaWidget.h:64:9: note: candidate: sptr_t Scintilla::scintilla_send_message(Scintilla::ScintillaObject*, unsigned int, uptr_t, sptr_t)
sptr_t scintilla_send_message (ScintillaObject *sci,unsigned int iMessage, uptr_t wParam, sptr_t lParam);
^~~~~~~~~~~~~~~~~~~~~~
ScintillaGTK.cxx: In function 'GType scintilla_object_get_type()':
ScintillaGTK.cxx:2961:28: error: call of overloaded 'scintilla_get_type()' is ambiguous
return scintilla_get_type();
^
ScintillaGTK.cxx:2931:7: note: candidate: GType scintilla_get_type()
GType scintilla_get_type() {
^~~~~~~~~~~~~~~~~~
In file included from ScintillaGTK.cxx:34:0:
./../include/ScintillaWidget.h:61:8: note: candidate: GType Scintilla::scintilla_get_type()
GType scintilla_get_type (void);
^~~~~~~~~~~~~~~~~~
In file included from /usr/include/glib-2.0/gobject/gobject.h:24:0,
from /usr/include/glib-2.0/gobject/gbinding.h:29,
from /usr/include/glib-2.0/glib-object.h:23,
from /usr/include/glib-2.0/gio/gioenums.h:28,
from /usr/include/glib-2.0/gio/giotypes.h:28,
from /usr/include/glib-2.0/gio/gio.h:26,
from /usr/include/gtk-3.0/gdk/gdkapplaunchcontext.h:28,
from /usr/include/gtk-3.0/gdk/gdk.h:32,
from ScintillaGTK.cxx:22:
ScintillaGTK.cxx: In function 'GtkWidget* scintilla_new()':
ScintillaGTK.cxx:3069:65: error: call of overloaded 'scintilla_get_type()' is ambiguous
GtkWidget *widget = GTK_WIDGET(g_object_new(scintilla_get_type(), NULL));
^
/usr/include/glib-2.0/gobject/gtype.h:2207:57: note: in definition of macro '_G_TYPE_CIC'
((ct*) g_type_check_instance_cast ((GTypeInstance*) ip, gt))
^~
/usr/include/gtk-3.0/gtk/gtkwidget.h:58:32: note: in expansion of macro 'G_TYPE_CHECK_INSTANCE_CAST'
#define GTK_WIDGET(widget) (G_TYPE_CHECK_INSTANCE_CAST ((widget), GTK_TYPE_WIDGET, GtkWidget))
^~~~~~~~~~~~~~~~~~~~~~~~~~
ScintillaGTK.cxx:3069:22: note: in expansion of macro 'GTK_WIDGET'
GtkWidget *widget = GTK_WIDGET(g_object_new(scintilla_get_type(), NULL));
^~~~~~~~~~
ScintillaGTK.cxx:2931:7: note: candidate: GType scintilla_get_type()
GType scintilla_get_type() {
^~~~~~~~~~~~~~~~~~
In file included from ScintillaGTK.cxx:34:0:
./../include/ScintillaWidget.h:61:8: note: candidate: GType Scintilla::scintilla_get_type()
GType scintilla_get_type (void);
^~~~~~~~~~~~~~~~~~
ScintillaGTK.cxx: In function 'GtkWidget* scintilla_object_new()':
ScintillaGTK.cxx:3076:23: error: call of overloaded 'scintilla_new()' is ambiguous
return scintilla_new();
^
ScintillaGTK.cxx:3068:12: note: candidate: GtkWidget* scintilla_new()
GtkWidget* scintilla_new() {
^~~~~~~~~~~~~
In file included from ScintillaGTK.cxx:34:0:
./../include/ScintillaWidget.h:62:12: note: candidate: GtkWidget* Scintilla::scintilla_new()
GtkWidget* scintilla_new (void);
^~~~~~~~~~~~~
makefile:87: recipe for target 'ScintillaGTK.o' failed
make: *** [ScintillaGTK.o] Error 1

Neil

[Mike Lischke]

>
>> Well, it's actually about SCNotification which is in the Scintilla namespace if enabled and this file won't compile then. Currently we manually change this to Scintilla::SCNotification, but that's not how it should be.
>
> Adding the ‘ namespace Scintilla { ‘ to the header leads to ambiguity errors.
>
> From both Clang and g++:
>
> clang++ -Wno-deprecated-register -pthread -I/usr/include/gtk-3.0 -I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 -I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/usr/include/gtk-3.0 -I/usr/include/gio-unix-2.0/ -I/usr/include/mirclient -I/usr/include/mircommon -I/usr/include/mircookie -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng16 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libpng16 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include --std=gnu++0x -DNDEBUG -Os -Wall -pedantic -fPIC -DGTK -DSCI_LEXER -DSCI_NAMESPACE -I ./../include -I ./../src -I ./../lexlib -c ScintillaGTK.cxx
> ScintillaGTK.cxx:2921:9: error: call to 'scintilla_send_message' is ambiguous
> return scintilla_send_message(sci, iMessage, wParam, lParam);
> ^~~~~~~~~~~~~~~~~~~~~~
> ./../include/ScintillaWidget.h:64:9: note: candidate function

Yes, I see. That's obviously not a good solution. What about this:

#ifdef SCI_NAMESPACE
void (* notify) (ScintillaObject *sci, int id, Scintilla::SCNotification *scn);
#else
void (* notify) (ScintillaObject *sci, int id, SCNotification *scn);
#endif

or you could define a macro SCINTILLA_NS which resolves to nothing or the Scintilla namespace depending on the SCI_NAMESPACE macro.

Mike
--
www.soft-gems.net

[Neil Hodgson]

Mike Lischke:

> Yes, I see. That's obviously not a good solution. What about this:
>
> #ifdef SCI_NAMESPACE
> void (* notify) (ScintillaObject *sci, int id, Scintilla::SCNotification *scn);
> #else
> void (* notify) (ScintillaObject *sci, int id, SCNotification *scn);
> #endif
>
> or you could define a macro SCINTILLA_NS which resolves to nothing or the Scintilla namespace depending on the SCI_NAMESPACE macro.

Its difficult to work out all the consequences of these changes as they are increasing the scope of the SCI_NAMESPACE divergence.

SCNotification has to be usable from C code as well as C++ so it has to be available without the namespace. It may be cleaner to move SCNotification out of the Scintilla namespace. Possibly move all the Sci_* structs in Scintilla.h out of the namespace since they are also used from C.

Neil

[Matthew Brush]

That's what I was going to suggest as well. As long the names are
prefixed it shouldn't cause much pollution. For C++ it could be made
even nicer by doing something like `using Notification =
::SCNotification;` for those structs from inside the Scintilla namespace.

Regards,
Matthew Brush

[Mike Lischke]

Am 10.11.2016 um 23:14 schrieb Neil Hodgson <nyama...@me.com>:

#ifdef SCI_NAMESPACE
void (* notify) (ScintillaObject *sci, int id, Scintilla::SCNotification *scn);
#else
void (* notify) (ScintillaObject *sci, int id, SCNotification *scn);
#endif

or you could define a macro SCINTILLA_NS which resolves to nothing or the Scintilla namespace depending on the SCI_NAMESPACE macro.

  Its difficult to work out all the consequences of these changes as they are increasing the scope of the SCI_NAMESPACE divergence.

  SCNotification has to be usable from C code as well as C++ so it has to be available without the namespace. It may be cleaner to move SCNotification out of the Scintilla namespace. Possibly move all the Sci_* structs in Scintilla.h out of the namespace since they are also used from C.

Well, then the first variant is the one with least impact (it affects a single line only), even though it makes the code a bit less readable.

Mike
-- 
www.soft-gems.net

[Neil Hodgson]

Mike Lischke:

> Well, then the first variant is the one with least impact (it affects a single line only), even though it makes the code a bit less readable.

It affects the way that declaration is seen from different sites. It doesn’t appear to handle the situation where SCI_NAMESPACE is turned on for the project but the header is included by a C file.

This isn’t the first time there have been issues with the namespace on GTK+ and the recent accessibility code had to deal with this. The namespace has been handled poorly in past changes with quick workarounds when it caused compilation problems.

I think there will be fewer problems in the future if the namespace is removed from Scintilla.h. There could be client code that defined SCI_NAMESPACE and contains "Scintilla::” qualifiers which may have to change but SCI_NAMESPACE was never a documented feature of the API - its not mentioned in http://www.scintilla.org/ScintillaDoc.html .

Neil

[Mike Lischke]

>> Well, then the first variant is the one with least impact (it affects a single line only), even though it makes the code a bit less readable.
>
> It affects the way that declaration is seen from different sites. It doesn’t appear to handle the situation where SCI_NAMESPACE is turned on for the project but the header is included by a C file.

For me that sounds like optimizing it for the wrong side. Scintilla is a C++ lib, hence it should always behave like that.

>
> This isn’t the first time there have been issues with the namespace on GTK+ and the recent accessibility code had to deal with this. The namespace has been handled poorly in past changes with quick workarounds when it caused compilation problems.

IMO the namespace should always be enabled. This is how a good C++ library works to isolate its parts. Just because **C** programs would have trouble using it then is IMO no a good argument against that. How large is the C user base compared to C++?

>
> I think there will be fewer problems in the future if the namespace is removed from Scintilla.h. There could be client code that defined SCI_NAMESPACE and contains "Scintilla::” qualifiers which may have to change but SCI_NAMESPACE was never a documented feature of the API - its not mentioned in http://www.scintilla.org/ScintillaDoc.html .

Every other C++ lib (I know, and that are quite a few) defines its own namespace(s) itself and doesn't rely on the user code to do that. And all that to make a special case work...

Mike
--
www.soft-gems.net

[Nicholai Benalal]

On 12 November 2016 at 10:34:44 +01:00, 'Mike Lischke' via scintilla-interest <scintilla...@googlegroups.com> wrote:

For me that sounds like optimizing it for the wrong side. Scintilla is a C++ lib, hence it should always behave like that.

This isn’t the first time there have been issues with the namespace on GTK+ and the recent accessibility code had to deal with this. The namespace has been handled poorly in past changes with quick workarounds when it caused compilation problems.

IMO the namespace should always be enabled. This is how a good C++ library works to isolate its parts. Just because **C** programs would have trouble using it then is IMO no a good argument against that. How large is the C user base compared to C++?

I have no idea of the relative sizes of the user bases but I do suspect that you're right in that C++ is used much more widely. 

However, there are many people like myself who continue to use plain C and Scintilla was always a library that was designed in a way that would make this straight forward: the external interface is very small (essentially one message passing function) and it does not need the user to make use of weirdo C++ features. Nowhere have I seen is it documented to be "a C++ lib" and the fact that it is written in C++ does not change this in my mind. I'm sure I would be able to work around any namespace changes but this is just to let you know that there are people who would really hate if the Scintilla external API design was gradually made to depend on C++. I'm really relieved that Neil seems to have no such plans.

Best,

Nicholai

[Mike Lischke]

Hi Nicolai,

Thanks a lot for your input. As usual, you cannot please everyone. The question is always whom to make unhappy, right?

Mike
-- 
www.soft-gems.net

[Matthew Brush]

On 2016-11-12 01:34 AM, 'Mike Lischke' via scintilla-interest wrote:
>
>>> Well, then the first variant is the one with least impact (it affects a single line only), even though it makes the code a bit less readable.
>>
>> It affects the way that declaration is seen from different sites. It doesn’t appear to handle the situation where SCI_NAMESPACE is turned on for the project but the header is included by a C file.
>
> For me that sounds like optimizing it for the wrong side. Scintilla is a C++ lib, hence it should always behave like that.
>

Of the "official" platforms in the source tree, GTK+ and Win32 are C,
Cocoa is Objective-C, and only Qt exposes an actual C++ API.

>>
>> This isn’t the first time there have been issues with the namespace on GTK+ and the recent accessibility code had to deal with this. The namespace has been handled poorly in past changes with quick workarounds when it caused compilation problems.
>
> IMO the namespace should always be enabled. This is how a good C++ library works to isolate its parts. Just because **C** programs would have trouble using it then is IMO no a good argument against that. How large is the C user base compared to C++?
>

Neither of the two widely used C++ platforms/toolkits for Scintilla (Qt
and wxWidgets) use namespaces by default in their own code, so not using
namespaces is fairly common (albeit weird).

Regards,
Matthew Brush

[Neil Hodgson]

Mike Lischke:

> Scintilla is a C++ lib, hence it should always behave like that.

Scintilla is primarily a platform library. For Win32 it behaves as a control - its housed in a DLL and created as a HWND. On GTK+, its a GtkWidget which is defined in C. On Cocoa, its an Objective C class derived from NSView. On Qt, its a C++ class derived from QObject.

On every platform its interface is defined by a set of #define statements and struct definitions which are compatible with C. So, it is C-oriented. Only on Qt (out of the platforms in the main repository) is it represented as a C++ class.

It is implemented in C++ but that shouldn’t affect clients and shouldn’t even be seen by those clients.

> IMO the namespace should always be enabled. This is how a good C++ library works to isolate its parts.

The Scintilla namespace was added for Carbon on OS X so that the internal Scintilla identifiers wouldn’t clash with the Carbon headers. This is still the case: ‘Point’ and ‘Style’ clash with types defined in the old MacTypes.h system header.

> Just because **C** programs would have trouble using it then is IMO no a good argument against that. How large is the C user base compared to C++?

Indirectly, its large: most scripting languages and language interoperation tools are based on C.

> Every other C++ lib (I know, and that are quite a few) defines its own namespace(s) itself and doesn't rely on the user code to do that. And all that to make a special case work…

To me, this makes the general case consistent: SCNotification is always just SCNotification - its not two types (SCNotification and Scintilla::SCNotification) with one used in some contexts and the second in other contexts.

Neil

[Mike Lischke]

Am 12.11.2016 um 22:56 schrieb Neil Hodgson <nyama...@me.com>:

  The Scintilla namespace was added for Carbon on OS X so that the internal Scintilla identifiers wouldn’t clash with the Carbon headers. This is still the case: ‘Point’ and ‘Style’ clash with types defined in the old MacTypes.h system header.

If these are the only clashes why don't you rename them? And add a prefix to each Scintilla type to avoid future clashes (kinda poor man's namespace) - that way you could remove the namespace.

And for SCNotification you could define a new type for the C-like interface which is defined either with or without namespace part, depending on the SCI_NAMESPACE flag (if you want to keep the namespace, which I would appreciate).

Mike
-- 
www.soft-gems.net

[Matthew Brush]

On 2016-11-13 01:07 AM, 'Mike Lischke' via scintilla-interest wrote:
>
>> Am 12.11.2016 um 22:56 schrieb Neil Hodgson <nyama...@me.com>:
>>
>> The Scintilla namespace was added for Carbon on OS X so that the internal Scintilla identifiers wouldn’t clash with the Carbon headers. This is still the case: ‘Point’ and ‘Style’ clash with types defined in the old MacTypes.h system header.
>
>
> If these are the only clashes why don't you rename them? And add a prefix to each Scintilla type to avoid future clashes (kinda poor man's namespace) - that way you could remove the namespace.
>

+1, AFAICT neither of those are "public", so they could prefixed/renamed
as needed without affecting applications using the documented API. Seems
a lot easier than adding conditional compilation guards all over.

Regards,
Matthew Brush

[Neil Hodgson]

Mike Lischke:

> If these are the only clashes why don't you rename them?

At the time (2005-2007), the reason was to avoid other platform layers needing to change. Particularly externally maintained platform layers. Earlier, there was an issue on Win32 with an identifier which was actually a macro in the Windows headers and that was avoided with a rename (macros aren’t affected by namespaces). New platform layers may also have new clashes and the namespace avoided those potential clashes.

‘Style’ would likely be fine but ‘Point' is very common in platform interface code and would cause all the externally maintained platform layers to fail to compile and hence require work.

The OS X patch was contributed using the namespace and that appeared OK to me. It was conditional and I expected it to only be used on OS X. Some similar issues had occurred before (on wxWidgets) but it hadn’t been possible to use a namespace as wxWidgets didn’t allow namespace as it wasn’t available from older compilers.

Since the current OS X platform layer is for Cocoa rather than Carbon and Cocoa uses prefixed names, I wonder if it is possible to avoid MacTypes.h which appears to be for access to really old layers of Mac operating systems.

Here is the announcement of the merge although there were earlier mails (some private) about the change:
http://www.mail-archive.com/scintilla...@lyra.org/msg02665.html

> And for SCNotification you could define a new type for the C-like interface which is defined either with or without namespace part, depending on the SCI_NAMESPACE flag (if you want to keep the namespace, which I would appreciate).

I don’t see how that differs from the current situation.

Neil

[Neil Hodgson]

Mike Lischke:

> IMO the namespace should always be enabled. This is how a good C++ library works to isolate its parts.

Scintilla has never placed uptr_t and sptr_t in the Scintilla namespace and Sci_Position, Sci_PositionU and Sci_PositionCR weren’t included in the namespace when they were added 16 months ago.

An additional SCI_CLIENT_NAMESPACE could be added for C++ clients that want to segregate the Scintilla library’s names. This would only ever be defined for clients and the namespace would be invisible to the Scintilla implementation. I wouldn’t want to implement or maintain this.

Neil

[Mike Lischke]

>> IMO the namespace should always be enabled. This is how a good C++ library works to isolate its parts.
>
> Scintilla has never placed uptr_t and sptr_t in the Scintilla namespace and Sci_Position, Sci_PositionU and Sci_PositionCR weren’t included in the namespace when they were added 16 months ago.
>
> An additional SCI_CLIENT_NAMESPACE could be added for C++ clients that want to segregate the Scintilla library’s names. This would only ever be defined for clients and the namespace would be invisible to the Scintilla implementation. I wouldn’t want to implement or maintain this.

I don't think we need to discuss how important it is to avoid namespace pollution, but since Scintilla sees itself more like a black box with a simple C interface, a namespace is perhaps not that important. Fact is that all symbols in Scintilla's headers, which applications import, are added to the root namespace, regardless if there is a prefix or not. It's not only about name clashes, but also what appears when a tool (e.g. an IDE) lists for namespace members.

However, I wouldn't want yet another macro to control the namespace in Scintilla. If you don't want the namespace at all then remove it. A solution which only works half way is even worse than that.

Mike
--
www.soft-gems.net

[Neil Hodgson]

Mike Lischke:

> I don't think we need to discuss how important it is to avoid namespace pollution, but since Scintilla sees itself more like a black box with a simple C interface, a namespace is perhaps not that important.

> …

> However, I wouldn't want yet another macro to control the namespace in Scintilla. If you don't want the namespace at all then remove it. A solution which only works half way is even worse than that.

OK, committed the change. For Cocoa, this meant removing the Scintilla namespace from ScintillaView.h. The ScintillaCocoa declaration was also removed as it is inside the Scintilla namespace. This was done using a private category and a non-fragile ivar as available since the modern (2009) runtime. We already required the modern runtime since the change to libc++ in 2014.

I’d quite like to move the @private sections for the classes in ScintillaView.h into the ScintillaView.cxx implementation file since they are of no interest to client code.

Change to Scintilla.h:
https://sourceforge.net/p/scintilla/code/ci/1e7f830b65caeb2e0382905841174b6ddeec900b/
Cocoa changes:
https://sourceforge.net/p/scintilla/code/ci/732ad02df26d4a8396854de9138f138b28d51149/

Neil

[Mike Lischke]

>
> OK, committed the change. For Cocoa, this meant removing the Scintilla namespace from ScintillaView.h. The ScintillaCocoa declaration was also removed as it is inside the Scintilla namespace. This was done using a private category and a non-fragile ivar as available since the modern (2009) runtime. We already required the modern runtime since the change to libc++ in 2014.

Very good. Thanks Neil.

>
> I’d quite like to move the @private sections for the classes in ScintillaView.h into the ScintillaView.cxx implementation file since they are of no interest to client code.

Yes, that makes sense - encapsulation is a good thing (in this context).

Mike
--
www.soft-gems.net

[Neil Hodgson]

Moved the @private sections out of the ScintillaView.h header into the implementation file along with internal properties and methods:
https://sourceforge.net/p/scintilla/code/ci/259a7abaf4a6a6852816d82e09e46fef43998afd/

Neil