Pavel Sanda <[EMAIL PROTECTED]> writes:

> i tried to incorporate the comments, now sending the resulting patch
> and propose to merge it into trunk.

More comments:

> +     } else if (token == "\\use_hyperref") {
> +             lex >> pdfoptions().use_hyperref;
> +     } else if (token == "\\pdf_title") {
> +             lex >> pdfoptions().title;
> +     } else if (token == "\\pdf_author") {
> +             lex >> pdfoptions().author;

Just a suggestion: in order to keep everything in one place, what
about doing

         } else if (prefixIs(token, "\\pdf_")
                pdfoptions().read(token, lex);

and move the long string of tests to PDFOptions.cpp?

> @@ -1176,6 +1223,12 @@ bool BufferParams::writeLaTeX(odocstream & os, 
> LaTeXFeatures & features,
>       }
>  
>       os << lyxpreamble;
> +
> +     // PDF support. Hypreref manual: "Make sure it comes last of your loaded
> +     // packages, to give it a fighting chance of not being over-written,
> +     // since its job is to redefine many LATEX commands."
> +     pdfoptions().writeLatex(os);
> +
>       return use_babel;
>  }

This one is wrong: you should find a way to add your code to
lyxpreamble, because as it is you break the line counting (for error
parsing) that is done a few lines above. Something like:

odocstringstream pos;
pdfOptions.writeLatex(pos);
os << pos.str();

Alternatively, turn PDFOptions::writeLatex (that should be renamed to
writeLaTeX anyway) into something that returns a docstring.

I am glad somebody finally decided to make this work.

JMarc

Reply via email to