With recommended changes. ******** BEGIN PATCH ********* diff --git a/ox-latex.el b/ox-latex.el index 19f055e..f6e5a09 100644 --- a/ox-latex.el +++ b/ox-latex.el @@ -103,7 +103,7 @@ (:latex-class-options "LATEX_CLASS_OPTIONS" nil nil t) (:latex-header "LATEX_HEADER" nil nil newline) (:latex-header-extra "LATEX_HEADER_EXTRA" nil nil newline) - (:latex-hyperref-p nil "texht" org-latex-with-hyperref t) + (:latex-hyperref nil nil org-latex-hyperref-template t) ;; Redefine regular options. (:date "DATE" nil "\\today" t))) @@ -341,10 +341,18 @@ the toc:nil option, not to those generated with #+TOC keyword." :group 'org-export-latex :type 'string) -(defcustom org-latex-with-hyperref t - "Toggle insertion of \\hypersetup{...} in the preamble." +(defcustom org-latex-hyperref-template "\\hypersetup{\n pdfkeywords={%k},\n pdfsubject={%d},\n pdfcreator={%c}}\n" + "Template for hyperref package options. + +Value is a format string, which can contain the following placeholders: + + %k for KEYWORDS line + %d for DESCRIPTION line + %c for CREATOR line + +An empty string disables the setup." :group 'org-export-latex - :type 'boolean) + :type 'string) ;;;; Headline @@ -1118,12 +1126,13 @@ holding export options." ;; Title (format "\\title{%s}\n" title) ;; Hyperref options. - (when (plist-get info :latex-hyperref-p) - (format "\\hypersetup{\n pdfkeywords={%s},\n pdfsubject={%s},\n pdfcreator={%s}}\n" - (or (plist-get info :keywords) "") - (or (plist-get info :description) "") - (if (not (plist-get info :with-creator)) "" - (plist-get info :creator)))) + (format-spec (plist-get info :latex-hyperref) + (format-spec-make + ?k (or (plist-get info :keywords) "") + ?d (or (plist-get info :description)"") + ?c (if (plist-get info :with-creator) + (plist-get info :creator) + ""))) ;; Document start. "\\begin{document}\n\n" ;; Title command. ******** END PATCH ********* On Thu, Feb 20, 2014 at 3:52 PM, Joe Hirn wrote: > I really appreciate your review. Sorry for the pedestrian code submission. > I don't get candid critical feedback on my e-lisp, so it's great to learn > more about its idioms and conventions. > > I'll incorporate your feedback into a new patch. > > > On Thu, Feb 20, 2014 at 2:51 PM, Nicolas Goaziou wrote: > >> Hello, >> >> Joe Hirn writes: >> >> > I was able to test this on my local machine and it seems to work as we >> > discussed. >> > >> > If there are any other changes to the patch you'd like to see, please >> let >> > me know. >> >> Thank you for the patch. Here are a few comments. >> >> > - (:latex-hyperref-p nil "texht" org-latex-with-hyperref t) >> > + (:latex-hyperref-p nil "texht" (if org-latex-hyperref-template t) t) >> >> I think we can drop the "-p" suffix since this is no longer a predicate. >> So the property can be named :latex-hyperref. >> >> Also we can replace "texht" with nil since it doesn't make much sense to >> specify a full template from the OPTIONS line. >> >> Eventually, the default value should be `org-latex-hyperref-template'. >> >> This boils down to the following line: >> >> (:latex-hyperref nil nil org-latex-hyperref-template t) >> >> > -(defcustom org-latex-with-hyperref t >> > - "Toggle insertion of \\hypersetup{...} in the preamble." >> > + >> > +(defcustom org-latex-hyperref-template "\\hypersetup{\n >> > pdfkeywords={%k},\n pdfsubject={%d},\n pdfcreator={%c}}\n" >> > + "The value of \\hyperrefsetup{...} in the preamble. String is a >> > format-spec which accepts keywords for %k (pdfkeywords), %d >> > (pdfdescription) and %c (pdfcreator). Set to nil for no >> \\hyperrefsetup." >> > :group 'org-export-latex >> > - :type 'boolean) >> > + :type 'string) >> >> The first line of the docstring should contain complete sentences only. >> I would say something along the lines: >> >> "Template for hyperref package options. >> >> Value is a format string, which can contain the following placeholders: >> >> %k for KEYWORDS line >> %d for DESCRIPTION line >> %c for CREATOR line >> >> An empty string disables the setup." >> >> Since you specify :type as 'string, it is wrong to expect a nil value in >> the variable. Note that nil is not an absolute necessity. We can allow >> to disable the template with an empty string instead. >> >> > + (format-spec org-latex-hyperref-template >> > + (format-spec-make >> > + ?k (or (plist-get info :keywords) "") >> > + ?d (or (plist-get info :description)"") >> > + ?c (if (plist-get info :with-creator) >> > + (plist-get info :creator) >> > + "")))) >> >> You are not using the :latex-hyperref property. This should be: >> >> (format-spec (plist-get info :latex-hyperref) >> ...) >> >> >> Regards, >> >> -- >> Nicolas Goaziou >> > >