[Poppler-bugs] [Bug 38586] support text rotation

Thu Jul 28 11:13:33 PDT 2011

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

--- Comment #18 from Joshua Richardson <joshuarbox-junk1 at yahoo.com> 2011-07-28 11:13:33 PDT ---
(In reply to comment #17)
> Some comments now that i have more time.
> 
> Please do not declare "namespace poppler" in popplerUtils.h since that is a
> namespace we reserve for the poppler cpp frontend.
> Please do not use "using namespace ;" constructs since we do not use them
> anywhere in the utils code.

Thanks for taking the time, Albert!

I think namespacing in general is a good practice, but I will go ahead and take
it out and supply another patch.

> 
> Please do not redefine the DEBUG macro or explain why you need to do it

Sorry, Albert, I am unable to find another definition of this macro.  Can you
point me to the relevant file where it is previously defined?

> I'd prefer you could remove the use of  
> static GooString *gstr_buff0 = NULL; // a workspace in which I format strings
> since makes me a bit worried

I have removed it in the "font extraction" patches, because there I extended
the functionality of GooString.

> 
> Also in 
> -#define DOCTYPE "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01
> Transitional//EN\">"
> -#define DOCTYPE_FRAMES "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01
> Frameset//EN\"\n\"http://www.w3.org/TR/html4/frameset.dtd\">"
> +//#define DOCTYPE "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01
> Transitional//EN\">"
> +//#define DOCTYPE_FRAMES "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01
> Frameset//EN\"\n\"http://www.w3.org/TR/html4/frameset.dtd\">"
> +#define DOCTYPE "<!DOCTYPE html>"
> +#define DOCTYPE_FRAMES "<!DOCTYPE html>"
> If we are not going to need the old definitions, please just delete them
> instead of commenting them

No problem.  Will be included in the next patch.

> 
> Also I'll need some pdf file showing the improvement (like the ones you used
> for your screen captures)

I have a test PDF I think I can upload without a legal issue, since it's just
an excerpt.  But just to be sure, I'll send it to you via email.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.