GNOME Bugzilla – Bug 682875
EvView.find_changed method is not introspectable
Last modified: 2012-09-09 08:30:24 UTC
In the old bindings, we could use EvView.find_changed(), but the method is not available anymore because is marked as introspectable=0. There are another option to search, or should be fixed in the binding? I am using the python bindings, in the Read activity (Sugar environment)
I have tried the following (without success): [sugar-build libview]$ git diff diff --git a/libview/ev-jobs.c b/libview/ev-jobs.c diff --git a/libview/ev-jobs.c b/libview/ev-jobs.c index 2fe62e1..99ea489 100644 --- a/libview/ev-jobs.c +++ b/libview/ev-jobs.c @@ -1659,6 +1659,12 @@ ev_job_find_has_results (EvJobFind *job) return job->has_results; } +/** + * ev_job_find_get_results + * @job: #EvJobFind instance + * Return value: (element-type EvPage) (transfer none) : GList of results + * Get the results found by a JobFind + */ GList ** ev_job_find_get_results (EvJobFind *job) { diff --git a/libview/ev-view.c b/libview/ev-view.c index 3c390bb..88c4494 100644 --- a/libview/ev-view.c +++ b/libview/ev-view.c @@ -5761,6 +5761,13 @@ jump_to_find_page (EvView *view, EvViewFindDirection direction, gint shift) } } +/** + * ev_view_find_changed + * @view: #EvView instance + * @results: (element-type EvPage) (transfer none) : GList of results + * @page: current page number + * Set the results found by a JobFind + */ void ev_view_find_changed (EvView *view, GList **results, gint page) { In my python code: def find_changed(self, job, page=None): job_results = job.get_results() logging.error('job find results = %s', job_results) self._view.find_changed(job_results, page) In the log, I get: job find results = [None] The code is very similar to the code in evince (shell/ev-window.c): static void ev_window_find_job_updated_cb (EvJobFind *job, gint page, EvWindow *ev_window) { ev_window_update_actions (ev_window); ev_view_find_changed (EV_VIEW (ev_window->priv->view), ev_job_find_get_results (job), page); ev_window_update_find_status_message (ev_window); } I have tried different options in the transfer mode.
Created attachment 222945 [details] [review] Proposed patch
Comment about the patch: Asking in #introspection channel, Tomeu said, is not possible pass as a parameter a array of GList with the bindings. The patch implements another method, passing the JobFind as parameter to be able to be used with introspection. This mean duplication of code, then if you agree, we can replace the old method, and modify the call in shell/ev-window.c
Well... even if we are not guaranteeing api-stability, I think we could just "deprecate" old api that is not introspectable, but keeping it for the time being.
(In reply to comment #3) > Comment about the patch: > > Asking in #introspection channel, Tomeu said, is not possible pass as a > parameter a array of GList with the bindings. > > The patch implements another method, passing the JobFind as parameter to be > able to be used with introspection. This mean duplication of code, then if you > agree, we can replace the old method, and modify the call in shell/ev-window.c Also, could you provide an additional patch that uses the new API in shell/ev-window.c?
Review of attachment 222945 [details] [review]: The previous method should be marked as deprecated, and ev-window should be changed to use the new one. ::: libview/ev-view.c @@ +5777,3 @@ +/** + * ev_view_find_changed_set_job Trailing : missing here. @@ +5780,3 @@ + * @view: #EvView instance + * @job: #EvJobFind + * @page: current page number Why current? This is the page where find has changed. The view already knows which page is the current one. @@ +5784,3 @@ + */ +void +ev_view_find_changed_set_job (EvView *view, EvJobFind *job, gint page) This name is very confusing, we are actually notifying that find job has changed, and find results are now available for the given page. Maybe we could merge ev_view_find_changed() and ev_view_find_search_changed() into something like ev_view_set_find_results() and handle NULL results as a special case. We still have the problem of what passing as find results. Another option would be to have a single method, ev_view_find_started(), for example, that receives the EvFindJob so that the view can connect to the "updated" signal of the job. @@ +5791,3 @@ + jump_to_find_page (view, EV_VIEW_FIND_NEXT, 0); + jump_to_find_result (view); + } Instead of duplicating the code, move it to a helper function and call it from both, the new and the deprecated function.
Created attachment 223323 [details] [review] New patch, addressing comments
What do you think about my proposal of having a method like ev_view_find_started(), for example, that receives the EvFindJob so that the view can connect to the "updated" signal of the job and update the results internally?
(In reply to comment #8) > What do you think about my proposal of having a method like > ev_view_find_started(), for example, that receives the EvFindJob so that the > view can connect to the "updated" signal of the job and update the results > internally? Honestly, I prefer keep the changes minimal. I don't program in c usually, then the probabilities of breaking something increase with the patch size :) And reading the code I have more doubts: In that case, should I connect "updated" or "finished" signal? What to do with the old method? Of course, if you think is better change this, is ok with me.
Created attachment 223683 [details] [review] New patch, implementing proposal in comment 8
(In reply to comment #8) > What do you think about my proposal of having a method like > ev_view_find_started(), for example, that receives the EvFindJob so that the > view can connect to the "updated" signal of the job and update the results > internally? Today I feel more confident :) I have implemented your proposal (I hope). Tested it in evince, and with python bindings.
Review of attachment 223683 [details] [review]: Pushed to git master, thanks!