From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp10.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id yLAMOSouyWLIwgAAbAwnHQ (envelope-from ) for ; Sat, 09 Jul 2022 09:28:43 +0200 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp10.migadu.com with LMTPS id EAoXOCouyWLZsgAAG6o9tA (envelope-from ) for ; Sat, 09 Jul 2022 09:28:42 +0200 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 68BC0D888 for ; Sat, 9 Jul 2022 09:28:42 +0200 (CEST) Received: from localhost ([::1]:58904 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oA4tN-00022a-4q for larch@yhetil.org; Sat, 09 Jul 2022 03:28:41 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:57390) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oA4s5-00022O-JW for emacs-orgmode@gnu.org; Sat, 09 Jul 2022 03:27:21 -0400 Received: from mail-pg1-x52b.google.com ([2607:f8b0:4864:20::52b]:43843) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oA4s3-00039n-IH for emacs-orgmode@gnu.org; Sat, 09 Jul 2022 03:27:21 -0400 Received: by mail-pg1-x52b.google.com with SMTP id 73so706329pgb.10 for ; Sat, 09 Jul 2022 00:27:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:in-reply-to:references:date:message-id :mime-version; bh=za2MGnScUzoIfAwvXBriJWrtVXPhoecaP2ql6osLS0s=; b=hrrn5T5CkKnLyOwak4Kzw/adz4Zh80So/VBnzxMBoG6Oh9UXKmrgJJF3C3/ldNdRtE 6ZM95RrWgGeonn02JReYVNsS32B6XJbrzktRi1ig40gH3yQsMoYLOTkRN2+o4X4Z1Oo6 LY+HItrMqhL8JC2FGMysBuu1O3PgOC2A47jKIlbDtQwpbIuz7cmMT3f2EejCXt6fVPMg BU3pSCcwl37dF0W2p1MqRCquFGdELDS/YoFHRfB2sbvhyYsgO8sXFOdu47T1/+kgEtvC jImV2SdJHHI5JxRW7Z/jgqqLrVDrjwDCbVfcmtfof1lvybV+RINOJJFgn9kuavKsd8eQ 7Y8g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=za2MGnScUzoIfAwvXBriJWrtVXPhoecaP2ql6osLS0s=; b=DCwmRyQ3rMDXeGoxdEv7+/ASSto50VnmVc/yBBO3fjtLOAI9F/DeaT7TZcSAa+e7hl bBP50NBBzHD/1edMzns9RR3l4S1PCqTTJQiJZRysR2zTsihPmzRr6pku1quU7mQJZde9 yXYAxnJndgZKfSbkocbQiF1KUiCQCJqcnvO/av30UPRJZw8FTys4iaR2R6aTCiBldWMf dY3/JZdbo4f0bidpxsxYKrmOaex1P7j1z/fwqzPIyj0VcuNsXSqTlh9gYQ+dUddDG6Xc c5MEpCrCNEhCWn+cFKYPQ3VfbaDib2EbVmK/LfZ1IIeyV84GpIh4zvOX/zxwEhOP/TXG C4mQ== X-Gm-Message-State: AJIora+SKj7y+LOeNUWGu/xhLI5QLbrTBp+9pwqOfQPou+fiSs873zdR 1tifUZ+/4+ONewD3pAVHEi4= X-Google-Smtp-Source: AGRyM1t+nMxqFQBWMZL3+oI4JorDqIIQvKKrq5Y7/2hkYUiWDguFlI5TdrIndV6Z1b25WxVHN/+9KA== X-Received: by 2002:a63:1c5c:0:b0:414:df9a:7dea with SMTP id c28-20020a631c5c000000b00414df9a7deamr6524395pgm.542.1657351637268; Sat, 09 Jul 2022 00:27:17 -0700 (PDT) Received: from localhost ([134.73.242.163]) by smtp.gmail.com with ESMTPSA id a8-20020a621a08000000b00528e84c3093sm811378pfa.143.2022.07.09.00.27.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 09 Jul 2022 00:27:16 -0700 (PDT) From: Ihor Radchenko To: Matt Huszagh Cc: Emacs-Orgmode Subject: Re: [PATCH] Remove additional newline at end of results block In-Reply-To: <87zghsi3ki.fsf@localhost> References: <87zghsi3ki.fsf@localhost> Date: Sat, 09 Jul 2022 15:28:23 +0800 Message-ID: <87ilo6sq20.fsf@localhost> MIME-Version: 1.0 Content-Type: text/plain Received-SPF: pass client-ip=2607:f8b0:4864:20::52b; envelope-from=yantar92@gmail.com; helo=mail-pg1-x52b.google.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_ENVFROM_END_DIGIT=0.25, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham 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" X-Migadu-Flow: FLOW_IN X-Migadu-To: larch@yhetil.org X-Migadu-Country: US ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1657351722; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=za2MGnScUzoIfAwvXBriJWrtVXPhoecaP2ql6osLS0s=; b=lt6NNegJ/rue8MX7DTjqZhMkrmO+bAfdP+JNFSxyUgQ34oijqHlzx4BER5UHAHOsfodDX2 fpGGMno73SGt9yZfbPcTIF6frmhp+kpgg41QaG/YVfOG6mWrb414OB+VwLam6tnZoGPjiy +8e87t+gMW8E1ENIpxZRrM74h+8QRipHXGDNv57vufSow/AwB79y+Iulgsdq9EhmCXXVw4 SBPTfduOPNRIfqy1QwFu9GliExlOPkzOuO0etOa7d20sS8T8EcDIvoy9biAnonfxH9eYRK fOHOuUlGIt+rH7h7xIE4gwFOPR0R1otHtknlsTNttqwBZUYd2TFeyjDT1I+vcA== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1657351722; a=rsa-sha256; cv=none; b=sbqp7CVCUvrbQLTh5S5Rjric6RXR9AEMo9eaDpaBqzrDgxvxxh97l17/ITUeMEjWLHKcXo +hiWAyvYQSgAJpEIW62CmN2wnuqB2l+fo80HJMewEfSTQofNcXg2HGLObxmMvAu9vjb069 V86xvmHLeYCBis1+rGxO0h7c+DqvKOyyIDqdYvD76C4EFS/4J6PSh9iPiKUkPLI4BAlLw+ baUsjO3rpGySbNMuynVk6/tZbYrTvcWZw78XysQO4s4DL/0da+aVwErb1Q4dvfF7oWllX5 m+0t/weg50s5wNXrlipx9furVW24tu8O3+PLT9o9GAejl7kc77o5k3a5EssgYg== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=hrrn5T5C; dmarc=pass (policy=none) header.from=gmail.com; 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" X-Migadu-Spam-Score: -3.45 Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=hrrn5T5C; dmarc=pass (policy=none) header.from=gmail.com; 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" X-Migadu-Queue-Id: 68BC0D888 X-Spam-Score: -3.45 X-Migadu-Scanner: scn1.migadu.com X-TUID: qxHiqOaM3LUN I am replying here because the original reply did not go into my inbox. Matt Huszagh writes: >> I'd like to request other people who use export and source blocks >> extensively to try the patch and see if it breaks anything. >> >> Meanwhile, could you please reword the commit message and make it more >> concise and clear? > > Can you clarify what you find to be unclear? Rereading my own commit > message, my only problem with it is how it starts: I'd add one sentence > to contextualize it a bit. For instance, I will provide a detailed response below. Also, I have found an edge case where your patch creates an issue: #+begin_src emacs-lisp 2 #+end_src : fixed-width area, unrelated to the above. If you execute the above code with your patch, the fixed width area gets deleted. > The previous behavior always ensured the presence of an empty line > between the results block of a source block and the subsequent > text. However, inserting this newline prevents a valid use-case and > protects against an edge-case that is completely avoidable without the > additional guarantee it provides. ... (the rest remains unchanged) The second sentence is hard to understand because (1) it contains two non-obvious statements; (2) the statements will only become clear for the reader after reading the next sentences. In writing, it is generally advised to start from something reader is familiar with and introduce new things one by one. > This commit message isn't short, but I think it's very clear. It > describes the previous behavior, explains the rationale for that > behavior, and then illustrates (with a complete example) how this > prevents a valid use case. It also explains why the new change does not > prohibit any behavior that was previously possible. > > As someone who frequently uses git log, I'd much rather see a commit > message like this than the typical (usually) unhelpful one or two > sentences that fail to provide the motivation for a change. There's no > downside to a long commit message, and this one is structured such that > it proceeds from more general to more specific information - not > everyone has to read the entire thing. > > I'm not trying to be difficult :) but I do care about the quality of > this codebase (as I know do you), so I'm reluctant to change something > in a way I feel is inferior. But, if you have specific parts etc you > feel are unclear I'm more than happy to address those. Sorry, I think you misunderstood my intentions. I am not against detailed commit messages. I also prefer when commits have sufficient information to understand the motivation behind. However, there is no reason to give lengthy and hard-to-understand explanations when more concise wording is possible. Now, detailed comments on what is confusing about the commit message: > * lisp/ob-core.el (org-babel--insert-results-keyword): Remove newline > at end of results block. This is minor, but I'd rather say "Do not add newline ...". Because it is what the patch actually does. > Inserting this newline prevents a valid use-case and protects against > an edge-case that is completely avoidable without the additional > guarantee it provides. The original intention for inserting the > newline was to avoid the edge case in which a user does not insert a > newline between a source block and the subsequent text and the > subsequent text is merged with the results. This is hard to understand. It is unclear which part of code you are referring to. If a reader is not familiar with the changed function, the first statement "inserting this newline..." is unclear. What does "this" refer to? In the second sentence you are trying to describe the original edge case, but it is really hard to imagine without having an example. I'd say something like: "Previously, `org-babel--insert-results-keyword' inserted an empty line after result of evaluation even when the original source block had no empty lines. This was done to address the following issue:" Then, I'd provide an example on the original issue. Then, I'd continue with your original wording: > However, there are valid cases in which a user would not want a > newline between a results block and the subsequent buffer text. For > example, many display equations in LaTeX are considered as part of the > surrounding paragraph. Additionally, it is possible to setup a LaTeX > source block to be executable and insert results into the org > buffer. Consider the following example: > > some org file (leading colons to prevent git ignoring lines starting > with #): > ``` > : We can write the simplest equation as > : #+begin_src latex > : \begin{equation} > : 1 + 1 = 2, > : \end{equation} > : #+end_src > : and hope that no one is confused by this. > ``` "and hope that no one is confused by this" sounds too similar to the main commit message. When I was reading your example Org text, I kept confusing this phrase with the actual message. > We might then execute this source block to generate some output. For > example, this might generate and SVG image of the block and insert it > into the buffer. That should result in something like > > ``` > : We can write the simplest equation as > : #+begin_src latex > : \begin{equation} > : 1 + 1 = 2, > : \end{equation} > : #+end_src > > : #+RESULTS: > : #+begin_results > : [[file:some/file.svg]] > : #+end_results > : and hope that no one is confused by this. > ``` This is confusing because image is _not_ produced by ob-latex.el with default settings. Moreover, attempting to export will _not_ export to :exports both. The default value is :exports results. Are you talking about export here? Something else? Where is the newline you were talking about? > When formatted this way, the resulting tex > file (org-latex-export-to-latex) will be: > > ``` > We can write the simplest equation as > \begin{equation} > 1 + 1 = 2, > \end{equation} > and hope that no one is confused by this. > ``` > which will render correctly. Specifically, the display math > environment will not start a new paragraph after the leading "as" and > a new paragraph will not start between the end of the math display and > the trailing "and". This is confusing because you introduce the behavior after the patch before showing what is wrong with the current behavior. I was reading the above as the current behavior until I reached the next paragraph. Then, I felt lost. > However, the current behavior results in > > ``` > : We can write the simplest equation as > : #+begin_src latex > : \begin{equation} > : 1 + 1 = 2, > : \end{equation} > : #+end_src > > : #+RESULTS: > : #+begin_results > : [[file:some/file.svg]] > : #+end_results > > : and hope that no one is confused by this. > ``` > > This blank line necessarily starts a new paragraph in TeX. This is again confusing. You just showed export output that did not generate the image and now you suddenly do show an image. Note that I do understand that you structured the logic as: (1) show expected result of C-c C-c; (2) show expected result of export; (3) show the current wrong result of C-c C-c; (4) show the current wrong result of export. However, it is not the logic I expected. I only managed to grasp it after reading the whole commit message multiple times. > Finally, as previously stated, it is entirely possible to control > whether there is a newline between the results block and subsequent > text by leaving a newline between the source block and text. For > example, > > ``` > : We can write the simplest equation as > : #+begin_src latex > : \begin{equation} > : 1 + 1 = 2, > : \end{equation} > : #+end_src > > : and hope that no one is confused by this. > ``` > > would still result in > > ``` > : We can write the simplest equation as > : #+begin_src latex > : \begin{equation} > : 1 + 1 = 2, > : \end{equation} > : #+end_src > > : #+RESULTS: > : #+begin_results > : [[file:some/file.svg]] > : #+end_results > > : and hope that no one is confused by this. > ``` And now my item (4) in the above understanding is not followed. I do not see where this example is coming from. Current wrong behavior? On export? On C-c C-c? After patch? Confusing. I would drop the C-c C-c examples and just focus on export, what is wrong with current export behavior and what the patch will improve. Best, Ihor