After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 673117 - Wrong definition of TargetList.add_table ()
Wrong definition of TargetList.add_table ()
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Bindings
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2012-03-29 21:34 UTC by Axel FILMORE
Modified: 2012-04-05 10:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fixes Gtk+-2.0 and Gtk+-3.0 Vapi files... (1.34 KB, patch)
2012-03-29 21:35 UTC, Axel FILMORE
needs-work Details | Review
new patch against "gi" files (1.14 KB, patch)
2012-04-05 08:45 UTC, Axel FILMORE
rejected Details | Review
third take :) (1.35 KB, patch)
2012-04-05 10:18 UTC, Axel FILMORE
none Details | Review
gtk+-2.0, gtk+-3.0: Fix TargetList.add_table binding (2.99 KB, patch)
2012-04-05 10:31 UTC, Luca Bruno
none Details | Review

Description Axel FILMORE 2012-03-29 21:34:13 UTC
According to the docs and to the Vapi files, this function is defined as :

public void add_table (TargetEntry[] targets, uint ntargets)

This doesn't compile currently, Vala appends one more size parameter and it fails to compile.

This patch fixes the definition as :

	public class TargetList {
		public void add_table (Gtk.TargetEntry[] targets);
	}

Thanks for you work on the Vala compiler, great language, great project.

;)

Regards.
Comment 1 Axel FILMORE 2012-03-29 21:35:25 UTC
Created attachment 210903 [details] [review]
fixes Gtk+-2.0 and Gtk+-3.0 Vapi files...
Comment 2 Luca Bruno 2012-04-05 07:48:14 UTC
Review of attachment 210903 [details] [review]:

Thanks for the patch. Please note that the gtk+-2.0.vapi and gtk+-3.0.vapi are autogenerated using vapi/packages/gtk+-2.0 and vapi/packages/gtk+-3.0 metadata  respectively. To regenerate both vapi, go into the vapi/ directory and type make gtk+-2.0 && make gtk+-3.0 .
Comment 3 Axel FILMORE 2012-04-05 08:45:03 UTC
Created attachment 211359 [details] [review]
new patch against "gi" files

Thanks for the information, Luca, this is a new patch against ".gi" files, I'm not sure if it's the correct way to do it though, that's my first attempt to fix vapi files.
:)
Comment 4 Luca Bruno 2012-04-05 08:54:37 UTC
Review of attachment 211359 [details] [review]:

Sorry for not providing enough information :-( .gi files must not be manually modified, they are autogenerated too :-) The files to modify are .metadata.
This is the flow: C headers -> autogenerate .gi -> vapigen + .metadata -> autogenerate .vapi
In the .metadata an hidden="1" should be added relatively to that parameter.
Comment 5 Axel FILMORE 2012-04-05 10:01:12 UTC
I reverted my patched vapi files to do a test case, in a real life program, this line doesn't build :

targets.add_table (dnd_targets, dnd_targets.length); 

I've this error :
error: too many arguments to function ‘gtk_target_list_add_table’

This is the generated code :
gtk_target_list_add_table (targets, DESKTOP_dnd_targets, G_N_ELEMENTS (DESKTOP_dnd_targets), (guint) G_N_ELEMENTS (DESKTOP_dnd_targets));

It takes only 3 parameters not 4.

I've done this case :
//--------------------------------------------------------------------
// valac --pkg gtk+-2.0 -C target_list_add_table.vala

private const Gtk.TargetEntry dnd_targets[] = {};

int main (string[] args) {
    Gtk.init (ref args);

    Gtk.TargetList targets = new Gtk.TargetList (dnd_targets);
    targets.add_table (dnd_targets, dnd_targets.length); 

    Gtk.main ();
    return 0;
}
//--------------------------------------------------------------------

It's strange because, this compiles and generates a binary but the generated C code is still wrong :)

I still have the additional parameter :
gtk_target_list_add_table (targets, dnd_targets, G_N_ELEMENTS (dnd_targets), (guint) G_N_ELEMENTS (dnd_targets));

Here is the definition in the metadata :
gtk_target_list_add_table.targets is_array="1"
gtk_target_list_new.targets is_array="1"
gtk_target_list_new.ntargets hidden="1"

Is it "gtk_target_list_add_table.ntargets hidden="1"" that is missing ?

:)
Comment 6 Axel FILMORE 2012-04-05 10:18:02 UTC
Created attachment 211363 [details] [review]
third take :)

This one should be better :-P

(About the test case, it doesn't generate a binary, only the .c file)

:)
Comment 7 Luca Bruno 2012-04-05 10:31:24 UTC
Created attachment 211364 [details] [review]
gtk+-2.0, gtk+-3.0: Fix TargetList.add_table binding

Based on patch by Axel FILMORE.

Fixes bug 673117.

Thanks for taking time to provide the patches.
This is how the patch should look like in the end.
Comment 8 Luca Bruno 2012-04-05 10:33:28 UTC
commit a5d52a8343a1387006d4ed08a4bf5f185970affc
Author: Luca Bruno <lucabru@src.gnome.org>
Date:   Thu Apr 5 12:28:35 2012 +0200

    gtk+-2.0, gtk+-3.0: Fix TargetList.add_table binding
    
    Based on patch by Axel FILMORE.
    
    Fixes bug 673117.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.