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.