emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: "Kévin Le Gouguec" <kevin.legouguec@gmail.com>
To: Kyle Meyer <kyle@kyleam.com>
Cc: Bastien <bzg@gnu.org>,
	Brandon Guttersohn <brandon@guttersohn.org>,
	emacs-orgmode@gnu.org
Subject: Re: Possible fix for :includes header argument in org-babel C source blocks
Date: Thu, 28 May 2020 12:09:35 +0200	[thread overview]
Message-ID: <87blm8v09s.fsf@gmail.com> (raw)
In-Reply-To: <875zcgwjo7.fsf@gmail.com> (=?utf-8?Q?=22K=C3=A9vin?= Le Gouguec"'s message of "Thu, 28 May 2020 10:25:12 +0200")

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> That leads me to believe that the coercion was an unintended side-effect
> of (format …).

Never mind, the ORG-NEWS entry for 9.1 shows an example of unquoted
header, so I guess it is intentional.

Here is a patch to fix the regression:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Coerce-symbols-found-in-includes-header-arguments-to.patch --]
[-- Type: text/x-diff, Size: 1062 bytes --]

From b68122821a26578470506938c3a358f52f5d7a46 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Thu, 28 May 2020 11:09:18 +0200
Subject: [PATCH 1/2] Coerce symbols found in :includes header arguments to
 strings

Fix regression from 2020-05-24T16:23:26Z!brandon@guttersohn.org
(commit 44cb98fdb), which broke test ob-C/string-var.

* lisp/ob-C.el (org-babel-C-expand-C): Make sure items in :includes
arguments are strings before performing string operations on them.
---
 lisp/ob-C.el | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lisp/ob-C.el b/lisp/ob-C.el
index c3e72c680..42c60c296 100644
--- a/lisp/ob-C.el
+++ b/lisp/ob-C.el
@@ -233,6 +233,9 @@ its header arguments."
 		;; includes
 		(mapconcat
 		 (lambda (inc)
+		   ;; :includes '(<foo> <bar>) gives us a list of
+		   ;; symbols; convert those to strings.
+		   (when (symbolp inc) (setq inc (symbol-name inc)))
 		   (if (string-prefix-p "<" inc)
 		       (format "#include %s" inc)
 		     (format "#include \"%s\"" inc)))
-- 
2.17.1


[-- Attachment #3: Type: text/plain, Size: 146 bytes --]


And here is a patch to add a test for the unquoted-single-header case,
since otherwise it's hard to tell whether this behaviour is intentional:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-Add-test-case-for-symbol-coercion-in-C-Babel-blocks.patch --]
[-- Type: text/x-diff, Size: 4531 bytes --]

From cf1bb27215a46a521bb2f50d16b7dbc7441d81ec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= <kevin.legouguec@gmail.com>
Date: Thu, 28 May 2020 11:47:25 +0200
Subject: [PATCH 2/2] Add test case for symbol coercion in C Babel blocks

The ORG-NEWS entry for version 9.1 suggests that this coercion was
always intended, though AFAICT there was no test case for it.

* testing/lisp/test-ob-C.el (ob-C/symbol-include): Check explicitly
that :includes <iostream> (with no double-quotes around <iostream>)
will be parsed correctly.
(ob-D/simple-program, ob-C/integer-var, ob-D/integer-var,
ob-C/two-integer-var, ob-D/two-integer-var, ob-C/string-var,
ob-D/string-var, ob-C/preprocessor): Adjust block indices.

* testing/examples/ob-C-test.org (Simple tests): Add input for the new
test.
---
 testing/examples/ob-C-test.org |  6 ++++++
 testing/lisp/test-ob-C.el      | 23 +++++++++++++++--------
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/testing/examples/ob-C-test.org b/testing/examples/ob-C-test.org
index 0faf630b9..347607cae 100644
--- a/testing/examples/ob-C-test.org
+++ b/testing/examples/ob-C-test.org
@@ -10,6 +10,12 @@
   return 0;
 #+end_src
 
+#+source: simple
+#+begin_src cpp :includes <iostream> :results silent
+  std::cout << 42;
+  return 0;
+#+end_src
+
 #+source: simple
 #+begin_src D :results silent
   writefln ("%s", 42);
diff --git a/testing/lisp/test-ob-C.el b/testing/lisp/test-ob-C.el
index 3e4a63f88..6b6b728a2 100644
--- a/testing/lisp/test-ob-C.el
+++ b/testing/lisp/test-ob-C.el
@@ -32,60 +32,67 @@
 		      (org-babel-next-src-block 1)
 		      (should (= 42 (org-babel-execute-src-block))))))
 
+(ert-deftest ob-C/symbol-include ()
+  "Hello world program with unquoted :includes."
+  (if (executable-find org-babel-C++-compiler)
+      (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
+		      (org-babel-next-src-block 2)
+		      (should (= 42 (org-babel-execute-src-block))))))
+
 (ert-deftest ob-D/simple-program ()
   "Hello world program."
   (if (executable-find org-babel-D-compiler)
       (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
-		      (org-babel-next-src-block 2)
+		      (org-babel-next-src-block 3)
 		      (should (= 42 (org-babel-execute-src-block))))))
 
 (ert-deftest ob-C/integer-var ()
   "Test of an integer variable."
   (if (executable-find org-babel-C++-compiler)
       (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
-		      (org-babel-next-src-block 3)
+		      (org-babel-next-src-block 4)
 		      (should (= 12 (org-babel-execute-src-block))))))
 
 (ert-deftest ob-D/integer-var ()
   "Test of an integer variable."
   (if (executable-find org-babel-D-compiler)
       (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
-		      (org-babel-next-src-block 4)
+		      (org-babel-next-src-block 5)
 		      (should (= 12 (org-babel-execute-src-block))))))
 
 (ert-deftest ob-C/two-integer-var ()
   "Test of two input variables"
   (if (executable-find org-babel-C++-compiler)
       (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
-		      (org-babel-next-src-block 5)
+		      (org-babel-next-src-block 6)
 		      (should (= 22 (org-babel-execute-src-block))))))
 
 (ert-deftest ob-D/two-integer-var ()
   "Test of two input variables"
   (if (executable-find org-babel-D-compiler)
       (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
-		      (org-babel-next-src-block 6)
+		      (org-babel-next-src-block 7)
 		      (should (= 22 (org-babel-execute-src-block))))))
 
 (ert-deftest ob-C/string-var ()
   "Test of a string input variable"
   (if (executable-find org-babel-C++-compiler)
       (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
-		      (org-babel-next-src-block 7)
+		      (org-babel-next-src-block 8)
 		      (should (equal "word 4" (org-babel-execute-src-block))))))
 
 (ert-deftest ob-D/string-var ()
   "Test of a string input variable"
   (if (executable-find org-babel-D-compiler)
       (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
-		      (org-babel-next-src-block 8)
+		      (org-babel-next-src-block 9)
 		      (should (equal "word 4" (org-babel-execute-src-block))))))
 
 (ert-deftest ob-C/preprocessor ()
   "Test of a string variable"
   (if (executable-find org-babel-C++-compiler)
       (org-test-at-id "fa6db330-e960-4ea2-ac67-94bb845b8577"
-		      (org-babel-next-src-block 9)
+		      (org-babel-next-src-block 10)
 		      (should (= 42 (org-babel-execute-src-block))))))
 
 (ert-deftest ob-C/table ()
-- 
2.17.1


[-- Attachment #5: Type: text/plain, Size: 382 bytes --]


(Is the Org source for the C/C++/D Babel syntax[1] committed somewhere,
BTW?  I could not find it in the Org repo.)


I'd like to say that all tests pass now, but I'm getting failures on
master (even without my changes) for two other tests:

>   FAILED  ob-tangle/jump-to-org
>   FAILED  test-org-attach/dir


[1] https://orgmode.org/worg/org-contrib/babel/languages/ob-doc-C.html

  reply	other threads:[~2020-05-28 10:10 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <21b0cd85-d678-2fe6-3c22-e41abc6cf242@guttersohn.org>
     [not found] ` <87wo51jo5w.fsf@gnu.org>
2020-05-27 22:20   ` Kévin Le Gouguec
2020-05-28  2:30     ` Kyle Meyer
2020-05-28  8:25       ` Kévin Le Gouguec
2020-05-28 10:09         ` Kévin Le Gouguec [this message]
2020-05-29  2:47           ` Kyle Meyer
2020-05-29 12:41             ` Failing tests (was: Possible fix for :includes header argument in org-babel C source blocks) Kévin Le Gouguec
2020-05-31  4:59               ` Kyle Meyer
2020-06-01 14:48                 ` Failing tests Kévin Le Gouguec
2020-06-01 14:56                   ` Kévin Le Gouguec
2020-06-03  4:20                   ` Kyle Meyer
2020-06-01 13:54               ` Bastien
2020-05-29  3:37           ` Possible fix for :includes header argument in org-babel C source blocks Brandon Guttersohn
2020-05-29  9:57             ` Kévin Le Gouguec
2020-05-30 17:29               ` Brandon Guttersohn
2020-06-01 13:55                 ` Bastien
2020-06-01 20:17                   ` Kévin Le Gouguec
2020-06-02  0:02                     ` Brandon Guttersohn
2020-05-29  2:45         ` Kyle Meyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87blm8v09s.fsf@gmail.com \
    --to=kevin.legouguec@gmail.com \
    --cc=brandon@guttersohn.org \
    --cc=bzg@gnu.org \
    --cc=emacs-orgmode@gnu.org \
    --cc=kyle@kyleam.com \
    --subject='Re: Possible fix for :includes header argument in org-babel C source blocks' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).