GNOME Bugzilla – Bug 616053
[GSEAL] GtkTextView lacks accessors for hadjustment and vadjustment
Last modified: 2010-04-27 00:31:45 UTC
GtkTextView has two sealed GtkAdjustment members for which there is no accessor function. These accessors are needed in GtkSourceView (which is derived from GtkTextView) so it can detect when the content overflows in order to maintain consistant margins.
Created attachment 158992 [details] [review] Add accessors for GtkTextView adjustments This patch adds gtk_text_view_get_horizontal_adjustment() and gtk_text_view_get_vertical_adjustment() to GtkTextView There are a few things I'm not 100% sure about: * Is it right to call these functions this way ? The members they retrive are called hadjustment / vadjustment and there are static getters called get_hadjustment and get_vadjustment. I don't know if the added functions should be called gtk_text_view_get_hadjustment / gtk_text_view_get_vadjustment instead * Are the GObject introspection annotations ok ? * I used internal getters which create new GtkAdjustments if they are null, is it the right way or should I return text_view->hajustment and text_view->vadjustment directly ?
logic seems fine, but you should use the short names. see also: gtk_scrolled_window_get_hadjustment gtk_scrolled_window_get_vadjustment i don't know much about introspection data and they're doing the boarding call for my flight now :)
btw: if the way that this thing works is that it always internally creates the hadjustment/vadjustment as needed then i recommend removing the language of "create a new one if needed" -- it's really an implementation detail that we don't need to document. if there's no way to observe from the outside that it didn't exist and if creating it has no side effect then just pretend as if it always existed.
Created attachment 158994 [details] [review] Add accessors for GtkTextView adjustments Revised patch with gtk_text_view_get_hadjustment / gtk_text_view_get_vadjustment as the new accessor function names
@desrt: it is indeed the way it works if the sealed members are not considered as accessible. However, the accessors are meant to replace direct access to stuff that could be null, so maybe the change should be documented.
Oh, and have a good flight :)
I concur with Ryan that the language about creating new adjustments should be stripped from the docs. Other than that, looks good.
Created attachment 159352 [details] [review] Updated patch removing internal implementation details from the documentation.
A little comment about documentation (plase, correct me if I'm wrong) I think you only have to document the cases when you have to free the resources, ie, the exceptional cases. The normal case is that you dont' have to free anything.
Created attachment 159374 [details] [review] Updated patch removing mentions to normal memory management behaviors
Comment on attachment 159374 [details] [review] Updated patch removing mentions to normal memory management behaviors Looks fine now.
Comment on attachment 159374 [details] [review] Updated patch removing mentions to normal memory management behaviors commit 5eaff47e85e5a6cb8bd370ce1cccf5fad5cba5fe
This problem has been fixed in the development version. The fix will be available in the next major software release. Thanks a lot Mirsal.