Skip to content
This repository has been archived by the owner on Oct 1, 2024. It is now read-only.

Add generation of abstract GInterface base class #131

Closed
wants to merge 2 commits into from
Closed

Add generation of abstract GInterface base class #131

wants to merge 2 commits into from

Conversation

riccioclista
Copy link

UPDATE: This PR depends on PR #133. The first two commits (20450a8 and 414e7b1) belong to PR #133 and should be discussed and eventually merged there.

This PR aims to solve the issue that GInterface usage in gtk-sharp is cumbersome. This issue has been discussed thoroughly in PR #113. The solution presented in this PR is in accordance with comment #113 (comment) in PR #113.
In this PR a new additional class is generated for each GInterface. Up until now there were the 3 types IFoo, FooAdapter and IFooImplementor. In addition an abstract class Foo is generated that subclasses GLib.Object and implements IFoo and IFooImplementor. Thus, it is now possible to simply create a custom implementation of IFoo by writing

public class MyFoo : Foo
{
}

To accomplish this, several other changes were necessary (or convenient):

  • The ToolsVersion in csproj files was updated from 3.5 to 4.0.
  • In some places there were custom additions to GInterface interfaces. This made it necessary to extend the new abstract (partial) base classes to account for those custom interface members. (0818e95)
  • To facilitate a consistent interface of IFoo and FooAdapter on one side and IFooImplementor (and subsequentially) Foo on the other side, accessor methods are now handled differently. For details see the commit message of b9f1d58. This change also affected the overrides implementation in GObjectVM (a808599).

All changes in the generated code can be viewed here: https://github.com/antoniusriha/gtk-generated-files-compare/commit/b591d47e88b1286913b9d8b61295aad946ae9812 https://github.com/antoniusriha/gtk-generated-files-compare/commit/4d215f947b21bd3181e35e467d1d0f833e2f963a.

All changes to the API can be viewed here: https://gist.github.com/antoniusriha/532dda0ca6fa9bb1332d. This information has been generated using the tools mono-api-info.exe and mono-api-html.exe from the mono repository. (I've noticed that the member description is not always correct: abstract members are displayed as virtual.)

@knocte
Copy link
Contributor

knocte commented Jun 14, 2015

Hey Antonius! The work you have done here is great.

I will raise some concerns, but don't get put off by them please, as I love that we're having these conversations!

So here they are:

  • The ToolsVersion change seems to address an issue that is not directly related to this GInterface change, maybe it's better to propose it as a separate PR? (And state that this PR would depend on that one.)
  • Looking at https://github.com/antoniusriha/gtk-generated-files-compare/commit/b591d47e88b1286913b9d8b61295aad946ae9812 I see that the change in gdk/Gdk/Keymap.cs
    would be valuable even outside this PR (because it replaces an out argument with a return value). Maybe it's better to propose it as a separate PR?
  • The main motivation for me to originally propose the use of "Foo" classes was to get rid of the "FooAdapter" classes. So, as this PR adds them without getting rid of the FooAdapter ones (for compatibility reasons, I'm guessing), I'm slightly concerned that this would make the API a bit more complex. Shouldn't we mark the "FooAdapter" classes as [Obsolete] at least?

@riccioclista
Copy link
Author

  • ToolsVersion: I can certainly do that as its not necessary for this PR to work (It's only convenient for development, if you're working with newer versions of mono).
  • Replacement of out parameters with return values: I could create a separate PR for that. However, this is a dependency for the main change in this PR.
  • FooAdapter: I'm with you in that I prefer an opinionated API over an API that gives you all the options. However, that is my point of view. Some people might prefer the lower level Adapter/Implementor pattern.

You raised another concern, I didn't think about before: In this PR I'm removing some methods (actually I'm not removing them but changing the signature). However, I just changed the API without marking anything as Obsolete, because this would be quite some effort since it would have to be done in code generating code. Do you or anybody else reading know, how to do this properly? Or can we just do it, because we're on the verge of a major version change?

@riccioclista
Copy link
Author

I've extracted PRs for the ToolsVersion change (PR #132) and the out parameter change: (PR #133) as suggested by @knocte.

Antonius Riha added 2 commits May 10, 2016 11:58
This fixes inconsistent method generation in GIntrface interfaces, adapters and implementor
interfaces. For example, in Atk.IText the method GetRangeExtents existed with two different
signatures. In Atk.IText and Atk.TextAdapter we had:
 (1) Atk.TextRectangle GetRangeExtents(int start_offset, int end_offset, Atk.CoordType coord_type);
whereas the signature in Atk.ITextImplementor was:
 (2) void GetRangeExtents (int start_offset, int end_offset, Atk.CoordType coord_type,
                           out Atk.TextRectangle rect);

This fix ensures that all interface methods have the form as shown in (1). This has been
implemented by applying the same rules for "Accessor"-methods in InterfaceVM as are already in
place in class Method. Essentially these rules state that all methods with one out-parameter are
Accessor-methods.
@directhex directhex closed this Mar 9, 2021
@directhex directhex deleted the branch mono:master March 9, 2021 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants