From: Ihor Radchenko <yantar92@posteo.net>
To: Leo Butler <Leo.Butler@umanitoba.ca>
Cc: tbanelwebmin <tbanelwebmin@free.fr>,
"emacs-orgmode@gnu.org" <emacs-orgmode@gnu.org>
Subject: Re: How to use mpirun with C or C++ Org-babel?
Date: Thu, 14 Dec 2023 14:08:36 +0000 [thread overview]
Message-ID: <87r0joq3kr.fsf@localhost> (raw)
In-Reply-To: <87r0jq3x0e.fsf@t14.reltub.ca>
Leo Butler <Leo.Butler@umanitoba.ca> writes:
> From 7d8e406bc4a92e2e2eab772b2671dcd72ca8c202 Mon Sep 17 00:00:00 2001
> From: Leo Butler <leo.butler@umanitoba.ca>
> Date: Tue, 12 Dec 2023 12:32:41 -0600
> Subject: [PATCH] lisp/ob-C.el: add :compile-only header to compile to a named
> target
Thanks for the patch!
> * lisp/ob-C.el (org-babel-C-execute): The new header argument,
> `:compile-only', causes source and compiled binary files to be named
> using the `:file' header argument. When `:compile-only' is set,
> execution of source block ends at compilation. The naming of source
> and binary filenames is factored out to `org-babel-C-src/bin-file'.
What will happen if we have something like :results value or :results
output instead of :results file link?
> * lisp/ob-C.el (org-babel-C-src/bin-file): A new function that factors
> out the setting of source and binary filenames. It also signals an
> error if `:compile-only' is set, but `:file' is not.
> * testing/examples/ob-C-test.org: Add three example that exercise the
> `:compile-only' header argument, including one that causes an error.
> * testing/lisp/test-ob-C.el: Add three tests of the `:compile-only'
> header argument. New tests: ob-C/set-src+bin-file-name-{1,2,3}.
You should also announce the new feature in ORG-NEWS and document it in
WORG.
> + (compile-only . (nil no t yes)))
Why nil/t? No other header argument allow "nil" or "t". Just yes/no.
> +(defun org-babel-C-src/bin-file (params src? compile-only?)
> + "Return the src or bin filename to `org-babel-C-execute'.
> +
> +If `SRC?' is T, a file extension is added to the filename. By
Just SRC?. You should only quote Elisp symbols and upcase the function
arguments. See
https://www.gnu.org/software/emacs/manual/html_node/elisp/Documentation-Tips.html
Also, why upcase "T"?
> +default, the filename is created by `org-babel-temp-file'. If
> +`COMPILE-ONLY?' is T, the filename is taken from the `:file'
I think quoting :file is not necessary.
> +field in `PARAMS'; if that is NIL, an error occurs."
No need to upcase NIL.
Also, "if that is nil, throw an error" - this is more common style
(saying what function does).
> + (let ((f (cdr (assq :file params))))
Please avoid short variable names - they are harder to understand and
search in code.
> + (when (and compile-only? (null f))
> + (error "Error: When COMPILE-ONLY is T or YES, output FILE needs to be set"))
t or "yes". Also, what does "output FILE" refer to? Upcasing implies
function argument, but you are referring to :file header argument in PARAMS.
> + (let* ((file (cond (compile-only? f) (src? "C-src-") (t "C-bin-")))
> + (ext (if src? (pcase org-babel-c-variant
> + (`c ".c") (`cpp ".cpp") (`d ".d"))
We usually split `cond' and `case' into multiple lines for readability.
Otherwise, it is confusing, especially in `let' forms where one can
confuse `cond' forms with `let' bindings.
> + (unless compile-only?
> + (let ((results
> + (org-babel-eval
> ...
No return value at all? I'd expect file link to be returned, as we
discussed in another thread. Also, see the above considerations about
:results value/output. We might want return the compiler output for
:results output. Or maybe even arrange `org-babel-eval-error-notify' to
display compile-mode window when there are compilation warnings.
> --- a/testing/examples/ob-C-test.org
> +++ b/testing/examples/ob-C-test.org
> @@ -174,3 +174,29 @@ std::cout << "\"line 1\"\n";
> std::cout << "\"line 2\"\n";
> std::cout << "\"line 3\"\n";
> #+end_src
If you can, please avoid adding tests as Org files where possible.
Instead, we prefer using `org-test-with-temp-text' or
`org-test-with-temp-text-in-file' when the Org fragment for the test is
not too long.
--
Ihor Radchenko // yantar92,
Org mode contributor,
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>
next prev parent reply other threads:[~2023-12-14 14:06 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-07 23:10 How to use mpirun with C or C++ Org-babel? Edgar Lux
2023-12-08 10:36 ` tbanelwebmin
2023-12-08 15:47 ` Leo Butler
2023-12-08 22:24 ` Ihor Radchenko
2023-12-13 16:09 ` Leo Butler
2023-12-14 14:08 ` Ihor Radchenko [this message]
2023-12-19 22:14 ` [PATCH] ob-C.el compile-only header argument, was " Leo Butler
2023-12-22 12:20 ` Ihor Radchenko
2024-01-04 17:53 ` Leo Butler
2024-01-05 12:47 ` Ihor Radchenko
2023-12-20 18:15 ` Leo Butler
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=87r0joq3kr.fsf@localhost \
--to=yantar92@posteo.net \
--cc=Leo.Butler@umanitoba.ca \
--cc=emacs-orgmode@gnu.org \
--cc=tbanelwebmin@free.fr \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public 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).