From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: (V8) [PATCH] New feature: Use dvisvgm to preview latex formular Date: Thu, 19 May 2016 10:22:35 +0200 Message-ID: <87mvnmwj78.fsf@saiph.selenimh> References: <87lh3dxibp.fsf@163.com> <87lh3chenz.fsf@163.com> <8737pjluy5.fsf_-_@163.com> <87posncycg.fsf@163.com> <87inyfuf4d.fsf@saiph.selenimh> <87posmnr7i.fsf_-_@163.com> <87k2itu2h2.fsf@saiph.selenimh> <87mvnnev3u.fsf_-_@163.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:51655) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3JEc-00013J-0d for emacs-orgmode@gnu.org; Thu, 19 May 2016 04:23:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b3JEV-0000yW-Ru for emacs-orgmode@gnu.org; Thu, 19 May 2016 04:23:08 -0400 Received: from relay3-d.mail.gandi.net ([217.70.183.195]:51590) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b3JEV-0000yP-Hk for emacs-orgmode@gnu.org; Thu, 19 May 2016 04:23:03 -0400 In-Reply-To: <87mvnnev3u.fsf_-_@163.com> (Feng Shu's message of "Wed, 18 May 2016 14:30:29 +0800") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Feng Shu Cc: orgmode-devel Hello, "Feng Shu" writes: > I have rebase my patch to new function org-compile-file, the v8 patch is > very different with the earlier version, it is more simpler i think, > please comment again, thanks for your help! Thank you. I added `org-compile-file' to master branch and incorporated some of you suggestions. Beware, however, that there are some differences with the version you're using here. In particular, 1. the order of arguments is different; 2. %b, %f, and %o are always replaced in commands, no need to specify them again when using argument SPEC; 3. extension argument is a string, not a symbol or a list of symbols. By the way, I don't understand why you do need to provide a list of extensions. The extension argument is used to check if the process actually succeeded, but the process itself can create as many files as it sees fit. Some additional comments follow. > + (dvisvgm > + :programs ("latex" "dvisvgm" "gs") > + :message "you needed to install latex, dvisvgm and ghostscript." > + :use-xcolor t > + :image-input (dvi xdv) Per above, it is either dvi or xdv > + "List definitions of external processes for LaTeX previewing. > +Org mode can use some external commands to generate TeX snippet's image for > +previewing or inserting into HTML files, e.g. dvipng, dvisvgm or imagemagick. > +This variable tells `org-create-formula-image' how to use external commands. > + > + :name symbol, the process setting name. > + :inherit symbol, inhert options from an exist process setting. I suggest to drop the property above. It is only partially implemented (e.g., you cannot inherit from an inherited process), it complicates code, and I don't think it is terribly useful overall. > + :image-input symbol, input file type, for example: dvi. > + :image-output symbol, output file type, for example: png. symbol -> string > + :use-xcolor boolean, if set to `t', LaTeX \"xcolor\" macro is used > + to deal with background and foreground color of image, > + if set to `nil', dvipng style background and foregroud color > + format are generated; you should used them in command options > + with special string: \"%F\" and \"%B\". when non-nil, `LaTeX' \"xcolor\"... color of image. Otherwise, dvipng style... are generated. You may then refer to them in command options with "%F" and "%B". > + :image-size-adjust cons of numbers, the car element is used to adjust latex image latex -> LaTeX > + size showed in buffer and the cdr element is for html file. html -> HTML > + This option is only useful for backend developers, users > + should use variable `org-format-latex-options' instead. > + :post-clean list of strings, files matched are to be cleaned up once the > + image is generated. If set to `nil', the files with dvi, xdv, > + pdf, tex, aux, log, svg, png, jpg, jpeg or out extension will > + be cleaned up. > + :latex-header list of string, latex snippet file's header, if set to `nil', latex -> `LaTeX' > + the return value of `org-create-formula--latex-header' will be > + used, which is controled `org-format-latex-header', > + `org-latex-default-packages-alist' and `org-latex-packages-alist'. No need to reference an internal function: the return value is controlled by `org-format-latex-header' `org-latex-default-packages-alist' and `org-latex-packages-alist', which see. > + :latex-compiler list of latex commands, each of them will be given to the shell > + as a command. the special strings, %t, %b and %o, will be replaced > + to according value before commands called. as a command. Place-holders "%t", "%b" and "%o" are replaced with values defined below. > + :image-converter list of image converter command strings, each of them will be > + given to the shell as a command. the following special strings, > + will be replaced to according value before commands called. each of them is given to the shell as a command and support any of the following place-holders defined below. > +Special strings used by `:image-converter' and `:latex-compiler': Place-holders used by... > +5. %t tex file name. Couldn't you use %f so as to be compatible with default place-holders? > -(defun org--format-latex-make-overlay (beg end image) > +(defun org--format-latex-make-overlay (beg end image &optional imagetype) > "Build an overlay between BEG and END using IMAGE file." You need to explain IMAGETYPE in docstring, e.g., Argument IMAGETYPE is the extension of the displayed image, as a symbol. It defaults to "png". > + (let* ((processing-info > + (cdr (assq processing-type org-preview-latex-process-alist))) > + (inherit-processing-info > + (cdr (assq (plist-get processing-info :inherit) > + org-preview-latex-process-alist))) > + (inherit-processing-info > + (if (or (plist-get inherit-processing-info :inherit) > + (eq (plist-get processing-info :inherit) type)) > + (progn (error "'%s' inherit an invalid process setting." processing-type) nil) > + inherit-processing-info)) Again, I suggest to drop the part about inheriting process info. If you think it is a must-have, then it should be factored out of this function and from `org-create-formula-image', as there is too much code duplication. It should also support inheriting from inherited processes. > + (face (face-at-point)) > ;; Get the colors from the face at point. > (fg > (let ((color (plist-get org-format-latex-options > @@ -19172,9 +19271,12 @@ Some of the options can be changed using the variable > org-latex-packages-alist > org-format-latex-options > forbuffer value fg bg)))) > + (imagetype (or (plist-get processing-info :image-output) > + (plist-get inherit-processing-info :image-output) > + 'png)) Ditto. > +(defun org-create-formula-image (string tofile options buffer &optional type) > + "Create an image from LaTeX source using external processes. > + > +The external processes are defined in `org-preview-latex-process-alist'." Ideally, each argument need to be explained in the docstring. I know the previous function didn't do that, but, if you can improve it, that's better. > (require 'ox-latex) > - (let* ((tmpdir (if (featurep 'xemacs) > - (temp-directory) > - temporary-file-directory)) > + (let* ((type (or type 'dvipng)) > + (processing-info > + (cdr (assq type org-preview-latex-process-alist))) > + (inherit-processing-info > + (cdr (assq (plist-get processing-info :inherit) > + org-preview-latex-process-alist))) > + (inherit-processing-info > + (if (or (plist-get inherit-processing-info :inherit) > + (eq (plist-get processing-info :inherit) type)) > + (progn (error "'%s' inherit an invalid process setting." type) nil) > + inherit-processing-info)) > + (programs > + (or (plist-get processing-info :programs) > + (plist-get inherit-processing-info :programs))) > + (error-message > + (or (plist-get processing-info :message) > + (plist-get inherit-processing-info :message))) > + (use-xcolor > + (or (plist-get processing-info :use-xcolor) > + (plist-get inherit-processing-info :use-xcolor))) > + (image-input-types > + (or (plist-get processing-info :image-input) > + (plist-get inherit-processing-info :image-input))) > + (image-output-type > + (or (plist-get processing-info :image-output) > + (plist-get inherit-processing-info :image-output))) > + (post-clean (or (plist-get processing-info :post-clean) > + (plist-get inherit-processing-info :post-clean) > + '(".dvi" ".xdv" ".pdf" ".tex" ".aux" ".log" > + ".svg" ".png" ".jpg" ".jpeg" ".out"))) > + (latex-header (or (plist-get processing-info :latex-header) > + (plist-get inherit-processing-info :latex-header) > + (org-create-formula--latex-header))) > + (latex-compiler > + (or (plist-get processing-info :latex-compiler) > + (plist-get inherit-processing-info :latex-compiler))) > + (image-converter > + (or (plist-get processing-info :image-converter) > + (plist-get inherit-processing-info :image-converter))) This is the repetitive pattern I was talking about. > + (setq image-input-file > + (org-compile-file > + texfile image-input-types latex-compiler log-buf > + (format "Please adjust '%s' part in `org-preview-latex-process-alist'." type) > + `((?t . ,(shell-quote-argument texfile)) > + (?b . ,(shell-quote-argument texfilebase)) > + (?o . ,(shell-quote-argument tmpdir))))) As explained above, beware the order of arguments and the type of IMAGE-OUTPUT-TYPE. Also, I don't think you need to provide SPEC, as it is the default, with a slight change ?t is actually ?f. > + (setq image-output-file > + (org-compile-file > + image-input-file image-output-type image-converter log-buf > + (format "Please adjust '%S' part in `org-preview-latex-process-alist'." type) > + `((?F . ,(shell-quote-argument fg)) > + (?B . ,(shell-quote-argument bg)) > + (?d . ,(shell-quote-argument (format "%s" dpi))) > + (?s . ,(shell-quote-argument (format "%s" (/ dpi 140.0)))) > + (?t . ,(shell-quote-argument texfile)) > + (?i . ,(shell-quote-argument image-input-file)) > + (?p . ,(shell-quote-argument image-output-file)) > + (?b . ,(shell-quote-argument texfilebase)) > + (?o . ,(shell-quote-argument tmpdir))))) As explained above, beware the order of arguments and the type of IMAGE-OUTPUT-TYPE. Also, you don't need to provide ?o, ?b and probably ?t in SPEC. > + (when (file-exists-p image-output-file) IMAGE-OUTPUT-FILE is guaranteed to exist. Otherwise, `org-compile-file' returns an error. > +(defun org-compile-file (source extensions process > + &optional log-buffer message spec) You can remove this, as it is integrated in master. > -`dvipng' Process the LaTeX fragments to images. This will also > +`dvipng' Process the LaTeX fragments to png images. This will also > + include processing of non-math environments. > +`dvisvgm' Process the LaTeX fragments to svg images. This will also > include processing of non-math environments. > `imagemagick' Convert the LaTeX fragments to pdf files and use > imagemagick to convert pdf files to png files. Process types are mostly hard-coded, this is a reason why I don't think inheritance is terribly useful: you almost always want to modify existing processes, not create new ones. I think we're getting close to the merge. Regards, -- Nicolas Goaziou