Jarmo Hurri <jarmo.hu...@iki.fi> writes: >> Should be 9.8. We do not make breaking changes in bugfix releases. >> See https://orgmode.org/worg/org-maintenance.html#branches > > Changed it to 9.8, although I think we will have no breaking changes > after all your latest improvement ideas have been incorporated.
We also avoid non-trivial changes on bugfix. (See the link.) >> I just checked FSF records, and it does not look like you have an >> active copyright assignment. Could you please check? > > I have signed one a long time ago. I will check the situation at some > point. Note that I will need to confirm the assignment before I can commit your patch. >> The newlines are completely off in the commit message. Also, the >> commit mesage does not have a short patch description on the first >> line and does not conform to >> https://orgmode.org/worg/org-contribute.html#commit-messages > > Not sure what happened there, weird end result of copy-pasting, amend > and the like. Fixed in new version. Thanks, but it does not yet follow all the commit message conventions. Please, (1) fill the commit message lines; (2) Use `quote', not `quote`; (3) use double space between sentences. >>> +(defun ob-ditaa--ensure-jar-file (file) >>> + "Small internal helper function returning file if it exists and >>> signalling error otherwise." >> >> Please shorten the first line of the docstring below 60 characters. >> See D.6 Tips for Documentation Strings section of Eliso manual. > > Done. Not like that. The first line should be a complete sentence. • Format the documentation string so that it fits in an Emacs window on an 80-column screen. It is a good idea for most lines to be no wider than 60 characters. The first line should not be wider than 74 characters, or it will look bad in the output of ‘apropos’. • The first line of the documentation string should consist of one or two complete sentences that stand on their own as a summary. ‘M-x apropos’ displays just the first line, and if that line's contents don't stand on their own, the result looks bad. In particular, start the first line with a capital letter and end it with a period. > * lisp/ob-ditaa.el (org-babel-default-header-args:ditaa) Include new header > arg `:encoding`. Add `graphics` to default value of `:results`. Add `png` as > default value of header arg `:file-ext`. There is no need to quote Org mode's keywords. Just Elisp symbols. > (org-ditaa-ensure-jar-file): Write a small helper function checking existence > of jar file. *ob-ditaa--ensure-jar-file > Character encoding of ditaa source code blocks was passed via Java, > while the encoding can be specified directly as a parameter to > ditaa. Character encoding is now passed directly to ditaa, with a > corresponding header argument `:encoding`. I am wondering if it makes sense to pass arbitrary encoding here. AFAIU, the encoding specifies input encoding and that's what Org mode controls, not user. In fact, we can even explicitly specific utf-8 in (with-temp-file in-file (insert body)) by let-binding `coding-system-for-write' around. I suspect that users will never use :encoding parameter and if they do, they will run into problems as they can't easily control the encoding ob-ditaa uses to write the file. (Is it even useful to have custom encoding?) > +*** =ob-ditaa=: output type control, ditaa executable, SVG output, and > chararacter encoding > + > +Output file type is determined as specified in Babel documentation: > +the suffix of =:file= is the primary determinant, and =:file-ext= > +secondary. Header argument =:eps= is supported for backwards > +compatibility. Default output type is still PNG. > + > +In order to use an executable instead of a JAR file, you can set > +=org-ditaa-default-exec-mode= to ='ditaa=. The location of the > +executable can be configured via =org-ditaa-exec=. > + > +SVG output can now be generated; note, however, that this requires a > +ditaa version of at least 0.11.0. > + > +Character encoding is passed directly to ditaa (earlier to Java) and > +can be controlled by new header argument =:encoding=. > + > +To align with other customizable variable names, which do not contain > +the word =babel=, variable =org-babel-ditaa-java-cmd= has been renamed > +to =org-ditaa-java-exec=. The old variable =org-babel-ditaa-java-cmd= > +is still available as an obsolete alias. This needs to work. In particular, you need to split the announcement into new features/new options/removed or renamed functions and variable - headings under "Version 9.8 (not released yet)" in ORG-NEWS. > +(defcustom org-ditaa-default-exec-mode 'jar > + "Method to use for ditaa diagram generation when generating png or svg > output. > +`jar' means to use java together with a JAR. > +The JAR must be set via `org-ditaa-jar-path'. > + > +`ditaa' means to use the ditaa executable. > +The executable can be configured via `org-ditaa-exec'." > + > + :group 'org-babel > + :package-version '(Org . "9.8") > + :type 'symbol > + :options '(ditaa jar)) :options is not what you meant here. It is about widgets, not about variable values. You need custom :type '(choice ...). For example, see `org-num-face' (there are many examples in Org code). > + (let* ((out-file (org-babel-graphical-output-file params)) > + (out-file-suffix (file-name-extension out-file)) > + (svg (string= out-file-suffix "svg")) > + (pdf (string= out-file-suffix "pdf")) > + (png (string= out-file-suffix "png")) Note that :pdf parameter also used to be there. > + (eps (or (string= out-file-suffix "eps") > + ;; backwards-compatibility of :eps header argument > + (and (not (or svg pdf png)) > + (cdr (assq :eps params)))))) This is not exactly backwards-compatible I think. :eps used to take precedence over file name. That said, looking at the code, it does not look like :eps worked reliably. Looks like it had to be used together with :pdf. But maybe a miss something. -- Ihor Radchenko // yantar92, Org mode maintainer, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92>
