GNOME Bugzilla – Bug 642991
Prevent glade plugin from creating a callback that already exists.
Last modified: 2012-03-14 20:17:41 UTC
Created attachment 181623 [details] [review] Patch to prevent glade plugin from creating a callback that already exists. The attached patch prevents the glade plugin from creating a callback that already exists. Please review it.
Created attachment 181633 [details] [review] Identation fix. The attached patch fixes identation issues with my previous patch. Please review it.
Created attachment 181653 [details] [review] Scrolls to a callback if it is already defined Improved the on_glade_drop callback so that now it scrolls to the callback if it is already defined.
Created attachment 181668 [details] [review] Sets the caret position to the body of a callback The attached patch sets the cursor position to the body of a callback that is created using the glade plugin. It is an small but important usability improvement. Please review it.
Created attachment 181671 [details] [review] Small improvements for callback generation on glade signal drop. The attached patch improves the code for generation of callbacks on glade signal drop. Please review it.
Review of attachment 181623 [details] [review]: OK, looks fine with the indentation fix.
Review of attachment 181653 [details] [review]: Nice addition!
Review of attachment 181671 [details] [review]: Looks good overall, see my small comments below. ::: plugins/language-support-cpp-java/plugin.c @@ +553,2 @@ IAnjutaIterable *iter; + if ((iter = language_support_find_symbol (lang_plugin, split_signal_data[2])) == NULL) I would prefer if the handler temp variable would be kept because split_signal_data[2] doesn't tell you much about what actually happens here. @@ +559,3 @@ { + language_support_add_c_callback (editor, iterator, split_signal_data, + "\n", "\n{\n\n}\n", 4); The offset parameter should be a constant #define'd somewhere. No magic numbers in code please. I haven't tested it but I guess it could be useful to call ianjuta_indenter_indent() on the line where the caret is put because that would make sure the cursor is at the correct indendation. @@ +565,3 @@ { + language_support_add_c_callback (editor, iterator, split_signal_data, + " ", ";\n", 1); dito with the #define for the offset
Created attachment 181739 [details] [review] Code improvements on glade signal drop. The attached patch implements Johannes Schmid's requirements. Please review it.
Review of attachment 181739 [details] [review]: OK, this looks good now.
There is one more thing in C that we could implement. In on_glade_drop_possible() we could check if we are in the global scope. If ianjuta_symbol_manager_get_scope() returns NULL (or a file scope, not entirely sure) we are outside any existing function and dropping is possible. Otherwise, we are inside and dropping won't be a good idea. In addition we could possible check if we are inside a comment but that could be out-of-scope a bit. What would be great would be if you could implement the additions you made for C for Python, too. language-support-python has basically the same concept as C (with some easier things because python is simpler than C). But I could also benefit from setting the cursor postions correctly. Thanks!
It is strange: http://anjuta.sourceforge.net/documents/libanjuta/libanjuta-ianjuta-symbol-manager.html says something about the method: IAnjutaIterable* (*get_scope) (IAnjutaSymbolManager *obj, const gchar* filename, gulong line, IAnjutaSymbolField info_fields, GError **err); But libanjuta/interfaces/ianjuta-symbol-manager.h does no declares it. Was it removed recently?
Looks like it was removed by this commit: http://git.gnome.org/browse/anjuta/commit/?id=ee5b00981f9045ada874baf12996887b4b29cd8c Can it be reverted?
(In reply to comment #10) > There is one more thing in C that we could implement. In > on_glade_drop_possible() we could check if we are in the global scope. If > ianjuta_symbol_manager_get_scope() returns NULL (or a file scope, not entirely > sure) we are outside any existing function and dropping is possible. Otherwise, > we are inside and dropping won't be a good idea. Doesn't seem too complicated, but I'm stuck in https://bugzilla.gnome.org/show_bug.cgi?id=643139
Glade plugin already avoids creating a callback that already exists. The rest of the discussion should go to https://bugzilla.gnome.org/show_bug.cgi?id=643139 . Please close this bug.