From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2.migadu.com ([2001:41d0:303:e16b::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms13.migadu.com with LMTPS id QL/qGrXwLWfvVAEAe85BDQ:P1 (envelope-from ) for ; Fri, 08 Nov 2024 11:06:29 +0000 Received: from aspmx1.migadu.com ([2001:41d0:303:e16b::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2.migadu.com with LMTPS id QL/qGrXwLWfvVAEAe85BDQ (envelope-from ) for ; Fri, 08 Nov 2024 12:06:29 +0100 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org"; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1731063989; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=FALeAINQ2lMuHloTD9hIxiYqKHruVo45XbZqP4wgoxM=; b=BjJz+OMRcimN/sxwq5KpP1i07E+8dmdFDfKkKGflD5OL7JnUrhgnQbgI9hQ0n7hi28JiaS F8+pxQ7s3fWED4lMROMUazQRQ+1K8bS/IYMk0wiD5c9NMdBWlUvQlOqXxfuFb8bHVrQXJe x6Bf2KxMqkYFRgxDgJeIBIF9q+/+mU/Uw0MGL+ZG8euLTDTSg5mcXvuzBftWrh70p+PJjA RRA+5FOTER7n/lmpieInBiX2FIOmyNbZ97RZdrudHljAV3PwRpQxZAjzvV+Ntiuy6uF8uk B10QKyIPPhl/HWDiK1iRBKA4sCY3cqDOoj7X2DUp4ZQqFtNna6ORbmYg4YLJbw== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1731063989; a=rsa-sha256; cv=none; b=FJCj9LZshYZpCYSJlsTywOsu7faoDcDlQA81O9USVe7BfUkPX5K0PbAo1Z1ruWXeXC9wV+ xZ6e1JkBpJTsQ31VCVKm6+p3bX3fvrG215IJmYc3MDHqBVM1Bfqf7Pqs/8GE1J41DT6hce JgClaPd+69ebtKDJGb27KAqvB69E9qLK3UVkuDslDyv9xSTliRYxeDhw4kPStTENKqyAHu YsxpDuSvEzYCM8SQ2Vnc0wpEvQPrtmCavPvH4R2hqAM1QfW26YAjVmJPgypB60wuJbREzD FE+EWNEsZ0uk5wqWJXl7I+hq7T3hEiMWsbQQIXQRHzWbpiOmSaexxoPYLUM2Jg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; spf=pass (aspmx1.migadu.com: domain of "emacs-orgmode-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="emacs-orgmode-bounces+larch=yhetil.org@gnu.org"; dmarc=none Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 20A837A5F6 for ; Fri, 08 Nov 2024 12:06:29 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t9MoA-0005Hz-8J; Fri, 08 Nov 2024 06:05:42 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t9Mnz-0005HU-OF for emacs-orgmode@gnu.org; Fri, 08 Nov 2024 06:05:32 -0500 Received: from ciao.gmane.io ([116.202.254.214]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1t9Mnx-0000AG-Oa for emacs-orgmode@gnu.org; Fri, 08 Nov 2024 06:05:31 -0500 Received: from list by ciao.gmane.io with local (Exim 4.92) (envelope-from ) id 1t9Mnt-0004kl-F4 for emacs-orgmode@gnu.org; Fri, 08 Nov 2024 12:05:25 +0100 X-Injected-Via-Gmane: http://gmane.org/ To: emacs-orgmode@gnu.org From: Jarmo Hurri Subject: Re: [PATCH] ob-ditaa.el: custom var name, ditaa executable, SVG output, and chararacter encoding Date: Fri, 08 Nov 2024 13:05:15 +0200 Message-ID: <87a5eanfys.fsf@iki.fi> References: <875xp476pt.fsf@iki.fi> <87msiggqjm.fsf@localhost> Mime-Version: 1.0 Content-Type: text/plain User-Agent: Gnus/5.13 (Gnus v5.13) Cancel-Lock: sha1:/cXa6XCDfJc6621L19W+OdTJXFE= Received-SPF: pass client-ip=116.202.254.214; envelope-from=geo-emacs-orgmode@m.gmane-mx.org; helo=ciao.gmane.io X-Spam_score_int: -15 X-Spam_score: -1.6 X-Spam_bar: - X-Spam_report: (-1.6 / 5.0 requ) BAYES_00=-1.9, HEADER_FROM_DIFFERENT_DOMAINS=0.249, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-orgmode@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+larch=yhetil.org@gnu.org Sender: emacs-orgmode-bounces+larch=yhetil.org@gnu.org X-Migadu-Flow: FLOW_IN X-Migadu-Country: US X-Migadu-Spam-Score: -2.15 X-Spam-Score: -2.15 X-Migadu-Queue-Id: 20A837A5F6 X-Migadu-Scanner: mx13.migadu.com X-TUID: PbEyA56EQ8fF Greetings Ihor. Thanks for your feedback. A couple of notes and questions before I can proceed to format the next version of the patch. Ihor Radchenko writes: > Note that we will also need to update > https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-ditaa.html > after we finalize changes in the code. Noted. >> -(defcustom org-babel-ditaa-java-cmd "java" > >> +(defcustom org-ditaa-java-exec "java" >> + "Java executable to use when evaluating ditaa blocks using a JAR." >> + :group 'org-babel >> + :type 'string) > > We generally do not rename variables irreversibly. Please leave an > obsolete alias for `org-babel-ditaa-java-cmd' pointing to the new > variable name. Otherwise, the existing configs that were using the old > variable name will be broken. Will do so. This will also move the contents in ORG-NEWS to a different section, since there will no longer be any "breaking changes." >> +;;; small helper function returning file if it exists and signalling >> +;;; error otherwise >> +(defun org-ditaa-ensure-jar-file (file) >> + (if (file-exists-p file) >> + file >> + (error "could not find jar file %s" file))) > > Rather than writing what the function does in the comment, please do > it in the docstring. We might also make this function internal. Check. > Also, the error sounds very generic. It would be nicer to indicate to > the user that the problem is related to ob-ditaa. Check. >> + (png (cdr (assq :png params))) >> + (svg (cdr (assq :svg params))) >> (eps (cdr (assq :eps params))) > > I am wondering if we could instead deprecate the :png/:eps parameters > and instead use the :file extension to decide. This could be done, but I do not see much harm in providing an override. Note that the file extension is used by default. So, your choice: is it a) file extension only b) file extension with possibility to override with parameters? >> + (message cmd) >> + (shell-command cmd) >> + (when pdf >> + (let ((pdf-cmd (concat "epstopdf" " " ditaa-out-file " " >> + "-o=" (org-babel-process-file-name out-file)))) >> + (message pdf-cmd) > > Why message? I was originally directed to ob-plantuml, which message's its command. During the testing of this patch I found messaging useful to observe what was happening. So, your choice: a) no messaging b) message always c) defcustom a toggle for messaging? And, finally, should I add myself as the maintainer? All the best, Jarmo