pdf: Fix wrong cairo_pdf_outline_flags_t item prefix

Submitted by Kouhei Sutou on Jan. 4, 2017, 2:47 p.m.

Details

Message ID 20170104.234709.2068459964897668542.kou@cozmixng.org
State New
Series "pdf: Fix wrong cairo_pdf_outline_flags_t item prefix"
Headers show

Commit Message

Kouhei Sutou Jan. 4, 2017, 2:47 p.m.
Hi,

cairo_pdf_outline_flags_t is defined as the following:

typedef enum _cairo_pdf_outline_flags {
    CAIRO_BOOKMARK_FLAG_OPEN   = 0x1,
    CAIRO_BOOKMARK_FLAG_BOLD   = 0x2,
    CAIRO_BOOKMARK_FLAG_ITALIC = 0x4,
} cairo_pdf_outline_flags_t;

cairo_pdf_outline_flags_t items use "CAIRO_BOOKMARK_FLAG_"
prefix. Other cairo_*_flags_t (cairo_text_cluster_flags_t)
uses capitalized type name ("CAIRO_TEXT_CLUSTER_FLAG_") as
prefix. How about using the same convention for
cairo_pdf_outline_flags_t?

It seems that the "CAIRO_BOOKMARK_FLAG_" prefix is wrongly
used. cairo_pdf_outline_flags_t was called
cairo_pdf_bookmark_flags_t in the initial implementation:
  https://lists.cairographics.org/archives/cairo/2016-June/027427.html


Thanks,
--
kou

Patch hide | download patch | download mbox

From d138a827fd546da945c85de0beca70804b746b4b Mon Sep 17 00:00:00 2001
From: Kouhei Sutou <kou@clear-code.com>
Date: Wed, 4 Jan 2017 23:38:17 +0900
Subject: [PATCH] pdf: Fix wrong cairo_pdf_outline_flags_t item prefix

---
 src/cairo-pdf-interchange.c |  6 +++---
 src/cairo-pdf.h             | 12 ++++++------
 test/pdf-tagged-text.c      |  7 ++++---
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/cairo-pdf-interchange.c b/src/cairo-pdf-interchange.c
index 18dd4d8a8..1fc8197f8 100644
--- a/src/cairo-pdf-interchange.c
+++ b/src/cairo-pdf-interchange.c
@@ -579,9 +579,9 @@  cairo_pdf_interchange_write_outline (cairo_pdf_surface_t *surface)
 
 	if (outline->flags) {
 	    int flags = 0;
-	    if (outline->flags & CAIRO_BOOKMARK_FLAG_ITALIC)
+	    if (outline->flags & CAIRO_PDF_OUTLINE_FLAG_ITALIC)
 		flags |= 1;
-	    if (outline->flags & CAIRO_BOOKMARK_FLAG_BOLD)
+	    if (outline->flags & CAIRO_PDF_OUTLINE_FLAG_BOLD)
 		flags |= 2;
 	    _cairo_output_stream_printf (surface->output,
 					 "   /F %d\n",
@@ -1338,7 +1338,7 @@  _cairo_pdf_interchange_add_outline (cairo_pdf_surface_t        *surface,
     /* Update Count */
     outline = outline->parent;
     while (outline) {
-	if (outline->flags & CAIRO_BOOKMARK_FLAG_OPEN) {
+	if (outline->flags & CAIRO_PDF_OUTLINE_FLAG_OPEN) {
 	    outline->count++;
 	} else {
 	    outline->count--;
diff --git a/src/cairo-pdf.h b/src/cairo-pdf.h
index 1e46a7c61..6047161e0 100644
--- a/src/cairo-pdf.h
+++ b/src/cairo-pdf.h
@@ -87,9 +87,9 @@  cairo_pdf_surface_set_size (cairo_surface_t	*surface,
 
 /**
  * cairo_pdf_outline_flags_t:
- * @CAIRO_BOOKMARK_FLAG_OPEN: The outline item defaults to open in the PDF viewer (Since 1.16)
- * @CAIRO_BOOKMARK_FLAG_BOLD: The outline item is displayed by the viewer in bold text (Since 1.16)
- * @CAIRO_BOOKMARK_FLAG_ITALIC: The outline item is displayed by the viewer in italic text (Since 1.16)
+ * @CAIRO_PDF_OUTLINE_FLAG_OPEN: The outline item defaults to open in the PDF viewer (Since 1.16)
+ * @CAIRO_PDF_OUTLINE_FLAG_BOLD: The outline item is displayed by the viewer in bold text (Since 1.16)
+ * @CAIRO_PDF_OUTLINE_FLAG_ITALIC: The outline item is displayed by the viewer in italic text (Since 1.16)
  *
  * #cairo_pdf_outline_flags_t is used by the
  * cairo_pdf_surface_add_outline() function specify the attributes of
@@ -99,9 +99,9 @@  cairo_pdf_surface_set_size (cairo_surface_t	*surface,
  * Since: 1.16
  **/
 typedef enum _cairo_pdf_outline_flags {
-    CAIRO_BOOKMARK_FLAG_OPEN   = 0x1,
-    CAIRO_BOOKMARK_FLAG_BOLD   = 0x2,
-    CAIRO_BOOKMARK_FLAG_ITALIC = 0x4,
+    CAIRO_PDF_OUTLINE_FLAG_OPEN   = 0x1,
+    CAIRO_PDF_OUTLINE_FLAG_BOLD   = 0x2,
+    CAIRO_PDF_OUTLINE_FLAG_ITALIC = 0x4,
 } cairo_pdf_outline_flags_t;
 
 #define CAIRO_PDF_OUTLINE_ROOT 0
diff --git a/test/pdf-tagged-text.c b/test/pdf-tagged-text.c
index 14dbad187..74e7968ee 100644
--- a/test/pdf-tagged-text.c
+++ b/test/pdf-tagged-text.c
@@ -244,7 +244,7 @@  draw_section (cairo_surface_t *surface, cairo_t *cr, const struct section *secti
 	cairo_tag_end (cr, CAIRO_TAG_DEST);
 	cairo_tag_end (cr, "H1");
 	y_pos = MARGIN + HEADING_HEIGHT;
-	flags = CAIRO_BOOKMARK_FLAG_BOLD | CAIRO_BOOKMARK_FLAG_OPEN;
+	flags = CAIRO_PDF_OUTLINE_FLAG_BOLD | CAIRO_PDF_OUTLINE_FLAG_OPEN;
 	outline_parents[0] = cairo_pdf_surface_add_outline (surface,
 							    CAIRO_PDF_OUTLINE_ROOT,
 							    section->heading,
@@ -256,7 +256,7 @@  draw_section (cairo_surface_t *surface, cairo_t *cr, const struct section *secti
 	    flags = 0;
 	} else {
 	    cairo_set_font_size(cr, HEADING3_SIZE);
-	    flags = CAIRO_BOOKMARK_FLAG_ITALIC;
+	    flags = CAIRO_PDF_OUTLINE_FLAG_ITALIC;
 	}
 
 	if (y_pos + HEADING_HEIGHT + paragraph_height + MARGIN > PAGE_HEIGHT) {
@@ -335,7 +335,8 @@  create_document (cairo_surface_t *surface, cairo_t *cr)
 
     cairo_pdf_surface_add_outline (surface,
 				   CAIRO_PDF_OUTLINE_ROOT,
-				   "Contents", "TOC", CAIRO_BOOKMARK_FLAG_BOLD);
+				   "Contents", "TOC",
+                                   CAIRO_PDF_OUTLINE_FLAG_BOLD);
 
     cairo_tag_begin (cr, CAIRO_TAG_DEST, "name='TOC'");
     cairo_tag_begin (cr, "TOC", NULL);
-- 
2.11.0


Comments

Bryce Harrington Jan. 27, 2017, 2:02 a.m.
On Wed, Jan 04, 2017 at 11:47:09PM +0900, Kouhei Sutou wrote:
> Hi,
> 
> cairo_pdf_outline_flags_t is defined as the following:
> 
> typedef enum _cairo_pdf_outline_flags {
>     CAIRO_BOOKMARK_FLAG_OPEN   = 0x1,
>     CAIRO_BOOKMARK_FLAG_BOLD   = 0x2,
>     CAIRO_BOOKMARK_FLAG_ITALIC = 0x4,
> } cairo_pdf_outline_flags_t;
> 
> cairo_pdf_outline_flags_t items use "CAIRO_BOOKMARK_FLAG_"
> prefix. Other cairo_*_flags_t (cairo_text_cluster_flags_t)
> uses capitalized type name ("CAIRO_TEXT_CLUSTER_FLAG_") as
> prefix. How about using the same convention for
> cairo_pdf_outline_flags_t?
> 
> It seems that the "CAIRO_BOOKMARK_FLAG_" prefix is wrongly
> used. cairo_pdf_outline_flags_t was called
> cairo_pdf_bookmark_flags_t in the initial implementation:
>   https://lists.cairographics.org/archives/cairo/2016-June/027427.html
> 
> 
> Thanks,
> --
> kou

Yes, this seems like a good observation.  I don't remember from previous
discussions if this was intended, but given that the enums are provided
in the cairo-pdf.h header, prefixing as you suggest would seem more
appropriate.

I don't see a problem with landing this, but I'd be like to hear
Adrian's thoughts.

Acked-by: Bryce Harrington <bryce@osg.samsung.com>

(Btw, when sending patches to the list, standard convention is to
include them inline rather than as attachments.)

> >From d138a827fd546da945c85de0beca70804b746b4b Mon Sep 17 00:00:00 2001
> From: Kouhei Sutou <kou@clear-code.com>
> Date: Wed, 4 Jan 2017 23:38:17 +0900
> Subject: [PATCH] pdf: Fix wrong cairo_pdf_outline_flags_t item prefix
> 
> ---
>  src/cairo-pdf-interchange.c |  6 +++---
>  src/cairo-pdf.h             | 12 ++++++------
>  test/pdf-tagged-text.c      |  7 ++++---
>  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/src/cairo-pdf-interchange.c b/src/cairo-pdf-interchange.c
> index 18dd4d8a8..1fc8197f8 100644
> --- a/src/cairo-pdf-interchange.c
> +++ b/src/cairo-pdf-interchange.c
> @@ -579,9 +579,9 @@ cairo_pdf_interchange_write_outline (cairo_pdf_surface_t *surface)
>  
>  	if (outline->flags) {
>  	    int flags = 0;
> -	    if (outline->flags & CAIRO_BOOKMARK_FLAG_ITALIC)
> +	    if (outline->flags & CAIRO_PDF_OUTLINE_FLAG_ITALIC)
>  		flags |= 1;
> -	    if (outline->flags & CAIRO_BOOKMARK_FLAG_BOLD)
> +	    if (outline->flags & CAIRO_PDF_OUTLINE_FLAG_BOLD)
>  		flags |= 2;
>  	    _cairo_output_stream_printf (surface->output,
>  					 "   /F %d\n",
> @@ -1338,7 +1338,7 @@ _cairo_pdf_interchange_add_outline (cairo_pdf_surface_t        *surface,
>      /* Update Count */
>      outline = outline->parent;
>      while (outline) {
> -	if (outline->flags & CAIRO_BOOKMARK_FLAG_OPEN) {
> +	if (outline->flags & CAIRO_PDF_OUTLINE_FLAG_OPEN) {
>  	    outline->count++;
>  	} else {
>  	    outline->count--;
> diff --git a/src/cairo-pdf.h b/src/cairo-pdf.h
> index 1e46a7c61..6047161e0 100644
> --- a/src/cairo-pdf.h
> +++ b/src/cairo-pdf.h
> @@ -87,9 +87,9 @@ cairo_pdf_surface_set_size (cairo_surface_t	*surface,
>  
>  /**
>   * cairo_pdf_outline_flags_t:
> - * @CAIRO_BOOKMARK_FLAG_OPEN: The outline item defaults to open in the PDF viewer (Since 1.16)
> - * @CAIRO_BOOKMARK_FLAG_BOLD: The outline item is displayed by the viewer in bold text (Since 1.16)
> - * @CAIRO_BOOKMARK_FLAG_ITALIC: The outline item is displayed by the viewer in italic text (Since 1.16)
> + * @CAIRO_PDF_OUTLINE_FLAG_OPEN: The outline item defaults to open in the PDF viewer (Since 1.16)
> + * @CAIRO_PDF_OUTLINE_FLAG_BOLD: The outline item is displayed by the viewer in bold text (Since 1.16)
> + * @CAIRO_PDF_OUTLINE_FLAG_ITALIC: The outline item is displayed by the viewer in italic text (Since 1.16)
>   *
>   * #cairo_pdf_outline_flags_t is used by the
>   * cairo_pdf_surface_add_outline() function specify the attributes of
> @@ -99,9 +99,9 @@ cairo_pdf_surface_set_size (cairo_surface_t	*surface,
>   * Since: 1.16
>   **/
>  typedef enum _cairo_pdf_outline_flags {
> -    CAIRO_BOOKMARK_FLAG_OPEN   = 0x1,
> -    CAIRO_BOOKMARK_FLAG_BOLD   = 0x2,
> -    CAIRO_BOOKMARK_FLAG_ITALIC = 0x4,
> +    CAIRO_PDF_OUTLINE_FLAG_OPEN   = 0x1,
> +    CAIRO_PDF_OUTLINE_FLAG_BOLD   = 0x2,
> +    CAIRO_PDF_OUTLINE_FLAG_ITALIC = 0x4,
>  } cairo_pdf_outline_flags_t;
>  
>  #define CAIRO_PDF_OUTLINE_ROOT 0
> diff --git a/test/pdf-tagged-text.c b/test/pdf-tagged-text.c
> index 14dbad187..74e7968ee 100644
> --- a/test/pdf-tagged-text.c
> +++ b/test/pdf-tagged-text.c
> @@ -244,7 +244,7 @@ draw_section (cairo_surface_t *surface, cairo_t *cr, const struct section *secti
>  	cairo_tag_end (cr, CAIRO_TAG_DEST);
>  	cairo_tag_end (cr, "H1");
>  	y_pos = MARGIN + HEADING_HEIGHT;
> -	flags = CAIRO_BOOKMARK_FLAG_BOLD | CAIRO_BOOKMARK_FLAG_OPEN;
> +	flags = CAIRO_PDF_OUTLINE_FLAG_BOLD | CAIRO_PDF_OUTLINE_FLAG_OPEN;
>  	outline_parents[0] = cairo_pdf_surface_add_outline (surface,
>  							    CAIRO_PDF_OUTLINE_ROOT,
>  							    section->heading,
> @@ -256,7 +256,7 @@ draw_section (cairo_surface_t *surface, cairo_t *cr, const struct section *secti
>  	    flags = 0;
>  	} else {
>  	    cairo_set_font_size(cr, HEADING3_SIZE);
> -	    flags = CAIRO_BOOKMARK_FLAG_ITALIC;
> +	    flags = CAIRO_PDF_OUTLINE_FLAG_ITALIC;
>  	}
>  
>  	if (y_pos + HEADING_HEIGHT + paragraph_height + MARGIN > PAGE_HEIGHT) {
> @@ -335,7 +335,8 @@ create_document (cairo_surface_t *surface, cairo_t *cr)
>  
>      cairo_pdf_surface_add_outline (surface,
>  				   CAIRO_PDF_OUTLINE_ROOT,
> -				   "Contents", "TOC", CAIRO_BOOKMARK_FLAG_BOLD);
> +				   "Contents", "TOC",
> +                                   CAIRO_PDF_OUTLINE_FLAG_BOLD);
>  
>      cairo_tag_begin (cr, CAIRO_TAG_DEST, "name='TOC'");
>      cairo_tag_begin (cr, "TOC", NULL);
> -- 
> 2.11.0
> 

> -- 
> cairo mailing list
> cairo@cairographics.org
> https://lists.cairographics.org/mailman/listinfo/cairo
Kouhei Sutou Jan. 27, 2017, 1:13 p.m.
Hi,

Thanks for replying.

In <20170127020242.GI14762@bryceharrington.org>
  "Re: [cairo] [PATCH] pdf: Fix wrong cairo_pdf_outline_flags_t item prefix" on Thu, 26 Jan 2017 18:02:42 -0800,
  Bryce Harrington <bryce@osg.samsung.com> wrote:

> I don't see a problem with landing this, but I'd be like to hear
> Adrian's thoughts.

Adrian has been merged this patch:
  https://lists.cairographics.org/archives/cairo/2017-January/027828.html
  https://lists.cairographics.org/archives/cairo-commit/2017-January/012991.html

> (Btw, when sending patches to the list, standard convention is to
> include them inline rather than as attachments.)

OK. I'll do.


Thanks,
--
kou