Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project structure (Core / Wrapper) #68

Closed
badcel opened this issue Aug 30, 2020 · 30 comments
Closed

Project structure (Core / Wrapper) #68

badcel opened this issue Aug 30, 2020 · 30 comments

Comments

@badcel
Copy link
Member

badcel commented Aug 30, 2020

I've seen that the way we are separating projects now (Name.Core and Name.Wrapper) give us some limitations (by @na2axl)

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 #67 (comment)

@badcel
Copy link
Member Author

badcel commented Aug 30, 2020

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.

@badcel
Copy link
Member Author

badcel commented Sep 2, 2020

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:

  • 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.

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:

  1. The method signature almost always involves the handle of the object as the first parameter. We can probably hide this in the autogenerated code.
  2. Some methods contain an error pointer, meaning that an exception occured. We can probably catch this in the generated code, too and throw an exception instead.

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)

@na2axl
Copy link
Member

na2axl commented Sep 2, 2020

@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#.

@mjakeman
Copy link
Member

mjakeman commented Sep 2, 2020

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?

@badcel
Copy link
Member Author

badcel commented Sep 2, 2020

@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:

  • 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

@na2axl
Copy link
Member

na2axl commented Sep 2, 2020

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 👍🏾

@mjakeman
Copy link
Member

mjakeman commented Sep 3, 2020

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.

@badcel
Copy link
Member Author

badcel commented Sep 3, 2020

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.

  • Like I wrote before I would like to support easy integration into our stack. Meaning the user runs a generator dotnet tool and he is done. I don't want him to fix things in some configuration file. If the 3rd party C library is working our generator must ensure the endresult is working, too. It is just a bad experience if we generate code which is not working in the first place.
  • Why should we think about renaming things if they are actually the same? Why break the consistency?
  • Probably every C# dev has some kind of intelli sense with code completion. If we use some suffix it won't be harder to code.
  • Why do we want to write special code for corner cases? It is always easier to maintain code if those corner cases just can't exist.
  • We said we don't want to make the Core API especially user friendly. We will have a Toolkit layer for this.

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:

  1. Do not inherit from core. This would require a lot of wrapping on our side.
  2. Do inherit but: The API surface of the toolkit is defined via interfaces. So we expose methods / properties via interfaces (which is very nice for mocking, dependency injection, ...). As we will probably rename some properties and methods we can define them in the interface and just wrap the core member accordingly. If needed we can build upon the core members and create our own logic. A user could in case of need just cast to the class type and get access to the full blown thing.

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.

@badcel
Copy link
Member Author

badcel commented Sep 3, 2020

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?

@mjakeman
Copy link
Member

mjakeman commented Sep 3, 2020

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:

  • Gir.Core - Convenient, partially/mostly hand-written layer that wraps Gtk reasonably closely (but makes exceptions to be a little more C#-like). This would be what we've currently got in Custom GObjects #52, and would be an evolution of GtkSharp so to speak. It provides a first class Gtk experience in C#.
  • Toolkit - Builds on Gtk but provides a more Xamarin.Forms/Avalonia/Reactive UI style experience, where we do not have to worry about exposing all functionality because the user has the agency to choose whether to use the high level abstraction or write their own for their own needs. Provides a first class C# GUI experience, which just so happens to be Gtk based.

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.

@badcel
Copy link
Member Author

badcel commented Sep 3, 2020

@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):

  1. If I understand you right, you do not want to write naming rules to generate the code? (So the generated code would only consist of type hierarchy + type descriptor + private static wrapper methods.)
  2. We would probably generate the lower level methods (currently part of wrapper) and then call them manually in our public core api?
  3. We would manually rename members as necessary eg. 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:

  • Properties are adopted to the naming needs, to provide a convenient api (handwritten)
  • Keep the class names and type hierarchy as they are defined in the gir files. (autogenerated)
  • Support INPC
  • Support events / signals via descriptors, use the prefix On e.g. OnAdd (autogenerated)
  • Merge Wrapper and Core
  • Create methods manually but rely on the 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.

@badcel
Copy link
Member Author

badcel commented Sep 5, 2020

@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:

  • Merge Wrapper and Core
  • Generate global functions in Global.cs
  • properties are adopted to the naming needs, to provide a convenient api (handwritten) (see sample)
  • Keep the class names and type hierarchy as they are defined in the gir files. (autogenerated)
  • Support INPC
  • Support events / signals via descriptors, use the prefix On e.g. OnAdd (autogenerated)
  • Do not generate methods for now. If the work is done we evaluate how to autogenerate methods. (It could be possible to detect async methdos via Gio.AsyncResult parameter)

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);
}

@badcel
Copy link
Member Author

badcel commented Sep 6, 2020

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?

@na2axl
Copy link
Member

na2axl commented Sep 6, 2020

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:

  • Create our mockable set of interfaces like @badcel want, to use in unit tests and IoC,
  • ✨ Use XAML as the template engine of the Toolkit layer using XamlX 💭 (just thinking about, no assertion 🧙🏾‍♂️)
  • And many really fancy things 😃

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 layers

Now 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 ?

@badcel
Copy link
Member Author

badcel commented Sep 6, 2020

@na2axl thank you for your feedback! I like all your suggestions. Here the updated suggestions

TODO:

Core

  • Merge Wrapper and Core
  • Generate global functions in Global.cs
  • properties are adopted to the naming needs, to provide a convenient api (handwritten) (see sample)
  • Keep the class names and type hierarchy as they are defined in the gir files. (autogenerated)
  • Support INPC
  • Support events / signals via descriptors, use the prefix On e.g. OnAdd (autogenerated)
  • Do not generate methods for now. If the work is done we evaluate how to autogenerate methods.
  • Put native methods in a *.Native.cs with an internal class to get the following Syntax: Container.Native.add(widget)
  • Put generated core code in a *.Generated.cs

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?

@badcel
Copy link
Member Author

badcel commented Oct 7, 2020

We can run the quickstart sample again, yai 😃 (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

@na2axl
Copy link
Member

na2axl commented Oct 9, 2020

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.

@badcel
Copy link
Member Author

badcel commented Oct 9, 2020

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): #71 (comment)

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: #68 (comment)

@mjakeman
Copy link
Member

mjakeman commented Oct 10, 2020

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 😀).

@badcel
Copy link
Member Author

badcel commented Oct 11, 2020

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 badcel mentioned this issue Oct 25, 2020
9 tasks
@na2axl
Copy link
Member

na2axl commented Oct 25, 2020

@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....

@badcel
Copy link
Member Author

badcel commented Oct 25, 2020

@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.

@na2axl
Copy link
Member

na2axl commented Oct 25, 2020

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.

@badcel
Copy link
Member Author

badcel commented Oct 25, 2020

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.

@na2axl
Copy link
Member

na2axl commented Oct 25, 2020

@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...

@badcel
Copy link
Member Author

badcel commented Oct 25, 2020

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?

@na2axl
Copy link
Member

na2axl commented Oct 26, 2020

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.

  1. You are right, the Generator is used by us, but can also be used by others
  2. But even when it's used by others, they (may) need to create a new concrete class (maybe) extending GObjectGenerator 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.

@badcel
Copy link
Member Author

badcel commented Oct 26, 2020

@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:

  • No C# knowledge needed
  • No configuration files needed

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 ❤️.

@badcel
Copy link
Member Author

badcel commented Nov 3, 2020

Before working to much on the generator we should consider a rewrite of the generator. See #100

@badcel
Copy link
Member Author

badcel commented May 15, 2021

We landed the new generator.

@badcel badcel closed this as completed May 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants