[poppler] Releasing RC1 one week later

[Brad Hards]

On Monday 03 March 2008 09:25:10 pm Carlos Garcia Campos wrote:
> I've started to look at the optional content stuff and I've realized
> there are some things  implemented in the qt4 frontend that should be in
> the core, like the order array and radio buttons stuff. I've
> changed/fixed other minor issues. Bradh, could you have a look at these
> patches, please?
>
> http://people.freedesktop.org/~carlosgc/poppler-optcontent.diff
I haven't tried / tested any of them, but I'll still offer my opinion, based 
on initial review.

From 82f3d80caed7a5d42eab4328d44fc9e7afa0c4e4 Mon Sep 17 00:00:00 2001
From: Carlos Garcia Campos <carlosgc at gnome.org>
Date: Sun, 2 Mar 2008 12:07:46 +0100
Subject: [PATCH] Fix some memory leaks and error handling in optional content
This one is OK.  I thought I needed xref, but if you can make it work OK 
without it, I think it is a good change.



From 79ee040726e33a190cfad7293ee97bfed3cc7b74 Mon Sep 17 00:00:00 2001
From: Carlos Garcia Campos <carlosgc at gnome.org>
Date: Sun, 2 Mar 2008 13:07:50 +0100
Subject: [PATCH] Code cleanups in optional content
This one is OK, except for this part:
-  GooString* name() const;
+  GooString* getName() const { return m_name; }
 
-  Ref ref() const;
-  void setRef(const Ref ref);
+  Ref getRef() const { return m_ref; }
+  void setRef(const Ref ref) { m_ref = ref; }
 
-  State state() { return m_state; };
-  void setState(State state) { m_state = state; };
+  State getState() { return m_state; }
+  void setState(State state) { m_state = state; }
The foo() -> getFoo() rename is fine (not standard for Qt, but looks more 
standard for poppler), but please don't put the implementation in the header.



From 758391d3342489c329a1734a18a2736f0743c583 Mon Sep 17 00:00:00 2001
From: Carlos Garcia Campos <carlosgc at gnome.org>
Date: Sun, 2 Mar 2008 19:23:25 +0100
Subject: [PATCH] Return optional content items in getOCGs already ordered 
based on the order array
Don't do this - see my previous discussion about building the tree structure 
twice.



From 16eab98505150042f6bd8041f3bdef99ff880ac7 Mon Sep 17 00:00:00 2001
From: Carlos Garcia Campos <carlosgc at gnome.org>
Date: Sun, 2 Mar 2008 19:43:15 +0100
Subject: [PATCH] Use an array instead of a list for optional content groups
Sorry, I don't understand what you are trying to accomplish with this. Seems 
gratuitous. Can you explain more?



From 157d763c3e0e5432cce4da391fab99d0854a63f5 Mon Sep 17 00:00:00 2001
From: Carlos Garcia Campos <carlosgc at gnome.org>
Date: Sun, 2 Mar 2008 19:52:37 +0100
Subject: [PATCH] Fix memory leaks
I don't see the leaks. Again, can you explain why this change is needed?

Brad