[Poppler-bugs] [Bug 36653] Missing functions for setting document's properties

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=36653

--- Comment #18 from Carlos Garcia Campos <carlosgc at gnome.org> ---
Comment on attachment 107369
  --> https://bugs.freedesktop.org/attachment.cgi?id=107369
core document info dict API rev 2

Review of attachment 107369:
-----------------------------------------------------------------

::: glib/poppler-document.cc
@@ +778,5 @@
>  }
>  
> +gchar *
> +poppler_document_info_get_string (PopplerDocument *document,
> +                                  const gchar *key)

Instead of adding API to extend the info dict with new metadata, I would start
adding support for modifying the well known entries, like Rodrigo's patch did.
I think we could split this patch, first adding the core part and the glib API,
so that tthe glib api doesn't block the core part that could be used by qt.

::: poppler/PDFDoc.cc
@@ +538,5 @@
>  
> +// Set/get doc info string entries
> +GooString *PDFDoc::getDocInfoStringEntry(const char *key)
> +{
> +  Object info_obj;

I agree we are not always consistent, but in the core we usually prefer camel
case instead of underscores, so this would be infoObj, instead of info_obj, and
the same for all other cases.

@@ +546,5 @@
> +  if (info_obj.isDict()) {
> +    Object result_obj;
> +
> +    if (info_obj.dictLookup(key, &result_obj, 0)->isString()) {
> +      result = new GooString(result_obj.getString());

You can do result = resultObj.takeString() to avoid a memory allocation.

@@ +571,5 @@
> +
> +GBool PDFDoc::setDocInfoStringEntry(const char *key, GooString *value)
> +{
> +  Object info_obj;
> +  GBool success = gFalse;

I'm not sure this could fail, the getter can fail because the entry might not
be present, but here we are setting a new entry, how can that fail?

@@ +578,5 @@
> +
> +  if (info_obj.isDict()) {
> +    Object goo_str_obj;
> +
> +    if (value) {

The spec says we shouldn't store entry values, but omit the entries, so we
should also check that the string is not empty when not NULL.

@@ +593,5 @@
> +
> +    info_obj.dictSet(key, &goo_str_obj);
> +    setInfoModified(this, &info_obj);
> +    success = gTrue;
> +  }

If the document doesn't have an info dict we should create one instead of
failing.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20151218/a151c95a/attachment.html>