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 668749 - Improved support for named destinations
Improved support for named destinations
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: general
3.2.x
Other Linux
: Normal enhancement
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-01-26 15:01 UTC by Ron Yorston
Modified: 2012-02-05 12:03 UTC
See Also:
GNOME target: ---
GNOME version: 3.1/3.2


Attachments
Add --named-dest command line argument (1.75 KB, patch)
2012-01-26 15:01 UTC, Ron Yorston
committed Details | Review
Add spawn and dbus support for named destinations (3.26 KB, patch)
2012-01-26 15:02 UTC, Ron Yorston
committed Details | Review

Description Ron Yorston 2012-01-26 15:01:39 UTC
Created attachment 206189 [details] [review]
Add --named-dest command line argument

We ship our product documentation as a set of PDF files. To allow for context-sensitive help we use named destinations. They have the advantage over page indices or page labels that they don't change when the documentation is updated.

The attached patches allow named destinations to be used on the command line and in the dbus Reload method in a similar way to the existing support for page indices and page labels.

One thing I'm not happy about in the second patch is that when checking for link destinations the order of comparisons matters. It's possible to determine that a link destination isn't a page label or named destination because the return from ev_link_dest_get_page_label or ev_link_dest_get_named_dest is NULL. However the returned value from ev_link_dest_get_page is zero even if the link isn't a page index. Perhaps the default value of the page property should be -1?
Comment 1 Ron Yorston 2012-01-26 15:02:21 UTC
Created attachment 206190 [details] [review]
Add spawn and dbus support for named destinations
Comment 2 Carlos Garcia Campos 2012-02-05 12:01:07 UTC
Review of attachment 206189 [details] [review]:

Pushed! thanks
Comment 3 Carlos Garcia Campos 2012-02-05 12:03:22 UTC
Review of attachment 206190 [details] [review]:

Pushed the patch using a switch to get page_label, named_dest or page index depending on the dest type. Thank you very much.

::: shell/ev-application.c
@@ +264,3 @@
+			g_string_append_printf (cmd, " --named-dest=%s", named_dest);
+		else if ((page=ev_link_dest_get_page (dest)) != -1)
+			g_string_append_printf (cmd, " --page-index=%d", page+1);

We could simply check the dest type to make sure get_label/get_named_dest will return a valid pointer

@@ +478,3 @@
                                                "page-label",
                                                g_variant_new_string (page_label));
+                } else if ((named_dest=ev_link_dest_get_named_dest (data->dest))) {

Ditto.