[PATCH] ob-C.el compile-only header argument, was Re: How to use mpirun with C or C++ Org-babel?

2023-12-19 Thread Leo Butler
On Thu, Dec 14 2023, Ihor Radchenko  wrote:

> Leo Butler  writes:
>
>> From 7d8e406bc4a92e2e2eab772b2671dcd72ca8c202 Mon Sep 17 00:00:00 2001
>> From: Leo Butler 
>> 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!

Thank you for the feedback.

>
>> * 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?

Originally, I felt that only ":results file" makes sense. I have adopted
your suggestion, though, and added test cases so that the compiler
stderr output is caught.

>
>> * 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.

I have added the announcement in this patch. I will submit a separate
patch for worg.

>
>> +(compile-only . (nil no t yes)))
>
> Why nil/t? No other header argument allow "nil" or "t". Just yes/no.

Ok. I also noticed (in a separate thread) that it should be

(compile-only . ((no yes))

>
>> +(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"?

Corrected.

>
>> +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.

Yes.

>
>> +field in `PARAMS'; if that is NIL, an error occurs."
>
> No need to upcase NIL.

Yes.

> Also, "if that is nil, throw an error" - this is more common style
> (saying what function does).

Done.

>
>> +  (let ((f (cdr (assq :file params
>
> Please avoid short variable names - they are harder to understand and
> search in code.

Done.

>
>> +(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.

Ok, I think the current error message is consistent with Gnu
standards.

>
>> +(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.

Ok. I guess, to paraphrase, that readability is in the eye of the beholder.

>
>> +(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.

See my comment above.

> We might want return the compiler output for
> :results output.

Done.

> Or maybe even arrange `org-babel-eval-error-notify' to
> display compile-mode window when there are compilation warnings.

Yes, this is already done by `org-babel-eval' in `org-babel-C-execute'.

>
>> --- 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.

Ok.

Patch attached.

Leo

From 37de68b264bca5f8103a1664626f1572d43f07d4 Mon Sep 17 00:00:00 2001
From: Leo Butler 
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

* lisp/ob-C.el (org-babel-C-execute): The new header argument,
:compile-only, causes source and compiled binary 

Re: [PATCH] Justify/align image previews in org-mode

2023-12-19 Thread Karthik Chikmagalur
>> I've incorporated the following suggestions:
>>
>> - Order of precedence:
>>   + #+attr_org overrides #+attr_html and #+attr_latex
>>   + `:center t' overrides `:align ...'
>
> Why only html and latex? I think that it will be more consistent to
> follow what we do in `org-display-inline-image--width' - any #+attr_*
> attribute. And apart from html/latex, we have
> beamer/hugo/koma-letter/what not is implemented or will be implemented
> deriving from html/latex backend or with support of :center attribute.

Changed implementation to support any #+attr_.* keyword.

>> - Add a checker for `:align nil' to org-lint.  `:align nil' is not
>>   supported.
>
> I meant :align  being not supported in #+attr_org. Not just nil.

org-lint now complains if anything other than `:align left|center|right'
or `:center t' are specified in #+attr_org.

>> +  #+vindex: org-image-align
>> +  Org mode can left-align, center or right-align the display of inline
>> +  images.  This setting is controlled (globally) by ~org-image-align~.
>> +  Only standalone links, /i.e/ links with no surrounding text in their
>
> Maybe "standalone images"?
> Also, please avoid i.e - this is a general Emacs documentation guideline
> to avoid specialized language like i.e., e.g., and iff.

Changed text, removed specialized language.

>> +  paragraphs (except whitespace) are affected.  Its value can be the
>> +  following:
>> +  - (default) nil, insert the image where the link appears in the
>> +buffer.
>
> Why not simply setting the default to 'left and not having nil at all?

The original idea was that `left' and `nil' do the same thing, so it
should be fine to leave it at `nil'.  I changed the user option to
default to `left', and `nil' has been removed as an available option.
(It is still not an error to set it to `nil'.)

>> +  - The symbol ~left~, which is the same as nil.
>> +  - The symbol ~center~, which will preview standalone links centered
>> +in the Emacs window.
>> +  - The symbol ~right~, which will preview standalone links
>> +right-aligned in the Emacs window.
>
> "standalone" links is redundant here - you already mentioned that  

Fixed.

>> +  Inline image alignment can be specified for each link using the
>> +  =#+ATTR.*= keyword if it matches an alignment specification like:
>> +  #+begin_example
>> +  ,#+ATTR_HTML: :align center
>> +  #+end_example
>> +  Supported values for =:align= are =left=, =center= and =right=.
>
> I think that we do not need to specify the supported values here - they
> are the same as for `org-image-align' just above.

Removed.

>> +  Inline image display can also be centered using =:center=, as in
>
> I'd explain that inline image adjustment is also taken from :center
> attribute supported by some export backends (like HTML, LaTeX, and
> Beamer), when #+attr_org is not set.

Tweaked the description of :center.

>> +  #+begin_example
>> +  ,#+ATTR_HTML: :center t
>> +  #+end_example
>> +  Org will use the alignment specification from any =#+ATTR.*=
>> +  keyword, such as =#+ATTR_HTML= or =#+ATTR_LATEX=, but =#+ATTR_ORG=
>> +  (if present) will override the others.  For instance, this link
>> +  #+begin_example
>> +  ,#+ATTR_HTML: :align right
>> +  ,#+ATTR_ORG: :align center
>> +  [[/path/to/image/file.png]]
>> +  #+end_example
>> +  will be displayed centered.
>
> ... "but exported right-aligned to HTML"

Added.

>> +(defun org-lint-invalid-image-alignment (ast)
>> +  (org-element-map ast 'paragraph
>> +(lambda (p)
>> +  (let ((bad-align-re ":align[[:space:]]+nil")
>> +(keyword-string (mapconcat
>> + (lambda (attr)
>> +   (or (car-safe (org-element-property attr p)) 
>> ""))
>> + '(:attr_org :attr_latex :attr_html) " ")))
>
> :align nil is perfectly valid for HTML.
> I thought to warn only about using anything but :align left/right/center
> in #+attr_org. And about :center  in #+attr_org.

Yes, only the value of #+attr_org is checked now.

Patch v2 attached.

Karthik
>From 7176f43282749c06daf2756360633cc47d79b63c Mon Sep 17 00:00:00 2001
From: Karthik Chikmagalur 
Date: Mon, 18 Dec 2023 12:56:33 -0800
Subject: [PATCH] org: Add image alignment

* lisp/org.el (org-image--align, org-image-align,
org-toggle-inline-images): Add the ability to left-align, center
or right-align inline image previews in the Emacs window. This is
controlled globally using the new user option `org-image-align'.
Alignment can be specified per image using the `#+ATTR.*'
affiliated keywords.  The function `org-image--align' determines
the kind of alignment for its argument link.

* lisp/org-lint.el (org-lint-invalid-image-alignment): Add an
org-lint checker to catch invalid ":align" and ":center"
attributes in `#+attr_org' keywords.

* doc/org-manual.org: Document the new feature under the Images
section.
---
 doc/org-manual.org | 33 
 lisp/org-lint.el   | 35 +
 lisp/org.el| 

Re: Links & images with different attributes in the same paragraph

2023-12-19 Thread Max Nikulin

On 16/12/2023 21:44, Ihor Radchenko wrote:

Max Nikulin writes:


@wrap{{outer wrap; @wrap{{inner wrap allowing } as well}}; back}}.


Are you assuming invisible zero-width space as a way to escape literal
{{ or }}? I would prefer some visible characters.


No, not zero-width space. Literally, {{...}}. The idea is to define
delimiters as "[{]+" the matching number of "}". This way, we do not
need to worry about escaping "}" inside and can get nested markup for
free. It is more or less how Org parser works for special block:
the opening delimiter is #+begin_whatever is matched against
#+end_.


I am afraid, there is an issue if wrapped content is surrounded by 
braces. An ambiguity arises for


@wrap{{{content}}}

it may be @wrap{{{ content }}}, @wrap{{ {content} }},
or @wrap{ {{content}} }. It seems, some escape character is unavoidable.


Also, see https://list.orgmode.org/orgmode/87mtaez8do.fsf@localhost/
with my original proposal and some discussion that followed up.


I completely forgot that you wrote about balanced parenthesis earlier.


Juan Manuel MacĂ­as. Re: About 'inline special blocks'

Bringing that into the paragraph is
unnecessarily overloading the paragraph and breaking the social contract
of lightweight markup, where paragraphs should still look like
paragraphs.



I am pretty sure that I replied to that concern raised in a parallel
thread. My idea was to allow macro replacement inside attributes:

#+macro: alt #+attr_html :alt $1
What about @wrap[<<>>]{[[/path/to/image]]}


Certainly it allows to shorten in-text arguments. Just as noweb 
references, it has some disadvantages. I consider comma as macro 
argument separator as a kind of a pitfall. Macro definitions, unlike 
code blocks definitions for noweb references, can not be multiline

(unless defined in elisp code).

Unless something better will be proposed, I consider macro expansion 
inside arguments as a viable approach. (I can not recall if it was 
discussed earlier.)





Re: [PATCH] Justify/align image previews in org-mode

2023-12-19 Thread Ihor Radchenko
Karthik Chikmagalur  writes:

> From eb1b287c009c2f7eb83e7e31d64980ba79f44527 Mon Sep 17 00:00:00 2001
> From: Karthik Chikmagalur 
> Date: Mon, 18 Dec 2023 12:56:33 -0800
> Subject: [PATCH] org: Add image alignment

Bastien, may you please confirm that Karthik's FSF assignment is in order?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 



Re: [PATCH] Justify/align image previews in org-mode

2023-12-19 Thread Ihor Radchenko
Karthik Chikmagalur  writes:

> I've incorporated the following suggestions:
>
> - Order of precedence:
>   + #+attr_org overrides #+attr_html and #+attr_latex
>   + `:center t' overrides `:align ...'

Why only html and latex? I think that it will be more consistent to
follow what we do in `org-display-inline-image--width' - any #+attr_*
attribute. And apart from html/latex, we have
beamer/hugo/koma-letter/what not is implemented or will be implemented
deriving from html/latex backend or with support of :center attribute.

> - Add a checker for `:align nil' to org-lint.  `:align nil' is not
>   supported.

I meant :align  being not supported in #+attr_org. Not just nil.

> +  #+vindex: org-image-align
> +  Org mode can left-align, center or right-align the display of inline
> +  images.  This setting is controlled (globally) by ~org-image-align~.
> +  Only standalone links, /i.e/ links with no surrounding text in their

Maybe "standalone images"?
Also, please avoid i.e - this is a general Emacs documentation guideline
to avoid specialized language like i.e., e.g., and iff.

> +  paragraphs (except whitespace) are affected.  Its value can be the
> +  following:
> +  - (default) nil, insert the image where the link appears in the
> +buffer.

Why not simply setting the default to 'left and not having nil at all?

> +  - The symbol ~left~, which is the same as nil.
> +  - The symbol ~center~, which will preview standalone links centered
> +in the Emacs window.
> +  - The symbol ~right~, which will preview standalone links
> +right-aligned in the Emacs window.

"standalone" links is redundant here - you already mentioned that  

> +  Inline image alignment can be specified for each link using the
> +  =#+ATTR.*= keyword if it matches an alignment specification like:
> +  #+begin_example
> +  ,#+ATTR_HTML: :align center
> +  #+end_example
> +  Supported values for =:align= are =left=, =center= and =right=.

I think that we do not need to specify the supported values here - they
are the same as for `org-image-align' just above.

> +  Inline image display can also be centered using =:center=, as in

I'd explain that inline image adjustment is also taken from :center
attribute supported by some export backends (like HTML, LaTeX, and
Beamer), when #+attr_org is not set.

> +  #+begin_example
> +  ,#+ATTR_HTML: :center t
> +  #+end_example
> +  Org will use the alignment specification from any =#+ATTR.*=
> +  keyword, such as =#+ATTR_HTML= or =#+ATTR_LATEX=, but =#+ATTR_ORG=
> +  (if present) will override the others.  For instance, this link
> +  #+begin_example
> +  ,#+ATTR_HTML: :align right
> +  ,#+ATTR_ORG: :align center
> +  [[/path/to/image/file.png]]
> +  #+end_example
> +  will be displayed centered.

... "but exported right-aligned to HTML"

> +(defun org-lint-invalid-image-alignment (ast)
> +  (org-element-map ast 'paragraph
> +(lambda (p)
> +  (let ((bad-align-re ":align[[:space:]]+nil")
> +(keyword-string (mapconcat
> + (lambda (attr)
> +   (or (car-safe (org-element-property attr p)) 
> ""))
> + '(:attr_org :attr_latex :attr_html) " ")))

:align nil is perfectly valid for HTML.
I thought to warn only about using anything but :align left/right/center
in #+attr_org. And about :center  in #+attr_org.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at