Discussion:
Enable g-ir-scanner for ScintillaWidget.h
Thomas Martitz
2014-07-22 13:04:26 UTC
Permalink
Hello folks,

the following patch enables g-ir-scanner to correctly parse
ScintillaWidget.h, i.e. so that it creates a .gir file (to be used with
gobject-introspection for automatic language bindings) for scintilla.

This patch changes ScintillaObject to a more GObject-friendly
ScintillaWidget. I provided a compatibility header so that scintilla
clients continue to compile and run fine when they upgrade their copy.
The header should be removed after some deprecation period.

If you want I can also provide the stub program that I created to make
g-ir-scanner do it's job.

I need this to enable python bindings to scintilla automatically via
introspection for the Geany IDE project.

Please comment and consider this for merging.

Best regards

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-22 23:11:58 UTC
Permalink
Thomas Martitz:

> This patch changes ScintillaObject to a more GObject-friendly ScintillaWidget.

This seems to be quite a heavy set of changes which only really modifies a couple of aspects: there are 6 new preprocessor macros that follow a naming scheme and the name is changed from "Scintilla" to "ScintillaWidget" with corresponding name changes flowing on.

To prevent client code churn, I'd prefer a smaller set of changes that maintains the current names and simply adds alternates that follow the naming convention.

What is the absolute minimum set of changes that allow use of g-ir-scanner?

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Thomas Martitz
2014-07-23 06:17:36 UTC
Permalink
Am Mittwoch, 23. Juli 2014 01:12:03 UTC+2 schrieb Neil Hodgson:
>
> Thomas Martitz:
>
> > This patch changes ScintillaObject to a more GObject-friendly
> ScintillaWidget.
>
> This seems to be quite a heavy set of changes which only really
> modifies a couple of aspects: there are 6 new preprocessor macros that
> follow a naming scheme and the name is changed from “Scintilla” to
> “ScintillaWidget” with corresponding name changes flowing on.
>
> To prevent client code churn, I’d prefer a smaller set of changes that
> maintains the current names and simply adds alternates that follow the
> naming convention.
>
> What is the absolute minimum set of changes that allow use of
> g-ir-scanner?
>
> Neil
>


Client code churn is prevented by the compatibility header. It will
continue to compile and run without any changes, so downstream can decide
themselv when to adopt the new naming scheme.

Is that still not acceptable?

(PS: ignore my other mail, i didn't receive your response as mail so I
assumed I had to resend my mail after my membership to this group is
confirmed)

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-23 23:01:48 UTC
Permalink
Thomas Martitz:

> Client code churn is prevented by the compatibility header.

You have deprecated the current interface, thus requiring clients to update. It is better to retain the current interface allowing clients to ignore this change.

> Is that still not acceptable?

Not without stronger justification.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Thomas Martitz
2014-07-24 06:43:01 UTC
Permalink
Am Donnerstag, 24. Juli 2014 01:01:53 UTC+2 schrieb Neil Hodgson:
>
> Thomas Martitz:
>
> > Client code churn is prevented by the compatibility header.
>
> You have deprecated the current interface, thus requiring clients to
> update. It is better to retain the current interface allowing clients to
> ignore this change.
>
>

In my understanding: the current interface is still working and supported
with my patch, clients need _not_ to update now and can ignore the change
for a while now. Perhaps you have missed it but that's because the
compatibility header is implicitly included by ScintillaWidget.h. Clients
need to update only once that implicit inclusion is removed, I don't care
if/when that's going to happen.



> > Is that still not acceptable?
>
> Not without stronger justification.
>
>

That's unfortunate.

The major issue with the current interface is that the type is registered
as "Scintilla" and the methods begin with scintilla_ while the actual
structure is called ScintillaObject. g-ir-scanner expects to find a
registered type ScintillaXXX and scintilla_xxx_ (first and foremost
scintilla_xxx_get_type()). Another problem is that the class structure is
called ScintillaClass and not ScintillaXXXClass.

Naming the type only Scintilla is not appropriate (for g-ir-scanner)
because objects should be named NamespaceTypename for introspection to pick
it up. You can see how it used to construct the type macros, but more
importantly the generated .gir file must have a <namespace
name="Namespace"> attribute, and one or more <class name="Typename">
attribtues under it.

Therefore I decided to name the object ScintillaWidget all over the place,
with the old name ScintillaObject still being available by default and
without any changes to client code.

Best regards

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Thomas Martitz
2014-07-24 07:46:23 UTC
Permalink
Am Donnerstag, 24. Juli 2014 08:43:01 UTC+2 schrieb Thomas Martitz:
>
>
>
> Best regards
>


To give a little bit more background. I'm doing this to make the following
python snipped work (as an example) without creating any bindings manually

"""
from gi.repository import Scintilla

if __name__ == "__main__":
sci = Scintilla.Widget()
sci.send_message(Scintilla.SCI_SETREADONLY, True, 0)
sci.connect("sci-notify", lambda x,y : print(str(x)))
"""

This can be achieved using gobject introspection. And since Scintilla is
already a gobject we're mostly set. We just need to make the source code
more friendly towards the introspection tools.

As a bonus, the python code will probably be the same or at least very
similar across scintilla clients (geany and others).

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-25 01:22:09 UTC
Permalink
Thomas Martitz:

> In my understanding: the current interface is still working and supported with my patch, clients need _not_ to update now and can ignore the change for a while now. Perhaps you have missed it but that's because the compatibility header is implicitly included by ScintillaWidget.h. Clients need to update only once that implicit inclusion is removed, I don't care if/when that's going to happen.

You have changed the canonical names of several features then covered that up with a compatibility header. Clients are expected to use the new names and will eventually break if they don't. Therefore there is a burden on clients to change names at some point. That burden should be avoided/minimised if at all possible.

A different approach would be to retain all the current names as canonical and introduce g-ir-scanner friendly names in a new header that is maintained for use with introspection. Clients may choose either header depending on whether they care about introspection. Nothing would be deprecated.

> The major issue with the current interface is that the type is registered as "Scintilla" and the methods begin with scintilla_ while the actual structure is called ScintillaObject. g-ir-scanner expects to find a registered type ScintillaXXX and scintilla_xxx_ (first and foremost scintilla_xxx_get_type()). Another problem is that the class structure is called ScintillaClass and not ScintillaXXXClass.

Why wasn't this information in the original post? Patch dumps are not fun for maintainers. Not including the reasons for changes makes them difficult to review.

g-ir-scanner seems to allow omitting namespaces with --accept-unprefixed.

As I said in an earlier mail, "What is the absolute minimum set of changes that allow use of g-ir-scanner?"

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Thomas Martitz
2014-07-26 22:26:09 UTC
Permalink
Am Freitag, 25. Juli 2014 03:22:14 UTC+2 schrieb Neil Hodgson:
>
> Thomas Martitz:
>
> > In my understanding: the current interface is still working and
> supported with my patch, clients need _not_ to update now and can ignore
> the change for a while now. Perhaps you have missed it but that's because
> the compatibility header is implicitly included by ScintillaWidget.h.
> Clients need to update only once that implicit inclusion is removed, I
> don't care if/when that's going to happen.
>
> You have changed the canonical names of several features then covered
> that up with a compatibility header. Clients are expected to use the new
> names and will eventually break if they don’t. Therefore there is a burden
> on clients to change names at some point. That burden should be
> avoided/minimised if at all possible.
>


Right, and I understand that and I fully agree with your last point. Client
compatibility is important for me too. And I hoped I successfully minimized
the burden with the compat-header. Do yo disagree with that?. The compat
header might very well be never dropped to keep the current names working
forever.

On the other hand I also do not want to unnecessarily increase the
maintenance burden for the scintilla project by creating new headers with
duplicated definitions (see below).



>
> A different approach would be to retain all the current names as
> canonical and introduce g-ir-scanner friendly names in a new header that is
> maintained for use with introspection. Clients may choose either header
> depending on whether they care about introspection. Nothing would be
> deprecated.
>


For g-ir-scanner the struct name and the runtime-registered typename must
match. This isn't the case currently: the typename is registered as
Scintilla and the struct is ScintillaObject. Plus, plus naming the struct
just Scintilla makes g-ir-scanner unable to infer the namespace which is
important for the generated .gir and .typelib as well as the thing which is
imported by scripts (e.g. "from gi.repository import <namespace>")
(--accept-unprefixed is not an option because it pulls in all other
unrelated symbols too). The namespace is needed for inferring the
_get_type() function for each gobject as well.

IIUC a g-ir-scanner-friendly header doesnt suffice. You still need to match
the struct name and typename. And you'd have to duplicate the struct
defintion in the new header, as g-ir-scanner doesn't pull in the struct
defintion when it sees a typedef. Perhaps I'm missing something here?



>
> > The major issue with the current interface is that the type is
> registered as "Scintilla" and the methods begin with scintilla_ while the
> actual structure is called ScintillaObject. g-ir-scanner expects to find a
> registered type ScintillaXXX and scintilla_xxx_ (first and foremost
> scintilla_xxx_get_type()). Another problem is that the class structure is
> called ScintillaClass and not ScintillaXXXClass.
>
> Why wasn’t this information in the original post? Patch dumps are not
> fun for maintainers. Not including the reasons for changes makes them
> difficult to review.
>
> g-ir-scanner seems to allow omitting namespaces with
> —accept-unprefixed.
>
>
I'm sorry, you are right, I should have added that to the initial post. It
was not nice of me to provide insufficient information. It was an oversight
on my side. I hope to have cleared that up now. If you have other questions
feel free to ask them, and I hope to be able to answer them.

But, on the other hand, I'm just a user of GIR and am relatively new to the
related tools as well so there might be information that is not known to me
either.




> As I said in an earlier mail, “What is the absolute minimum set of
> changes that allow use of g-ir-scanner?"
>
>

a) The struct name and the runtime-registered typename must match
b) Methods must follow the <namespace>_<type>_xxx convention, and at least
<namespace>_<type>_get_type() must be present.

I'm pretty confident that I implemented the minimum set of changes while
keeping compatibility for clients. What we could do is to undo the
ScintillaObject -> ScintillaWidget part of the change, if you like to make
the diff a bit smaller. Though I find ScintillaWidget is more appropriate
because GtkWidget is earlier in the class hierarchy and
scintilla_new/scintilla_{object,widget}_new() returns a GtkWidget.

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Thomas Martitz
2014-07-28 14:15:07 UTC
Permalink
Moving us back to scintilla-interest, assuming you mailed privately by
accident.

Am 27.07.2014 01:44, schrieb Neil Hodgson:
> Thomas Martitz:
>
>> And I hoped I successfully minimized the burden with the compat-header. Do yo disagree with that?. The compat header might very well be never dropped to keep the current names working forever.
>
> The existence of the use of the compatibility header may cause confusion and support issues. If possible I'd like to avoid it.
>
>> For g-ir-scanner the struct name and the runtime-registered typename must match. This isn't the case currently: the typename is registered as Scintilla and the struct is ScintillaObject.
>
> I'm still not seeing why ScintillaObject is changed to ScintillaWidget. Does there have to be a correspondence between the name of the header file and the struct name?
>
>> Plus, plus naming the struct just Scintilla makes g-ir-scanner unable to infer the namespace which is important for the generated .gir and .typelib as well as the thing which is imported by scripts (e.g. "from gi.repository import <namespace>") (--accept-unprefixed is not an option because it pulls in all other unrelated symbols too).
>
> Python's import statement can pick individual classes.
>
>> I'm pretty confident that I implemented the minimum set of changes while keeping compatibility for clients. What we could do is to undo the ScintillaObject -> ScintillaWidget part of the change, if you like to make the diff a bit smaller. Though I find ScintillaWidget is more appropriate because GtkWidget is earlier in the class hierarchy and scintilla_new/scintilla_{object,widget}_new() returns a GtkWidget.
>
> That would be a start. Part of my problem is that this is a 300 line patch for something that doesn't appear that big.
>
> Neil
>


Okay, here is a patch that keeps ScintillaObject. The compatiblity
header is still required as an extra file because g-ir-scanner otherwise
chokes on the extra symbols.

Please tell me what you think.

Best regards

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-28 22:25:24 UTC
Permalink
Thomas Martitz:

> Moving us back to scintilla-interest, assuming you mailed privately by accident.

Yes, accident.

> Okay, here is a patch that keeps ScintillaObject.

So why did [_]ScintillaClass change to [_]ScintillaObjectClass?

> The compatiblity header is still required as an extra file because g-ir-scanner otherwise chokes on the extra symbols.

Isn't there an #if it understands?

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-28 23:03:11 UTC
Permalink
On 14-07-28 03:25 PM, Neil Hodgson wrote:
> Thomas Martitz:
>
>> Moving us back to scintilla-interest, assuming you mailed privately by accident.
>
> Yes, accident.
>
>> Okay, here is a patch that keeps ScintillaObject.
>
> So why did [_]ScintillaClass change to [_]ScintillaObjectClass?
>

FWIW, that typo also makes it hard to use Scintilla with Vala because
like g-ir-scanner, it also assumes the proper naming conventions are
followed (same goes for most of the other stuff Thomas fixed).

>> The compatiblity header is still required as an extra file because g-ir-scanner otherwise chokes on the extra symbols.
>
> Isn't there an #if it understands?
>

You can -DSOMETHING for the g-ir-scanner and then #ifdef guard certain
code when scanning, but then the improvements wouldn't be available
other places these fixes are useful (ex. subclassing, using from
Vala/Genie, likely GTKmm, etc.), unless of course they also define the
same macro before hand.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-28 23:34:54 UTC
Permalink
Matthew Brush:

> FWIW, that typo also makes it hard to use Scintilla with Vala because like g-ir-scanner, it also assumes the proper naming conventions are followed (same goes for most of the other stuff Thomas fixed).

Was this naming convention documented somewhere?

> You can -DSOMETHING for the g-ir-scanner and then #ifdef guard certain code when scanning, but then the improvements wouldn't be available other places these fixes are useful (ex. subclassing, using from Vala/Genie, likely GTKmm, etc.), unless of course they also define the same macro before hand.

The #if would protect the definitions that cause g-ir-scanner to choke. Does Vala also choke on these definitions?

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-29 00:01:16 UTC
Permalink
On 14-07-28 04:34 PM, Neil Hodgson wrote:
> Matthew Brush:
>
>> FWIW, that typo also makes it hard to use Scintilla with Vala because like g-ir-scanner, it also assumes the proper naming conventions are followed (same goes for most of the other stuff Thomas fixed).
>
> Was this naming convention documented somewhere?
>

https://developer.gnome.org/gobject/stable/gtype-conventions.html

>> You can -DSOMETHING for the g-ir-scanner and then #ifdef guard certain code when scanning, but then the improvements wouldn't be available other places these fixes are useful (ex. subclassing, using from Vala/Genie, likely GTKmm, etc.), unless of course they also define the same macro before hand.
>
> The #if would protect the definitions that cause g-ir-scanner to choke. Does Vala also choke on these definitions?
>

It's a bit different for Vala, instead when you write a class, like say:

class MyScintilla : Scintilla.Object {
// ...
}

It generates GObject C code that gets compiled like normal C code, and
it will output something like this for the structures:

struct _MyScintilla {
ScintillaObject parent;
}

struct _MyScintillaClass {
ScintillaObjectClass parent;
}

It may be possible to control how it outputs this using code generation
attributes in the API binding or Vala code, but by default it doesn't work.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-29 00:29:48 UTC
Permalink
Matthew Brush:

> It's a bit different for Vala, instead when you write a class, like say:

>
> class MyScintilla : Scintilla.Object {
> // ...
> }
>
> It generates GObject C code that gets compiled like normal C code, and it will output something like this for the structures:
>
> struct _MyScintilla {
> ScintillaObject parent;
> }
>
> struct _MyScintillaClass {
> ScintillaObjectClass parent;
> }

ScintillaClass is a typedef of _ScintillaClass (which is not defined by the naming convention) so can't the convention-following name be added with
typedef struct _ScintillaClass ScintillaObjectClass;

scintilla_init and scintilla_class_init are static so there is no need to change them to scintilla_object_init and scintilla_object_class_init.

Since the patch only registers (g_type_register_static) "ScintillaObject" and not "Scintilla" won't it break any applications that are calling something like g_type_from_name("Scintilla")?

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-29 00:44:17 UTC
Permalink
On 14-07-28 05:29 PM, Neil Hodgson wrote:
> Matthew Brush:
>
>> It's a bit different for Vala, instead when you write a class, like say:
>
>>
>> class MyScintilla : Scintilla.Object {
>> // ...
>> }
>>
>> It generates GObject C code that gets compiled like normal C code, and it will output something like this for the structures:
>>
>> struct _MyScintilla {
>> ScintillaObject parent;
>> }
>>
>> struct _MyScintillaClass {
>> ScintillaObjectClass parent;
>> }
>
> ScintillaClass is a typedef of _ScintillaClass (which is not defined by the naming convention) so can't the convention-following name be added with
> typedef struct _ScintillaClass ScintillaObjectClass;
>

Yes.

> scintilla_init and scintilla_class_init are static so there is no need to change them to scintilla_object_init and scintilla_object_class_init.
>

Yeah, I think since ScintillaGTK.cxx implements its own _get_type()
function instead of using the G_DEFINE_TYPE macro, which assumes certain
naming conventions.

> Since the patch only registers (g_type_register_static) "ScintillaObject" and not "Scintilla" won't it break any applications that are calling something like g_type_from_name("Scintilla")?
>

Yes, I think it would.

For the purposes of Vala and possibly g-ir-scanner, you could probably
make it work with a simple wrapper header like this:

// ScintillaWrapper.h
#ifndef SCINTILLA_WRAPPER_H
#define SCINTILLA_WRAPPER_H
#define GTK // ScintillaWidget.h needs this
#include "Scintilla.h" // ScintillaWidget.h needs this
#include <gtk/gtk.h> // ScintillaWidget.h needs this
#include <ScintillaWidget.h>
typedef struct _ScintillaClass ScintillaObjectClass;
#define SCINTILLA_TYPE_OBJECT (scintilla_object_get_type())
#define scintilla_object_get_type scintilla_get_type
#define SCINTILLA_IS_OBJECT(obj) IS_SCINTILLA(obj)
#define SCINTILLA_OBJECT(obj) SCINTILLA(obj)
#define scintilla_object_new scintilla_new
#define scintilla_object_send_message scintilla_send_message
#endif

Even less probably, but that handles the most common uses.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-29 00:54:25 UTC
Permalink
On 14-07-28 05:44 PM, Matthew Brush wrote:
> [snip]
> For the purposes of Vala and possibly g-ir-scanner, you could probably
> make it work with a simple wrapper header like this:
>
> // ScintillaWrapper.h
> [snip]

Actually a header like that probably wouldn't work with
GObject-Introspection since I guess it does code parsing and runtime
type system introspection and stuff, after the macros are expanded. For
Vala's code-generator, something like that suffices though.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-29 01:01:20 UTC
Permalink
Matthew Brush:

>> Since the patch only registers (g_type_register_static) "ScintillaObject" and not "Scintilla" won't it break any applications that are calling something like g_type_from_name("Scintilla")?
>>
>
> Yes, I think it would.

I suppose adding an second name for the type like this would lead to failures.
g_type_register_static(GTK_TYPE_CONTAINER, "Scintilla", ...
g_type_register_static(GTK_TYPE_CONTAINER, "ScintillaObject", ...


While asking on the mailing list won't find everyone, its best to try:

Is anyone accessing the Scintilla type by string name "Scintilla" on GTK+?

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-29 01:14:32 UTC
Permalink
On 14-07-28 06:01 PM, Neil Hodgson wrote:
> Matthew Brush:
>
>>> Since the patch only registers (g_type_register_static) "ScintillaObject" and not "Scintilla" won't it break any applications that are calling something like g_type_from_name("Scintilla")?
>>>
>>
>> Yes, I think it would.
>
> I suppose adding an second name for the type like this would lead to failures.
> g_type_register_static(GTK_TYPE_CONTAINER, "Scintilla", ...
> g_type_register_static(GTK_TYPE_CONTAINER, "ScintillaObject", ...
>

You would probably have to fully register a separate GType, like make
another _get_type() function that registers/returns a different static
GType id with the different names and such.

>
> While asking on the mailing list won't find everyone, its best to try:
>
> Is anyone accessing the Scintilla type by string name "Scintilla" on GTK+?
>

FWIW, I don't think Geany does (after a quick search).

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-29 01:19:16 UTC
Permalink
On 14-07-28 06:14 PM, Matthew Brush wrote:
> On 14-07-28 06:01 PM, Neil Hodgson wrote:
>> Matthew Brush:
>>
>>>> Since the patch only registers (g_type_register_static)
>>>> "ScintillaObject" and not "Scintilla" won't it break any
>>>> applications that are calling something like
>>>> g_type_from_name("Scintilla")?
>>>>
>>>
>>> Yes, I think it would.
>>
>> I suppose adding an second name for the type like this would lead
>> to failures.
>> g_type_register_static(GTK_TYPE_CONTAINER, "Scintilla", ...
>> g_type_register_static(GTK_TYPE_CONTAINER, "ScintillaObject", ...
>>
>
> You would probably have to fully register a separate GType, like make
> another _get_type() function that registers/returns a different static
> GType id with the different names and such.
>

...and also take care of static members of ScintillaGTK class and
multiple calls to ScintillaGTK::ClassInit().

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
kugel
2014-07-29 20:49:39 UTC
Permalink
Am Dienstag, 29. Juli 2014 02:29:53 UTC+2 schrieb Neil Hodgson:
>
> Matthew Brush:
>
> > It's a bit different for Vala, instead when you write a class, like say:
>
> >
> > class MyScintilla : Scintilla.Object {
> > // ...
> > }
> >
> > It generates GObject C code that gets compiled like normal C code, and
> it will output something like this for the structures:
> >
> > struct _MyScintilla {
> > ScintillaObject parent;
> > }
> >
> > struct _MyScintillaClass {
> > ScintillaObjectClass parent;
> > }
>
> ScintillaClass is a typedef of _ScintillaClass (which is not defined
> by the naming convention) so can’t the convention-following name be added
> with
> typedef struct _ScintillaClass ScintillaObjectClass;
>
>
I think that could work, though it's pretty equivalent to the "typedef
struct _ScintillaObjectClass ScintillaClass;" that my patch contains.
In the end both ScintillaClass and ScintillaObjectClass are defined to the
same type.



> scintilla_init and scintilla_class_init are static so there is no need
> to change them to scintilla_object_init and scintilla_object_class_init.
>
>

Ok, but since it's static client code is not affected and therefore it's a
good opportunity to adapt gobject naming style. It would be going to be
necessary if we wanted to change to one of the G_DEFINE_TYPE() macros.



> Since the patch only registers (g_type_register_static)
> “ScintillaObject” and not “Scintilla” won’t it break any applications that
> are calling something like g_type_from_name("Scintilla")?
>
>
Yes I think so. At least such breakage would be immediately obvious by an
application crash if clients do not update their code (once they use the
NULL return value). But as noted at the end of the mail I don't think
anyone is calling g_type_from_name("Scintilla").

> The compatiblity header is still required as an extra file because
> g-ir-scanner otherwise chokes on the extra symbols.
>
> Isn’t there an #if it understands?



There is not a predefined macro, but as Matthew said you can have
g-ir-scanner define one (and consequently skip sections). IMO a separate
file would be cleaner because we don't have to make up a define that all
clients have to follow. Is there a specific reason to avoid the extra
header (it doesn't affect client code)?


>
> I suppose adding an second name for the type like this would lead to
> failures.
> g_type_register_static(GTK_TYPE_CONTAINER, “Scintilla”, 

> g_type_register_static(GTK_TYPE_CONTAINER, “ScintillaObject”, 

>
>
I never tried, but in the best case you'll get to two distinct GTypes which
are not compatible for glib, so it's not an option.


>
> While asking on the mailing list won’t find everyone, its best to try:
>
> Is anyone accessing the Scintilla type by string name “Scintilla” on
> GTK+?
>


Seems unlikely given how scintilla is distributed: clients have immediate
access to all gtype-related macros and scintilla_get_type(). In fact I
haven't seen g_type_from_name() in any real code even outside scintilla.

Do you want another patch round or could we convince you? :)

PS: I would like to emphasize on what Matthew brought up; not only
g-ir-scanner friendly-ness is required. Being able to interface with vala
code, such as creating subclasses, is also important.

Best regards

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-30 05:26:31 UTC
Permalink
kugel:

> Ok, but since it's static client code is not affected and therefore it's a good opportunity to adapt gobject naming style.

I want to review a *minimal* patch. Bugs hide in unreviewed code and the more lines of change the less each is examined.

> Yes I think so. At least such breakage would be immediately obvious by an application crash if clients do not update their code (once they use the NULL return value). But as noted at the end of the mail I don't think anyone is calling g_type_from_name("Scintilla").

Its very dangerous to assume anything about how client code is using legitimate calls. Crashing applications leads to unhappiness.

> There is not a predefined macro, but as Matthew said you can have g-ir-scanner define one (and consequently skip sections).

How? For that matter, what command are you using to invoke g-ir-scanner?

> IMO a separate file would be cleaner because we don't have to make up a define that all clients have to follow. Is there a specific reason to avoid the extra header (it doesn't affect client code)?

Avoiding additional files avoids complexity and various forms of failure. Should make dependencies be updated for this change? Never multiply entities without need.

> I suppose adding an second name for the type like this would lead to failures.
> g_type_register_static(GTK_TYPE_CONTAINER, "Scintilla", ...
> g_type_register_static(GTK_TYPE_CONTAINER, "ScintillaObject", ...
>
>
> I never tried, but in the best case you'll get to two distinct GTypes which are not compatible for glib, so it's not an option.

Why isn't it an option? It looks to me like exporting two distinct types would support all current users as well as users of introspection.

> Seems unlikely given how scintilla is distributed: clients have immediate access to all gtype-related macros and scintilla_get_type(). In fact I haven't seen g_type_from_name() in any real code even outside scintilla.

I'd expect it to be used when applications want to be able to instantiate widgets that aren't known at compile time. Perhaps in form builders or testing applications. Ohloh showed 56 million hits for "g_type_from_name" and while I'm sure lots of those are checking before registration, some are interesting.

> Do you want another patch round or could we convince you? :)

I want to see a minimal change set that I can easily review. It took too much work to uncover the fact that the current changes are incompatible with client code that looks up Scintilla by name. It would have been even better if the original submission had included this information.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
kugel
2014-07-30 07:42:54 UTC
Permalink
Am Mittwoch, 30. Juli 2014 07:26:51 UTC+2 schrieb Neil Hodgson:
>
> kugel:
>
> Ok, but since it's static client code is not affected and therefore it's a
> good opportunity to adapt gobject naming style.
>
>
> I want to review a *minimal* patch. Bugs hide in unreviewed code and
> the more lines of change the less each is examined.
>


Ok, I understand that.



>
> Yes I think so. At least such breakage would be immediately obvious by an
> application crash if clients do not update their code (once they use the
> NULL return value). But as noted at the end of the mail I don't think
> anyone is calling g_type_from_name("Scintilla”).
>
>
> Its very dangerous to assume anything about how client code is using
> legitimate calls. Crashing applications leads to unhappiness.
>


You are right, but avoiding unhappiness sometimes also comes at a (serious)
cost. I'm still questioning that any clients even makes this call. I
conducted source codes of Geany (doesn't call g_type_from_name()),
Code::Blocks (doesn't call g_type_from_name()), Scite (doesn't call
g_type_from_name()) and Anjuta (the main application does call it, but the
scintilla editor plugin doesn't. the scintilla editor is not used by
default and is not even part of the anjuta source code distribution, nor
part of the GNOME git repositories. the scintilla editor plugin is in a
out-of-tree github repository and not actively maintained (updated last
year, scintilla copy is still at 3.3.4)). Is there any known ScintillaGTK
user that I missed?


>
> There is not a predefined macro, but as Matthew said you can have
> g-ir-scanner define one (and consequently skip sections).
>
>
> How? For that matter, what command are you using to invoke g-ir-scanner?
>


g-ir-scanner --program ./girhelperscintilla -DGTK -Duptr_t=gulong
-Dsptr_t=glong $(pkg-config --cflags gtk+-2.0) -Iscintilla/include
--cflags-begin -include gtk/gtk.h --cflags-end -n Scintilla -i Gtk-2.0 -i
GObject-2.0 --warn-all scintilla/inlcude/ScintillaWidget.h -o Scintilla.gir

girhelperscintilla is a helper program which statically links scintilla and
exports (via --export-dynamic) the scintilla_object_* symbols for
gobject-introspection.

g-ir-scanner does not define any CPP macro on its own, but as you can see
you can define some with -D. You could define something like G_IR_SCANNER
to exclude header/code section just for g-ir-scanner

I still would like to avoid enforcing a define on clients. They might
already use it for other purposes (unlikely though), but more importantly
it potentially must be used for their non-scintilla code because you
normally want to collect GIR information in a single g-ir-scanner
invokation. This is because you compile 1 .gir file into 1 .typelib
(g-ir-compiler cannot compile a .typelib out of multiple .gir files). So if
the client wants scintilla in its global typelib it has to be in the same
gir.

Note though that this is not what I'm _currently_ doing for geany, there I
have an extra scintilla .gir/.typelib for now (though it includes a bit
more than just ScintillaObject)



> IMO a separate file would be cleaner because we don't have to make up a
> define that all clients have to follow. Is there a specific reason to avoid
> the extra header (it doesn't affect client code)?
>
>
> Avoiding additional files avoids complexity and various forms of
> failure. Should make dependencies be updated for this change? Never
> multiply entities without need.
>


What does the make dependency have to do with this?



>
> I suppose adding an second name for the type like this would lead to
>> failures.
>> g_type_register_static(GTK_TYPE_CONTAINER, “Scintilla”, 

>> g_type_register_static(GTK_TYPE_CONTAINER, “ScintillaObject”, 

>>
>>
> I never tried, but in the best case you'll get to two distinct GTypes
> which are not compatible for glib, so it's not an option.
>
>
> Why isn’t it an option? It looks to me like exporting two distinct
> types would support all current users as well as users of introspection.
>


Then IS_SCINTILLA(x) != SCINTILLA_IS_OBJECT(x), that would certainly be
unexpected and may not be easy worked around by client code. You could not
interchange Scintilla and ScintillaObject instances. (imagine a python
script which creates ScintillaObject instances (via GIR) and passes it to
the main application which expects Scintilla objects).

And my patch does #define {TYPE,IS}_SCINTILLA SCINTILLA_{TYPE,IS}_OBJECT
(and friends) therefore scintilla_new()-constructed objects are not
compatible with g_type_by_name("Scintilla"). Do you want these problems
just to avoid clients having to update g_type_by_name() calls (which I
still doubt they exist)? That would be unacceptable to me.



>
> Seems unlikely given how scintilla is distributed: clients have immediate
> access to all gtype-related macros and scintilla_get_type(). In fact I
> haven't seen g_type_from_name() in any real code even outside scintilla.
>
>
> I’d expect it to be used when applications want to be able to
> instantiate widgets that aren’t known at compile time. Perhaps in form
> builders or testing applications. Ohloh showed 56 million hits for
> “g_type_from_name” and while I’m sure lots of those are checking before
> registration, some are interesting.
>
> Do you want another patch round or could we convince you? :)
>
>
> I want to see a minimal change set that I can easily review. It took
> too much work to uncover the fact that the current changes are incompatible
> with client code that looks up Scintilla by name. It would have been even
> better if the original submission had included this information.
>
>

I'm sorry about this. I didn't (and still don't) think it would have any
impact. But registering the type as "ScintillaObject" is really a
requirement for GIR to work.

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-02 00:40:18 UTC
Permalink
kugel:

> g-ir-scanner --program ./girhelperscintilla -DGTK -Duptr_t=gulong -Dsptr_t=glong $(pkg-config --cflags gtk+-2.0) -Iscintilla/include --cflags-begin -include gtk/gtk.h --cflags-end -n Scintilla -i Gtk-2.0 -i GObject-2.0 --warn-all scintilla/inlcude/ScintillaWidget.h -o Scintilla.gir

This doesn't appear to be the actual command you are using since "include" is misspelt. After fixing that I see these messages:

scintilla/include/ScintillaWidget.h:2: Warning: Scintilla: GTK-Doc comment block start token "/**" should not be followed by comment text:
/** @file ScintillaWidget.h
^
scintilla/include/ScintillaWidget.h:3: Error: Scintilla: identifier not found on the first line:
@file ScintillaWidget.h
^
Traceback (most recent call last):
File "/usr/bin/g-ir-scanner", line 46, in <module>
sys.exit(scanner_main(sys.argv))
File "/usr/lib/gobject-introspection/giscanner/scannermain.py", line 488, in scanner_main
shlibs = create_binary(transformer, options, args)
File "/usr/lib/gobject-introspection/giscanner/scannermain.py", line 377, in create_binary
gdump_parser.parse()
File "/usr/lib/gobject-introspection/giscanner/gdumpparser.py", line 110, in parse
tree = self._execute_binary_get_tree()
File "/usr/lib/gobject-introspection/giscanner/gdumpparser.py", line 167, in _execute_binary_get_tree
subprocess.check_call(args, stdout=sys.stdout, stderr=sys.stderr)
File "/usr/lib/python2.7/subprocess.py", line 535, in check_call
retcode = call(*popenargs, **kwargs)
File "/usr/lib/python2.7/subprocess.py", line 522, in call
return Popen(*popenargs, **kwargs).wait()
File "/usr/lib/python2.7/subprocess.py", line 710, in __init__
errread, errwrite)
File "/usr/lib/python2.7/subprocess.py", line 1327, in _execute_child
raise child_exception
OSError: [Errno 2] No such file or directory
>Exit code: 1

> girhelperscintilla is a helper program which statically links scintilla and exports (via --export-dynamic) the scintilla_object_* symbols for gobject-introspection.

Where is the source to this?

> What does the make dependency have to do with this?

Currently downstream projects have dependencies in their make files (or other build files) from ScintillaWidget.h to their executables and this would have to change to depending on both ScintillaWidget.h and ScintillaWidgetCompat.h.

> Here's a new patch that should be really minimal in terms of diff-size that realizes my goal. However I don't believe it's the best from a technical viewpoint (i.e. it doesn't have the extra header, I still would prefer ScintillaWidget as the new name).

This is still changing stuff that doesn't appear to need changing like _ScintillaClass to _ScintillaObjectClass.

+ * All of this is deprecated and may be removed in the future, so you should
+ * transition to ScintillaObject or maintain a copy of the blow
+ */

"blow"? Just drop the deprecation commentary.

+void scintilla_object_set_id (ScintillaObject *sci, uptr_t id);

Since there is now SCI_SETIDENTIFIER, this no longer needs a function.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Colomban Wendling
2014-07-30 14:35:58 UTC
Permalink
Le 29/07/2014 22:49, kugel a écrit :
> [...]
> I suppose adding an second name for the type like this would lead
> to failures.
> g_type_register_static(GTK_TYPE_CONTAINER, “Scintilla”, …
> g_type_register_static(GTK_TYPE_CONTAINER, “ScintillaObject”, …
>
>
> I never tried, but in the best case you'll get to two distinct GTypes
> which are not compatible for glib, so it's not an option.

What about having ScintillaObject subclassing Scintilla? This way a
ScintillaObject instance would be a valid Scintilla one -- and bypassing
the type system, the other way around would also be true if the subclass
adds absolutely nothing.
It's probably not the best thing performance-wise as it probably adds a
little creation-time overhead, but I doubt it's much if it adds nothing
to the parent.

Just a thought.

Regards,
Colomban

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-30 21:48:06 UTC
Permalink
Colomban Wendling:

> What about having ScintillaObject subclassing Scintilla? This way a
> ScintillaObject instance would be a valid Scintilla one -- and bypassing
> the type system, the other way around would also be true if the subclass
> adds absolutely nothing.

That sounds like it would result in complete compatibility with the current release. In C++ subclassing a type requires little code but I'm unsure just what to do in GTK+ and expect it will take a bit more code.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-30 22:37:09 UTC
Permalink
On 14-07-30 02:48 PM, Neil Hodgson wrote:
> Colomban Wendling:
>
>> What about having ScintillaObject subclassing Scintilla? This way a
>> ScintillaObject instance would be a valid Scintilla one -- and bypassing
>> the type system, the other way around would also be true if the subclass
>> adds absolutely nothing.
>
> That sounds like it would result in complete compatibility with the current release. In C++ subclassing a type requires little code but I'm unsure just what to do in GTK+ and expect it will take a bit more code.
>

GtkScintilla is this, plus actual methods, signals, properties, etc like
the Qt widgets do, rather than the current Win32-API-style interface. I
stopped working on GtkScintilla as it's pointless if it's not part of
Scintilla proper (like the similar Qt backend is), but if you ever want
to add a good GTK+ API/widget to core, I'd be willing to revive/finish
it. I also have some improvements on it locally somewhere too that make
it more automated from the Sctinilla.iface, I just never bothered
merging them once it was rejected from core last time.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Colomban Wendling
2014-07-30 22:53:31 UTC
Permalink
Le 31/07/2014 00:37, Matthew Brush a écrit :
> On 14-07-30 02:48 PM, Neil Hodgson wrote:
>> Colomban Wendling:
>>
>>> What about having ScintillaObject subclassing Scintilla? This way a
>>> ScintillaObject instance would be a valid Scintilla one -- and bypassing
>>> the type system, the other way around would also be true if the subclass
>>> adds absolutely nothing.
>>
>> That sounds like it would result in complete compatibility with
>> the current release. In C++ subclassing a type requires little code
>> but I'm unsure just what to do in GTK+ and expect it will take a bit
>> more code.

It would require more code than in C++ but would still be very
manageable. Something like that:

/* in header */
#define SCINTILLA_TYPE_WIDGET (scintilla_widget_get_type())
#define SCINTILLA_WIDGET(o) (G_TYPE_CHECK_INSTANCE_CAST((o),
SCINTILLA_TYPE_WIDGET, ScintillaWidget))
#define SCINTILLA_WIDGET_CLASS(k) (G_TYPE_CHECK_CLASS_CAST((k),
SCINTILLA_TYPE_WIDGET, ScintillaWidgetClass))
#define SCINTILLA_IS_WIDGET(o) (G_TYPE_CHECK_INSTANCE_TYPE((o),
SCINTILLA_TYPE_WIDGET))
#define SCINTILLA_IS_WIDGET_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((k),
SCINTILLA_TYPE_WIDGET))
typedef struct _ScintillaWidget { ScintillaObject parent; } ScintillaWidget;
typedef struct _ScintillaWidgetClass { ScintillaClass parent; }
ScintillaWidgetClass;
GType scintilla_widget_get_type(void) G_GNUC_CONST;
GtkWidget *scintilla_widget_new(void); /* optional */

/* in source */
G_DEFINE_TYPE(ScintillaWidget, scintilla_widget, SCINTILLA_TYPE)
static void scintilla_widget_class_init(gpointer klass) {}
static void scintilla_widget_init(gpointer instance) {}
GtkWidget *scintilla_widget_new(void) {
return g_object_new(SCINTILLA_TYPE_WIDGET, NULL);
}


> GtkScintilla is this, plus actual methods, signals, properties, etc like
> the Qt widgets do, rather than the current Win32-API-style interface. [...]

For what is worth, I would love to have that :)

Regards,
Colomban

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-07-30 22:55:31 UTC
Permalink
Matthew Brush:

> I stopped working on GtkScintilla as it's pointless if it's not part of Scintilla proper (like the similar Qt backend is),...

This is a bit strange to me as I don't really see inclusion in core as that important. My impression (without hard data) is that RiverBank's QScintilla is the second most widely used Scintilla platform layer after Win32 in the core.

What benefits are there to inclusion in core? This issue comes up with many open source projects. I'm most familiar with Python and much of the pressure for moving libraries into Python's main distribution appears to come from a desire for ongoing maintenance.

Neil


--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-07-31 02:01:32 UTC
Permalink
On 14-07-30 03:55 PM, Neil Hodgson wrote:
> Matthew Brush:
>
>> I stopped working on GtkScintilla as it's pointless if it's not part of Scintilla proper (like the similar Qt backend is),...
>
> This is a bit strange to me as I don't really see inclusion in core as that important. My impression (without hard data) is that RiverBank's QScintilla is the second most widely used Scintilla platform layer after Win32 in the core.
>
> What benefits are there to inclusion in core? This issue comes up with many open source projects. I'm most familiar with Python and much of the pressure for moving libraries into Python's main distribution appears to come from a desire for ongoing maintenance.
>

Just a few points anyway:

1) Make the "default" GTK+ API better for everyone
2) Not have to maintain separate build systems, packaging, distribution
channels, etc.
3) Not be "yet another dependency" for apps, for example SciTE probably
wouldn't even use it since it's not in core, so probably nobody else would.
4) More active contributors than 1 (me) to use, test, debug and improve it.

And generally for all the reasons people contribute improvements to
upstream open source projects instead of forking and maintaining a
separate project that only a tiny subset of upstream users would find,
try, and use, rather than (or actually, in addition to) using upstream.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-02 00:40:40 UTC
Permalink
Matthew Brush:

[Reordering]

> 3) Not be "yet another dependency" for apps, for example SciTE probably wouldn't even use it since it's not in core, so probably nobody else would.

SciTE tries to access Scintilla in a platform-indpendent manner through its ScintillaWindow class. Using the GTK+-specific GtkScintilla code would get in the way of this platform-independence. I suppose you could try to generalise GtkScintilla to be independent of GTK+.

> 1) Make the "default" GTK+ API better for everyone

ScintillaGTK is a flat C-level API. Most will be using Scintilla from an object-oriented language like C++ or Python so would be better served by an object based API. Its unlikely they will want to use GObject.

> And generally for all the reasons people contribute improvements to upstream open source projects instead of forking and maintaining a separate project that only a tiny subset of upstream users would find, try, and use, rather than (or actually, in addition to) using upstream.

Its not a fork, its a wrapper and separately maintained wrappers are quite common in open source.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-08-02 02:48:49 UTC
Permalink
On 14-08-01 05:40 PM, Neil Hodgson wrote:
> Matthew Brush:
>
> [Reordering]
>
>> 3) Not be "yet another dependency" for apps, for example SciTE probably wouldn't even use it since it's not in core, so probably nobody else would.
>
> SciTE tries to access Scintilla in a platform-indpendent manner through its ScintillaWindow class. Using the GTK+-specific GtkScintilla code would get in the way of this platform-independence. I suppose you could try to generalise GtkScintilla to be independent of GTK+.
>

If you generalized wrappers of Scintilla in the core, you wouldn't need
wrappers at all :) Actually just making Scintilla's "internal" API
public would make the situation much better with respect to not being a
type-unsafe flat win32-like API that is more easily mappable to OOP
languages (including C+GObject and all the languages that makes nearly
automatic bindings for).


>> 1) Make the "default" GTK+ API better for everyone
>
> ScintillaGTK is a flat C-level API. Most will be using Scintilla from an object-oriented language like C++ or Python so would be better served by an object based API. Its unlikely they will want to use GObject.
>

The problem *is* the flat API. If it instead followed platform norms
(similar to what is in the qt/ or cocoa/ directories, but for GTK+)
instead of the flat API, it would be automagically bindable to any OOP
languages that GIR supports, plus fairly easily semi-manually bound to
non-GIR stuff (eg. C++ using GTKmm, Vala using vapigen, etc.).

Also, many (most?) GObject/GTK+ using C applications heavily use
GObject's OOP features, despite being in C (see most any GNOME
application's source code, for example). Nobody wants to use a flat API
directly, except maybe win32-API C applications (it's still crazy, but
at least it's "normal" there).

>> And generally for all the reasons people contribute improvements to upstream open source projects instead of forking and maintaining a separate project that only a tiny subset of upstream users would find, try, and use, rather than (or actually, in addition to) using upstream.
>
> Its not a fork, its a wrapper and separately maintained wrappers are quite common in open source.
>

It is a fork. I have to make a copy of your upstream code, put it in my
repo, modify it to make it better on my platform, and then force users
to choose to use mine or yours. The situation would be greatly improved
if Scintilla "shipped" as a shared library that GtkScintila could link
to and extend, but a good upstream GTK+ widget would be even better, IMO.

FWIW, the intro to the Scintilla documentation sums up the problem
pretty well:

> The Windows version of Scintilla is a Windows Control. As such, its primary programming interface is through Windows messages.[SNIP]
>
> The GTK+ version also uses messages in a similar way to the Windows version. This is different to normal GTK+ practice but made it easier to implement rapidly.
>
> Scintilla also builds with Cocoa on OS X and with Qt, and follows the conventions of those platforms.

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-03 08:24:53 UTC
Permalink
Matthew Brush:

> If you generalized wrappers of Scintilla in the core, you wouldn't need wrappers at all :)

Not really seeing what this would look like.

> Actually just making Scintilla's "internal" API public would make the situation much better with respect to not being a type-unsafe flat win32-like API that is more easily mappable to OOP languages (including C+GObject and all the languages that makes nearly automatic bindings for).

Exposing the internal arrangement of classes inside Scintilla would be brittle. Clients that worked with 3.4.4 would probably not work with 3.4.5 due to the class refactoring.

> The problem *is* the flat API. If it instead followed platform norms (similar to what is in the qt/ or cocoa/ directories,
> but for GTK+)

The current GTK+ and Cocoa implementations tie into the platform at approximately the same level, as a basic system widget that can be sent messages. There are some more internals exposed on Cocoa and there are a few extra methods but its not really all that different.

> It is a fork. I have to make a copy of your upstream code, put it in my repo, modify it

Why?

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-08-03 10:24:40 UTC
Permalink
On 14-08-03 01:24 AM, Neil Hodgson wrote:
> Matthew Brush:
>
>> If you generalized wrappers of Scintilla in the core, you wouldn't need wrappers at all :)
>
> Not really seeing what this would look like.
>

I just meant that the "internal" API is roughly what I'd expect from a
decent editor widget (ie. awesome). If you could use that API directly
from an application, you wouldn't really need wrappers, although
obviously it's nice to have the platform-specific integration that is
found on non-GTK platforms currently.

>> Actually just making Scintilla's "internal" API public would make the situation much better with respect to not being a type-unsafe flat win32-like API that is more easily mappable to OOP languages (including C+GObject and all the languages that makes nearly automatic bindings for).
>
> Exposing the internal arrangement of classes inside Scintilla would be brittle. Clients that worked with 3.4.4 would probably not work with 3.4.5 due to the class refactoring.
>

GtkSourceView, for example, has kind of a similar API as Scintilla's
internal API, except it's public and they just don't break it. Obviously
it requires more work/planning but the result is a more robust API for
application code to use. For example, with Scintilla's current GTK+
platform, it's impossible to create a document/model independent of the
widget/view because of the flat API, all messages are sent to a view only.

>> The problem *is* the flat API. If it instead followed platform norms (similar to what is in the qt/ or cocoa/ directories,
>> but for GTK+)
>
> The current GTK+ and Cocoa implementations tie into the platform at approximately the same level, as a basic system widget that can be sent messages. There are some more internals exposed on Cocoa and there are a few extra methods but its not really all that different.
>
>> It is a fork. I have to make a copy of your upstream code, put it in my repo, modify it
>
> Why?
>

Because upstream Scintilla doesn't "ship" a shared library one can link
to, it requires any project using Scintilla to fork it. See, for example:

https://github.com/wxWidgets/wxWidgets/tree/master/src/stc/scintilla
https://github.com/geany/geany/tree/master/scintilla
http://svn.tuxfamily.org/viewvc.cgi/notepadplus_repository/trunk/scintilla/
https://git.gnome.org/browse/anjuta-extras/tree/plugins/scintilla/scintilla
https://github.com/codebrainz/GtkScintilla/tree/master/scintilla

Basically any project that uses Scintilla has to fork its source (in the
technical sense) and continually keep it synced up, potentially with
various patches that need to be applied, each release Scintilla makes,
or else let it drift out of sync with upstream. Any given distro has as
many duplicate copies of Scintilla as it has Scintilla-using applications.

And then to make it usable in GTK+, each project is has to write their
own wrapper to avoid using the Win32-like API, for example:

https://github.com/geany/geany/blob/master/src/sciwrappers.h
https://git.gnome.org/browse/anjuta-extras/tree/plugins/scintilla/text_editor.h

Cheers,
Matthew Brush


--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-03 21:58:16 UTC
Permalink
Matthew Brush:

> I just meant that the "internal" API is roughly what I'd expect from a decent editor widget (ie. awesome).

Can you point to this '"internal" API' in the code because I can't see what you are talking about.

> For example, with Scintilla's current GTK+ platform, it's impossible to create a document/model independent of the widget/view because of the flat API, all messages are sent to a view only.

Its possible but requires using extra hidden view objects.

> Because upstream Scintilla doesn't "ship" a shared library one can link to,

If you want to ship a Scintilla shared library then do so. I do not think it is a good idea and do not want the work.

> it requires any project using Scintilla to fork it. See, for example:
>
> https://github.com/wxWidgets/wxWidgets/tree/master/src/stc/scintilla
> https://github.com/geany/geany/tree/master/scintilla
> http://svn.tuxfamily.org/viewvc.cgi/notepadplus_repository/trunk/scintilla/
> https://git.gnome.org/browse/anjuta-extras/tree/plugins/scintilla/scintilla
> https://github.com/codebrainz/GtkScintilla/tree/master/scintilla
>
> Basically any project that uses Scintilla has to fork its source (in the technical sense)

That is not a fork. When you said "fork", I thought you meant a real fork. Not just taking a static library and wrapping it.

> and continually keep it synced up, potentially with various patches that need to be applied, each release Scintilla makes, or else let it drift out of sync with upstream. Any given distro has as many duplicate copies of Scintilla as it has Scintilla-using applications.

Which is great. Applications should ship with the version of Scintilla they are tested against. Not one distributed by the system.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-08-04 06:41:26 UTC
Permalink
On 14-08-03 02:58 PM, Neil Hodgson wrote:
> Matthew Brush:
>
>> I just meant that the "internal" API is roughly what I'd expect from a decent editor widget (ie. awesome).
>
> Can you point to this '"internal" API' in the code because I can't see what you are talking about.
>

I'm certainly not expert in Scintilla's code, but for a couple main
examples:

http://sourceforge.net/p/scintilla/code/ci/default/tree/src/Editor.h#l154

http://sourceforge.net/p/scintilla/code/ci/default/tree/src/Document.h#l193

And related classes. Like how you can use the doc watcher to get signals
on a document, rather than through a view, etc. I was pretty impressed
when I studied the Qt platforms wrapper compared to the one for GTK+.

>> For example, with Scintilla's current GTK+ platform, it's impossible to create a document/model independent of the widget/view because of the flat API, all messages are sent to a view only.
>
> Its possible but requires using extra hidden view objects.
>

But only because the GTK+ API lacks abilities, compared to the Qt
platform that exposes the Document properly:

http://sourceforge.net/p/scintilla/code/ci/default/tree/qt/ScintillaEdit/ScintillaDocument.h

>> Because upstream Scintilla doesn't "ship" a shared library one can link to,
>
> If you want to ship a Scintilla shared library then do so. I do not think it is a good idea and do not want the work.
>

It's how Linux libraries are shipped[0][1], for better or worse. For
more details, see:

https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
https://www.debian.org/doc/debian-policy/ch-source.html#s-embeddedfiles

If no shared lib is provided, there is no way for any project to extend
Scintilla, rather each project is doomed to re-distribute its own "fork"
that is incompatible with upstream and duplicates N other programs that
use it.

>> it requires any project using Scintilla to fork it. See, for example:
>>
>> https://github.com/wxWidgets/wxWidgets/tree/master/src/stc/scintilla
>> https://github.com/geany/geany/tree/master/scintilla
>> http://svn.tuxfamily.org/viewvc.cgi/notepadplus_repository/trunk/scintilla/
>> https://git.gnome.org/browse/anjuta-extras/tree/plugins/scintilla/scintilla
>> https://github.com/codebrainz/GtkScintilla/tree/master/scintilla
>>
>> Basically any project that uses Scintilla has to fork its source (in the technical sense)
>
> That is not a fork. When you said "fork", I thought you meant a real fork. Not just taking a static library and wrapping it.
>

Sorry, I meant "fork" like "independent, incompatible copy of the source
code from upstream, potentially with patches applied, distributed
independently from upstream". It's not a fork in spirit.

>> and continually keep it synced up, potentially with various patches that need to be applied, each release Scintilla makes, or else let it drift out of sync with upstream. Any given distro has as many duplicate copies of Scintilla as it has Scintilla-using applications.
>
> Which is great. Applications should ship with the version of Scintilla they are tested against. Not one distributed by the system.
>

Looking at it the other way, applications could test against the
versions that various distros that they support provide, like they do
with nearly every single other library (ex. GTK+, Qt, etc).

Cheers,
Matthew Brush

[0]: http://sourceforge.net/p/scintilla/feature-requests/555/
[1]:
https://lists.fedoraproject.org/pipermail/packaging/2009-July/006206.html

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Neil Hodgson
2014-08-04 23:25:37 UTC
Permalink
Matthew Brush:

> I'm certainly not expert in Scintilla's code, but for a couple main examples:
>
> http://sourceforge.net/p/scintilla/code/ci/default/tree/src/Editor.h#l154
>
> http://sourceforge.net/p/scintilla/code/ci/default/tree/src/Document.h#l193

Exposing all of those internal details as a supported API would lead to breaking compatibility with every release. It is also C++ so can not be consumed easily by other languages.

In general C++ is poorly supported as a source of APIs to be used from different languages. There are just too many possibilities in C++ to be easily converted to other languages. Boost.Python is about as good as it gets and it is still too complex for most - PySide (which is supported by Scintilla) gave up on it.

> And related classes. Like how you can use the doc watcher to get signals on a document, rather than through a view, etc. I was pretty impressed when I studied the Qt platforms wrapper compared to the one for GTK+.

This was driven by the WingWare guys, not me (maybe the things you like in Scintilla are the bits I didn't do ;-) ) and doesn't have the same API stability guarantees as the message API. ScintillaDocument has remained at its original state and hasn't added new and preferred operations like GetRelativePosition/GetCharacterAndWidth. These are provided by the COM-like IDocument/IDocumentWithLineEnd which do have strong compatibility guarantees (which has a cost in explicit version checks) and which would be my preferred way of exposing more APIs.

> But only because the GTK+ API lacks abilities, compared to the Qt platform that exposes the Document properly:
>
> http://sourceforge.net/p/scintilla/code/ci/default/tree/qt/ScintillaEdit/ScintillaDocument.h

I don't see this as 'properly'. It may be preferred by some in some cases.

> It's how Linux libraries are shipped[0][1], for better or worse. For more details, see:
>
> https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
> https://www.debian.org/doc/debian-policy/ch-source.html#s-embeddedfiles

Yes, I have read these and disagree strongly with that point of view. It adds an extra intermediary, the distribution, between vendors and their customers adding latency to distribution. With SciTE a new release can be used immediately from scintilla.org or within a few days via Apple's app store but when you are using a Linux distribution you are using a release that is 3-9 months old or even older in some cases.

> Looking at it the other way, applications could test against the versions that various distros that they support provide, like they do with nearly every single other library (ex. GTK+, Qt, etc).

Unsure whether you are wanting testing of Scintilla against each release of GTK+/Qt which I think would help with a web page showing the results.

Neil

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Matthew Brush
2014-08-05 05:37:13 UTC
Permalink
On 14-08-04 04:25 PM, Neil Hodgson wrote:
> Matthew Brush:
>
>> I'm certainly not expert in Scintilla's code, but for a couple main examples:
>>
>> http://sourceforge.net/p/scintilla/code/ci/default/tree/src/Editor.h#l154
>>
>> http://sourceforge.net/p/scintilla/code/ci/default/tree/src/Document.h#l193
>
> Exposing all of those internal details as a supported API would lead to breaking compatibility with every release. It is also C++ so can not be consumed easily by other languages.
>

Well not all of them, but more than than a single opaque pointer that
receives/sends all messages.

The API only breaks if it's broken, there are techniques to keep it
stable (even the ABI).

> In general C++ is poorly supported as a source of APIs to be used from different languages. There are just too many possibilities in C++ to be easily converted to other languages. Boost.Python is about as good as it gets and it is still too complex for most - PySide (which is supported by Scintilla) gave up on it.
>

Agreed, a C API is much easier to work with.

>> And related classes. Like how you can use the doc watcher to get signals on a document, rather than through a view, etc. I was pretty impressed when I studied the Qt platforms wrapper compared to the one for GTK+.
>
> This was driven by the WingWare guys, not me (maybe the things you like in Scintilla are the bits I didn't do ;-) ) and doesn't have the same API stability guarantees as the message API. ScintillaDocument has remained at its original state and hasn't added new and preferred operations like GetRelativePosition/GetCharacterAndWidth. These are provided by the COM-like IDocument/IDocumentWithLineEnd which do have strong compatibility guarantees (which has a cost in explicit version checks) and which would be my preferred way of exposing more APIs.
>

Yeah, the lack of stability guarantees would be a problem.

>> But only because the GTK+ API lacks abilities, compared to the Qt platform that exposes the Document properly:
>>
>> http://sourceforge.net/p/scintilla/code/ci/default/tree/qt/ScintillaEdit/ScintillaDocument.h
>
> I don't see this as 'properly'. It may be preferred by some in some cases.
>

Without, it makes application code weird and confused, unlike the
backing C++ code which makes a nice separation between the models and
the views. It's possible to hack around it with dummy views and/or
recycling views and stuff, but it's not as clean as the way the Qt
widget exposes, IMO.

>> It's how Linux libraries are shipped[0][1], for better or worse. For more details, see:
>>
>> https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
>> https://www.debian.org/doc/debian-policy/ch-source.html#s-embeddedfiles
>
> Yes, I have read these and disagree strongly with that point of view. It adds an extra intermediary, the distribution, between vendors and their customers adding latency to distribution. With SciTE a new release can be used immediately from scintilla.org or within a few days via Apple's app store but when you are using a Linux distribution you are using a release that is 3-9 months old or even older in some cases.
>

It works pretty good in practice, for libraries that provide API/ABI
guarantees and properly increment the library versions and stuff. At
least it's a lot easier to compile against such a lib (ex. using
pkg-config) than to embed a copy of the whole lib in tree and make it
built with own build system and such.

>> Looking at it the other way, applications could test against the versions that various distros that they support provide, like they do with nearly every single other library (ex. GTK+, Qt, etc).
>
> Unsure whether you are wanting testing of Scintilla against each release of GTK+/Qt which I think would help with a web page showing the results.
>

No I just meant, rather than having each app keep in-sync directly with
upstream, they could instead use typical platform mechanisms to link to
appropriate library versions they are tested against and such. For
example if you wrote an app against GTK+ 2.4 and your build system
checked against the GTK+ version >= 2.4, all the way until the breaking
GTK+ v3, your app would happily link and work (possibly with some
deprecation messages).

Cheers,
Matthew Brush

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Thomas Martitz
2014-07-31 12:31:15 UTC
Permalink
Replying to several points at once

> I want to see a minimal change set that I can easily review. It took too much work to uncover the fact that the current changes are incompatible with client code that looks up Scintilla by name. It would have been even better if the original submission had included this information.

Here's a new patch that should be really minimal in terms of diff-size
that realizes my goal. However I don't believe it's the best from a
technical viewpoint (i.e. it doesn't have the extra header, I still
would prefer ScintillaWidget as the new name).

It still has the registered name to ScintillaObject since this is a hard
requirement for GIR (struct name and registered (runtime) name must match).


>
> And generally for all the reasons people contribute improvements to
> upstream open source projects instead of forking and maintaining a
> separate project that only a tiny subset of upstream users would find,
> try, and use, rather than (or actually, in addition to) using upstream.


I agree with Matthew that GtkScintilla would be really nice to have
upstream so that GTK applications can use a natural interface to
scintilla, and one which is immediately compatible to gobject
introspection. Having it upstream makes a lot of sense, for keeping it
in sync and exposing it automatically to all GTK clients. The old API
doesn't need to be removed (not now, anyway).

> What about having ScintillaObject subclassing Scintilla? This way a
> ScintillaObject instance would be a valid Scintilla one -- and bypassing
> the type system, the other way around would also be true if the subclass
> adds absolutely nothing.

I thought about subclassing too but I see a number of problems.

* ScintillaObject is already there, for the struct name. The subclass
would need a different struct ("struct _ScintillaXXX { struct
ScintillaObject parent; }". I don't think it could use the same one, not
without confusing g-ir-scannler anyway, can it?
* It should have a separate header which doesn't seem to be liked.
* Subclassing certainly requires more boilerplate code, and therefore a
larger diff, than my proposed patch. I think a minimal diff is a goal
for this?
* The subclass provides little value (it's just a hack to enable better
compat) for being an extra (though minor) maintenance burden
* The biggest problem: ScintillaObject would be a Scintilla, but not the
other way around. Objects created with scintilla_new() would fail the
SCINTILLA_IS_OBJECT(o) check. Therefore the two classes could not be
used interchangeably. And you couldn't just cast Scintilla to
ScintillaObject, as any runtime check in the pipeline will uncover it.

Note that I prefer to not have two classes anyway. It's just a hack for
backward compatibility for clients. IMO two
almost-but-not-quite-the-same classes would only confuse these and new
clients. And IMO the backwards compatibility concerns, which boil down
to only the runtime type name problem, are not worse the hassle of
subclassing and stuff.

I thought of another solution to the g_type_from_name() problem.
Considering that scintilla is to be linked statically we could easily
add an override for g_type_from_name(), catching when it's called for
"Scintilla". How's this one? (Yes, it's also a hack.)

That said, I still don't believe that any client is even doing that. I
looked at the major GTK clients, and would look into more if I knew
about them. In my book this should be a non-issue.

Neil, I'm wondering if you even appreciate this work and see
GIR-compatibility as a useful improvement to Scintilla? IMO it'd be a
huge win for Scintilla if clients could readily enable GIR for scintilla
and have python/java script plugins mess with the text component.

If you say a patch that implements the subclassing idea would be
acceptable and the one attached to this mail not, then I would do the
change so please comment on this.

Best regards

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Thomas Martitz
2014-07-23 06:12:43 UTC
Permalink
Hello folks,

the following patch enables g-ir-scanner to correctly parse
ScintillaWidget.h, i.e. so that it creates a .gir file (to be used with
gobject-introspection for automatic language bindings) for scintilla.

This patch changes ScintillaObject to a more GObject-friendly
ScintillaWidget. I provided a compatibility header so that scintilla
clients continue to compile and run fine when they upgrade their copy.
The header should be removed after some deprecation period.

If you want I can also provide the stub program that I created to make
g-ir-scanner do it's job.

I need this to enable python bindings to scintilla automatically via
introspection for the Geany IDE project.

Please comment and consider this for merging.

Best regards

--
You received this message because you are subscribed to the Google Groups "scintilla-interest" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scintilla-interest+***@googlegroups.com.
To post to this group, send email to scintilla-***@googlegroups.com.
Visit this group at http://groups.google.com/group/scintilla-interest.
For more options, visit https://groups.google.com/d/optout.
Loading...