Closed badcel closed 3 years ago
You are right. In the beginning of this project I did not implement the (full) generation of structs as they were not very important to get things working. Primarily I'm missing the creation of struct fields, which i added manually. But afterwards I added convenience functions which do not belong into the wrapper.
We should not have any custom code in the Sys
namespace but just allow access to the C world. Custom code should only be in the Core
namespace, meaning we should just create their Core
counterparts.
This does clean up our project internals and alligns to the rest of our work. ~I think in this way we can solve our ambiguities, too.~
I'm going to add an additional TODO point into #54.
As we have decided to switch to a 3layer design in #71
We need to decide how the 2nd layer should look and which features are to be implemented.
In the 2nd layer we want to wrap GTK closely with as few differences as possible. To get this working we have to fix naming issues.
My suggestion would be:
On
for events (e.g. OnAdd
)Prop
for properties (e.g. Label.LabelProp
). As we follow a 3 layer design I fully agree to @firox263 that in this case the 2 lower layers must be autogenerated so we have no problems following up in the 3rd layer if s.th. changes. This means methods would be autogenerated:
But all other parameters will stay as is as we can't make any assumptuns regarding them. (At least I don't know any other rules we could use to interpret some parameters.)
Regarding properties and events I would suggest that we just use what we already have via @na2axl design and support INotifyPropertyChanged
.
That would be it. All the rest would be a higher layer (if needed)
@badcel is the Toolkit layer will be implemented in this repo ? Also the current architecture of the Wrapper layer is strange for me. For example to access enums we have to enter the Sys
namespace... If possible it will be better to have enums and other types in the Core layer, and limit the Wrapper layer to only those raw C functions called from C#.
As we follow a 3 layer design I fully agree to @firox263 that in this case the 2 lower layers must be autogenerated so we have no problems following up in the 3rd layer if s.th. changes. This means methods would be autogenerated:
To clarify, in my proposal, the bottom (wrapper) layer would be fully autogenerated, and the 2nd (core) layer would be a mixture of autogeneration and manually written code. We'd autogenerate whatever we can do safely and particularly parts with a high chance of human error (so inheritance chain, type descriptors, properties, and signals I would imagine), and write code for certain things we cannot infer from gir (so class ABI structures for virtual functions, glue code for composite templates in Widget.cs
and Builder.cs
, possibly constructors?).
We'd still have control over what the classes do, just we'd write them in a way that makes mapping GObject to C# easiest and not worry as much about C# features. So @na2axl's Properties and Signals would absolutely be a part of it, as well as INPC, and a user could very well use the library directly if they wanted to.
Of course, the toolkit gives the user a better, more C# like abstraction on top of it, so we'd encourage users to use this. The 'Toolkit' level for me is not so much a 'layer' on top of Gtk, but rather something that sits half on top/half side by side with Gtk. So while a user could use Gtk.Image
directly, perhaps they'd prefer Toolkit.FileImage : Gtk.Image
which acts similarly to FileImage.cs
at the moment - as a high(er) level abstraction with a clean C# interface.
Since the user will always have access to the underlying Gtk
API, we can go crazy with how we design the toolkit as the dependency on Gtk is one-way. The best part is we can use subclassing to achieve this, while keeping our toolkit widgets compatible with Gtk API with no work necessary on our part 😀
I think of it as the toolkit provides 80% of most common functionality, and if you need something more specific, then you can just go one step lower into using Gtk
directly, and since our toolkit is built on top of the same library, it all works together. That way, users get MVVM and all of the nice C# features (which we'll implement in the toolkit layer), and if they find that something doesn't meet their requirements, they can simply create their own subclass to add it.
That would be it. All the rest would be a higher layer (if needed)
Yep, this sounds good to me 👍
For example to access enums we have to enter the Sys namespace... If possible it will be better to have enums and other types in the Core layer, and limit the Wrapper layer to only those raw C functions called from C#.
We'll definitely need to do this. I think we should avoid 'leaking' the Wrapper layer at all costs.
@badcel is the Toolkit layer will be implemented in this repo ?
Eventually it probably makes sense to have the toolkit in a separate repo (and use e.g. nuget packages), but right now it might be easier having it here. I don't know. What do you guys think?
@firox263 : Yes, we need to create some custom code inside the core layer (especially in the GObject dll). But I believe that we should be able to generate almost everything for other libraries sitting on top of GObject. At least the generated code for those libraries should work as expected to allow 3rd party devs to easily get part of our stack. If there are special cases like Gtk.Builder
there is additional work needed for sure (exceptions prove the rule).
@badcel is the Toolkit layer will be implemented in this repo ?
Eventually it probably makes sense to have the toolkit in a separate repo (and use e.g. nuget packages), but right now it might be easier having it here. I don't know. What do you guys think?
To start with we could keep the code in this repository, as probably our core layer won't be stable any time soon. Currently I would suggest that we focus our energy to get the core layer right in the first place. If we know where we are heading and how the final core layer will look like we can start thinking about the toolkit layer.
For example to access enums we have to enter the Sys namespace... If possible it will be better to have enums and other types in the Core layer, and limit the Wrapper layer to only those raw C functions called from C#.
We'll definitely need to do this. I think we should avoid 'leaking' the Wrapper layer at all costs.
I agree on this. But we have a bit of a problem as the methods in the wrapper layer depend on the enums and structs. If we move the enums and structs into the core layer we could generate the methods in another partial file of the same class in the core layer e.g. Button.NativeMethods.cs
:
[DllImport("libgtk-3.so.0", EntryPoint = "gtk_button_clicked")]
private static extern void clicked(IntPtr button);
In this way we would merge the wrapper and core projects into one, what could actually be a good idea as it would make things easier for us. And because we want to follow GObject/GTK as close as possible with the core layer it is even a good idea if the structs are present in the core because than we can expose them as they are.
I like this aproach as we would be back to 2 layers again as the wrapper would be quite obsolete if it only contains the methods.
Updated suggestion on the core layer:
I agree on this. But we have a bit of a problem as the methods in the wrapper layer depend on the enums and structs. If we move the enums and structs into the core layer we could generate the methods in another partial file of the same class in the core layer
This is exactly what I wanted to suggest. Merging the Wrapper and the Core layer in the same project is a good solution for me 👍🏾
Updated suggestion on the core layer:
Use prefix On for events (e.g. OnAdd) Use suffix Prop for properties (e.g. Label.LabelProp). Keep the class names as they are defined in the gir files. Support INPC Support events / signals via descriptors Merge Wrapper and Core
Most of these sound good to me. The only one I'm not too sure about is suffixing with -Prop
as it seems a bit of a shame to apply this to all properties to avoid only a few clashes. That being said, we do of course need to be consistent. I feel like with Label.Label
it would be entirely reasonable to call the property Label.Text
(this being one of the few times we'd deviate from the official API), the only question is how (or if) this would work with the generator.
I definitely agree with 'flattening' Wrapper and Core into one. Wrapper would be largely redundant I think, so simplifying the project structure seems best.
Okay then lets fix the last remaining point, property names. AppChooserWidget
has the same problem like Label
but with another constellation. But if the property is renamed it works fine. I did not try to generate all other libraries they can have similar issues.
Probably we have to talk about the toolkit layer to get to a decission. Because actually I don't want the toolkit layer to have properties named Prop
. The Toolkit should hide Core completely otherwise we will have an API with overlapping methds and so on. I see 2 possible solutions to this:
When I was testing interfaces with gir.core I had one problem. A button has a click event in the interface it's easy. But if I wan't to access like a Child
property of which type would it be? An interface? because if someone else implements this interface and returns his own implementation of GObject we would fail. If we return the class we would not hide the original core api. Another solution could be to return an interface but split it into 2. One is like IGirContainer
containing the Child
Property and another is IContainer
with the reuseable GOBject independent Added
event or whatever. And IGirContainer would then implement IContainer
.
In my original idea I wanted to base Gir.Core heavily on interface because I don't know any toolkits which are heavily interface based. This is a spot in the coding world which is still available.
One more thing. I can accept "fixing" property names. But in this case I would argument that we should fix everyting else, too. Meaning: Let's be the successor of GtkSharp because this is what GTKSharp tries to do: Provide a convinient Gtk / GObject API for the end user. In this case we would skip the toolkit layer, too.
Edit: I suppose that we can find a somewhat elegant solution to renaming properties, classes... , which integrate nicely with the generator. Perhaps with some fluent code rules which can be easily integrated in the converter?
One more thing. I can accept "fixing" property names. But in this case I would argument that we should fix everyting else, too. Meaning: Let's be the successor of GtkSharp because this is what GTKSharp tries to do: Provide a convinient Gtk / GObject API for the end user. In this case we would skip the toolkit layer, too.
Hmm, you make a good point here.
New Idea: Perhaps an approach we could take is something akin to Gtk and libdazzle. Libdazzle is a library which implements a lot of useful features that are considered "out-of-scope" for Gtk itself, and is like a companion library. So we could make Gir.Core wrap Gtk reasonably closely, but with all the necessary functionality to then build a higher-level library on top. I hope this example illustrates my thinking:
using Gtk;
using Toolkit;
class Program
{
...
void OnApplicationActivate()
{
// Where Toolkit.Window is a subclass of
// Gtk.Window which implements developer friendly
// C# features (e.g. native MVVM, etc)
var window = new Toolkit.Window()
{
// Mix of Gtk and Toolkit properties
Title = "My Window"
SomeToolkitProperty = 5
};
// Add a "Flow Box" (some nice, high level container
// implemented in our toolkit).
var box = new Toolkit.FlowBox();
window.Add(box);
// We can create an image widget from a path
// to a file. FileImage improves upon the Gtk
// Image API and makes it better for C# devs
var img1 = new Toolkit.FileImage(pathToFile);
box.Add(img1);
// But we could just as well create a regular
// Gtk.Image should the user need some obscure feature
var img2 = new Gtk.Image("some-image-id");
box.Add(img2);
// Toolkit.Window ultimately is still a Gtk.Window
// we just improve upon the API by wrapping it in
// a more specific, better suited for C# type.
window.ShowAll();
}
}
My thinking is the above approach would satisfy both the need to have a relatively Gtk-faithful API (which is important for me in regards to translating certain Gtk paradigms/patterns to C# and keeping the bindings comprehensive but maintainable) while still allowing us complete freedom to experiment with the high level API design (Toolkit-layer), as there is only a one-way dependency (Toolkit -> Gtk).
I feel like if we force the Gtk library to be both a wrapper to Gtk and a fully integrated C# toolkit, we give the user the worst of both worlds. Here, it is like we are giving them all the building blocks to make their own application; if they want nice high level widgets, they can use Toolkit
, but if they need some advanced feature, rather than having to modify the toolkit library, they can just take one step lower and use Gtk
directly. This way, each library is doing one thing and doing it well:
Conclusion Regardless of whether we go with the above idea or not, I think going for a stable, straightforward, easy-to-maintain wrapper around Gtk (sort of like GtkSharp, but perhaps more akin to what we have now - slightly more integrated in C#) is our best option.
Therefore, with the one-way dependency on Gtk, if we decide our toolkit is better served as a complete wrapper/companion library/whatever, we are able to change that without impacting the bindings themselves. GirCore would be the foundation for both our toolkit, and other toolkits if they want linux GUI support.
Would very much appreciate your thoughts @badcel @na2axl
Because actually I don't want the toolkit layer to have properties named Prop.
Fully agree. I think regardless of what we choose, we should aim for a nice-ish C# API with Gir.Core directly, with the expectation that this can be improved further with our toolkit, whatever we choose. I would very much like to explore 'Reactive Gtk', as it seems quite interesting (thanks @na2axl!) and doing it this way gives us and others flexibility to build on top of a straightforward solid base.
@firox263
Toolkit - Builds on Gtk but provides a more Xamarin.Forms/Avalonia/Reactive UI style experience,
I like this in general as we probably don't want to create yet another high level wrapper around a UI toolkit. And your suggestion brings a lot more features to the user than my "hide it" or "use an interface" aproach. So extending the functionality of our Core sounds very nice.
Regarding your example with the Image-API new Toolkit.FileImage(pathToFile);
:
If the FileImage
would inherit from Gtk.Image
the available properties would still be no good match for an FileImage
because the Name
property of an Gtk.Image
is only relvant for a StockImage
. So I do not see the benefit here. If we add properties / features / ..., this is easy and nice. But if the given Gtk api is suboptimal we can't hide things.
We could define that we will just create a library that builds on top of Gtk. Then we would by definition do not care about bad api design of Gtk. This would be fine for me as it probably won't bother most users. Or do we improve on this already in the core layer? What do you think regarding this (@na2axl, @firox263 )?
The main focus of the toolkit would be to allow MVVM / reactive programming styles based on the original API what I quite like.
The other big bullet point for this would be: How to make the core layer convenient? If our toolkit does inherit from the core, the core already needs to be convenient, meaning we have all the naming issues to solve. To get your (@na2axl + @firox263 ) opinions, please answer the following questions (for me it would be 3 times yes):
Label.Label
to Label.Text
to avoid s.th. like Label.LabelProp
?Edit, we could even generate a bit more:
Updated suggestion on the core layer:
On
e.g. OnAdd
(autogenerated)private static
wrapper methods (this would support the async/await feature in our core layer, yay!)Edit2:
I just started some work on merging core + wrapper here (only GLib): https://github.com/gircore/gir.core/tree/GenerateV2
While doing this I saw that there are lots of delegates and global functions (functions independent of a class), which probably have to be public.
@na2axl: @firox263 and me just had a talk and discussed how to continue. We want to try to wrap GTK closely with the toolkit layer on top. The target is to get the Quicksample working with a limited set of GTK features. If this is done we declare PR #52 as done.
TODO:
Afterwards we can evaluate how to improve the current generation process and create new draft pull requests for the CustomObject branch until we are in a state that can be merged into develop.
Do you agree with this?
Some code snippets:
Method Wrapping:
public Widget GetChild() => WrapPointerAs<Widget>(gtk_widget_get_child(Handle));
Properties:
// Class.Generated.cs
const string LABEL_PROPERTY = "label";
// Class.cs
var TextProperty = Property.Wrap(LABEL_PROPERTY, nameof(Text), () => Text, (o) => Text = o);
public string Text
{
get => GetProperty(TextProperty);
set => SetProperty(TextProperty, value);
}
I saw an issue for MAUI which want's to add support for IObserveable
from reactive extensions (https://github.com/dotnet/reactive). Perhaps we should evaluate if / how we could integrate with this kind of programming model. Reactive UI is already doing this for some projects: https://github.com/reactiveui/ReactiveUI
@firox263 : If you want to, you could create some kind of concept for this?
Hi @badcel, @firox263, I'm just coming from the Hermite mode haha. Seems I've skipped a lot of talk...
First of all, I'm ok with the final TODO list. Now about how we can build the Toolkit layer and what we can add in this I agree with what @badcel have said
If the FileImage would inherit from Gtk.Image the available properties would still be no good match for an FileImage because the Name property of an Gtk.Image is only relvant for a StockImage. So I do not see the benefit here. If we add properties / features / ..., this is easy and nice. But if the given Gtk api is suboptimal we can't hide things.
Making the Toolkit layer extend the Core layer can lead us to a bad API design, since every public members available from the Core layer will be also available to the Toolkit layer directly, without any abstraction, but this is exactly why the Toolkit layer exist, hide the Gtk side of the scene to provide to users a more C# exprerience. However, I agree also with what @firox263 tried to show:
// But we could just as well create a regular
// Gtk.Image should the user need some obscure feature
var img2 = new Gtk.Image("some-image-id");
box.Add(img2);
This is a must with our Toolkit layer. We have to let end users able to use some native Gtk widgets, maybe from us, or also from other libraries built with the Core layer.
So for me the Toolkit layer will be a new, fresh and built from scratch layer which will cover, not extend, the Core layer. As example, imagine we have in the Toolkit layer this class:
using Gtk;
// A FlexBox is an abstract class for Row and Column
// All Toolkit widgets implement a kind of IToolkitWidget interface
public class Column : FlexBox
{
// Our native widget is here
private VBox _box;
// Give the possibility to the user to do something dark with the native widget
// Comes from the IToolkitWidget interface
public Widget Native => _box;
// Our custom methods/properties/events written on top of `_box`...
}
This way, we will not directly expose the Gtk API, and also we have the possibility to do real, more fancy things like:
About ⚛️ Reactive Programming, yes this must be added in the Toolkit layer. Using the model described above, this will not be a difficult task.
For the rest of things, I totally agree 👍🏾
If I can suggest something for the final architecture of the project (with the Core and Wrapper layers merged in one project):
*.Generated.cs
or *.Core.cs
: Are the generated files of the Core layer*.Native.cs
or *.Wrapper.cs
: Are the generated files of the Wrapper layer*.cs
: Are hand-crafted files for the Core or Wrapper layersNow if possible I think it will be better if every native functions are generated in the scope they are declared, in a nested class called Native
for example:
// Container.Native.cs
public partial class Container
{
// The class containing native methods about the Container class
internal static class Native
{
// Every gtk_container_* methods goes here
}
}
Like this even us will use a little great C# workflow (eg Container.Native.add(widget)
) when building the Core layer.
@badcel , @firox263 what do you think about ?
@na2axl thank you for your feedback! I like all your suggestions. Here the updated suggestions
TODO:
Core
Container.Native.add(widget)
Toolkit
For me it is not really clear on which layer the reactive support should be implemented. Are we going to stick with regular events in the core layer?
Another thing, we write a lot of things and I'm a bit afraid that we will loose information,because we close issue or pull requests. What do you guys think, should we write the most important code snippets / ideas into a github wiki?
We can run the quickstart sample again, yai :smiley: (see #74). The code has some warnings and can probably be optimized / be cleaner and so on. But for now I think it is okay. As @firox263 is going to generate the events (see #75) this would be another good step.
During the last days I resurrected an idea which could perhaps solve our naming problems. If we prefix all types with the letter "G" we could avoid collisions with properties. The only thing is, that "Object" would match it's namespace "GObject", but this is hopefully not a problem.
What do you guys think? @firox263 @na2axl
Hi @badcel, @firox263, good work with new changes 👍🏾.
During the last days I resurrected an idea which could perhaps solve our naming problems. If we prefix all types with the letter "G" we could avoid collisions with properties. The only thing is, that "Object" would match it's namespace "GObject", but this is hopefully not a problem.
I think since the Core layer will try to be tightly related to the native GTK lib, prefixing classes with a G is not a a bad thing, and yes it will finally resolve the naming issues.
Ah my assumption was incorrect, there are cases where methods and properties have the same name (at least if we make them pascal case, which is not allowed): https://github.com/gircore/gir.core/pull/71#issuecomment-684231264
So the problems would not be completely solved and it is probably not worth doing (because we still can not generate automatically). So perhaps we continue like planned: https://github.com/gircore/gir.core/issues/68#issuecomment-687885057
Personally, I'm not really a fan of the prefixing, since it feels like a large step backwards compared with the existing bindings. As @badcel mentioned, properties and methods can already have the same name, which makes this feel like a very "hacky" fix to a complicated problem.
My preference would be to continue with our current plan, where we auto-generate signals but write properties and methods manually for now, and in the future we can investigate generating more where it becomes feasible. I expect we'll have a much clearer picture of how to go about generation once we've started using the API in practice.
On another note, great to hear that QuickStart is working again. I think we're getting very close to feature partity (and being able to merge into develop 😀).
Another idea regarding property generation: we could generate all properties except those which will fail to generate due to c# naming restrictions.
This would save a lot of work and we can generate the remaining properties manually
@badcel seems I've missed theses comments...
Since the problem is mostly a property-method naming conflict, I think we can generate one and manually write the other, and I prefer to generate properties since it is too much verbose compared to methods which are mostly proxies to native ones....
@na2axl Yeah, but the Label
class is still clashing with the Label
property. So my suggestions would be to check conflicts during generation and omit properties which would later result in a compile error.
So we will completely rename the Label
property ? Or just add a suffix/prefix to it ?
If we will add a prefix/suffix to conflicting properties, I think this can also be generated. But if we completely rename the property I think about a way to generate them too. I'm not too much in the generator part of the project but can we have a kind of dictionary having the list of conflicting properties per types, and their new name ? So in the generator when we find theses properties we can automatically rename them at the generation time. I think if it is possible we will also be able to generate methods using the same way.
Actually I don't like the idea of configuration files, as they need to be written per project and can get quite hard to follow along (see GTKSharp). Generated pre- and suffixes look like a workaround for me from an API standpoint of view.
I would prefer some message during the generation process, which makes clear that not everything was generated due to conflicts. At least for our current projects it will generate probably like 1-2 errors which is more or less negligible.
@badcel At least for our current projects it will generate probably like 1-2 errors which is more or less negligible.
Yes is exactly why I think about the dictionary 😄. I don't mean to rewrite every properties and methods in it, but only ones who are conflicting with others, which are as you said, probably like 1-2 conflicts, so I think there will be no overhead on maintaining them...
Yes I agree it would not be hard for us. But my concern is not our code but third party usage of the generator.
A 3rd party would run the compiler and get errors without a compileable result. The user would need to check the documentation and create some external file which describes how to solve this error and would have to give it to the generator.
I would prefer a clean message and a compileable result in the first try.
How long will it take until one library author requests more features to configure what is happening?
What about libraries with lots of conflicts? I don't want them to maintain a large list of mappings I would always prefer partial code because it is more explicit.
Again my opinion is not set in stone but I worked a little bit with gtk sharp and never understood the files completely and why some things must be in there and some not. I want to avoid this complete configuration struct.
I think we should just generate the things which we can. The rest should be handwritten in partial classes.
@firox263 what do you think?
Hi @badcel, by saying dictionary I don't mean an external file like GTKSharp ones (I hate them too), I mean a real C# Dictionary
property written in the Generator
class.
Generator
is used by us, but can also be used by othersGObjectGenerator
or a custom implementation of Generator<CustomTemplateLoader>
So in the Generator
class we can have a virtual property storing an empty dictionary of mappings, and in concrete implementation we can override it if wanted.
This can now be combined with your behavior, where when the generator find a conflict, and this conflict is not resolved in the mapping dictionary, it just skip the generation of that property/method and output a warning in the console. This will ensure that the generated code will always compile.
I think like that users which are more favorable to write a mapping dictionary and users which are much favorable to write themselves missing properties/methods will be both happy.
@na2axl Your suggestion is good as it combines our suggestions. But I think it is not necessary at the moment:
In general I would like to provide a dotnet tool
which generates the code. The target audience for this tool is our own project and C devs who want to generate bindings for their projects. But if the dotnet tool
needs additional information for the generation process we are instantly using files again (at least I don't have another idea).
To provide the best possible solution for those folks, I would go as far as stripping out our generator in a separate repository (inside the gircore organization) and would use the dotnet tool
provided by the generator project for our own generation.
We would be our own best customer.
In the first step I would want to keep the entry barrier as low as possible:
If we see that we can't get methods working or handwriting those properties is a mess or there are other user needs we can adopt to them. But I would like to start with the smallest and simplest solution and improve on it if we see the need. This could mean that we implement your suggestion next week. But like I said lets start simple for the moment and try out our generator before improving it :heart:.
Before working to much on the generator we should consider a rewrite of the generator. See #100
We landed the new generator.
I think our current arrangement is not intentional as such, but rather because we just built on top of what was already there (initially we had pure ABI structs in
Name.Wrapper
, hence the strange split)In my opinion, we should keep the wrapper entirely auto-generated and move Sys.Value, etc to the Core namespace (we're already doing this with
Sys.Type
->GObject.Type
). This way, we get around any limitations while still keeping the nice separation between unmanaged functions and our 'safe' API.Originally posted by @firox263 in https://github.com/gircore/gir.core/pull/67#issuecomment-683391417