Re: [PATCH] pdf: Fix wrong cairo_pdf_outline_flags_t item prefix

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 <[email protected]>

(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 <[email protected]>
> 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
> [email protected]
> https://lists.cairographics.org/mailman/listinfo/cairo

-- 
cairo mailing list
[email protected]
https://lists.cairographics.org/mailman/listinfo/cairo