Org-mode mailing list
 help / color / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: John Herrlin <jherrlin@gmail.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: file-mode source code block header argument
Date: Sat, 01 Aug 2020 01:21:13 -0400
Message-ID: <878sez9c1i.fsf@kyleam.com> (raw)
In-Reply-To: <87h7twls64.fsf@gmail.com>

John Herrlin writes:

> I am looking for a way to set permission on a file created from source
> code block result when :file header argument is used. I was looking for
> something like :tangle-mode but could not find anything. I wrote a patch
> that does just that and it works for my small use case.

Thanks for the patch.

> It's a header argument called :file-mode and can be used in the same
> way as :tangle-mode.
>
> Example usage:
>
> #+BEGIN_SRC shell :results file :file script.sh :file-mode (identity #o755)
>   echo "#!/bin/bash"
>   echo "echo Hello World"
> #+END_SRC
>
> Is this a suitable way of doing it?

Looks sensible to me.  Hopefully others with more knowledge and interest
in Babel will chime in if they disagree.  In the meantime, I mostly just
have convention nit-picks...

> Subject: [PATCH] ob-core: file-mode option in source code block arguments
>
> * ob-core.el (org-babel-execute-src-block): Source code block header
> argument :file-mode can set file permission if :file argument is provided

Please end this sentence with a period.

Also, assuming the listing at
https://orgmode.org/worg/org-contribute.html is up to date, this should
come with a TINYCHANGE cookie.

> ---
>  doc/org-manual.org      | 12 ++++++++++++
>  lisp/ob-core.el         |  5 ++++-
>  testing/lisp/test-ob.el | 17 +++++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)

Please also add an ORG-NEWS entry.

> diff --git a/doc/org-manual.org b/doc/org-manual.org
> index b616446..2919139 100644
> --- a/doc/org-manual.org
> +++ b/doc/org-manual.org
> @@ -17440,6 +17440,18 @@ default behavior is to automatically determine the result type.
>    uses the generated file name for both the "link" and
>    "description" parts of the link.
>  
> +  #+cindex: @samp{file-mode}, header argument
> +  The =file-mode= header argument defines the file permission. For

s/permission/permissions/

Also, the convention for this project is two spaces following a
sentence.

> +  example, to make a read-only file, use ‘:file-mode (identity
> +  #o444)’. To make it executable, use ‘:file-mode (identity #o755)’

These ‘...’ should be =...=, I think.

I know this text is just following the text for :tangle-mode, but from
my point of view, "For example ..." through to the example block could
be dropped.  If the reader knows about file permissions, then it's
unnecessary; if they don't, I don't see how it'd be enough to help them
figure out what's going on.

> +  #+begin_example
> +  ,#+BEGIN_SRC shell :results file :file script.sh :file-mode (identity #o755)
> +    echo "#!/bin/bash"
> +    echo "echo Hello World"
> +  ,#+END_SRC
> +  #+end_example

> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
> index e798595..cc3e002 100644
> --- a/lisp/ob-core.el
> +++ b/lisp/ob-core.el
> @@ -731,7 +731,10 @@ block."
>  		    (with-temp-file file
>  		      (insert (org-babel-format-result
>  			       result
> -			       (cdr (assq :sep params))))))
> +			       (cdr (assq :sep params)))))
> +		    ;; Set permissions if header argument `:file-mode' is provided

To continue my nit-picking: Please follow the style of the surrounding
comments, ending with a period and filling the paragraph.

> +		    (when (assq :file-mode params)
> +		      (set-file-modes file (cdr (assq :file-mode params)))))

Looks good.

>  		  (setq result file))
>  		;; Possibly perform post process provided its
>  		;; appropriate.  Dynamically bind "*this*" to the
> diff --git a/testing/lisp/test-ob.el b/testing/lisp/test-ob.el
> index 7c44622..c4aaad1 100644
> --- a/testing/lisp/test-ob.el
> +++ b/testing/lisp/test-ob.el
> @@ -1746,6 +1746,23 @@ line 1
>  		   (cdr (assq :file (nth 2 (org-babel-get-src-block-info t))))))
>      ))
>  
> +(ert-deftest test-ob/file-mode ()
> +  "Ensure that file have correct permissions."
> +  (let* ((file       (org-babel-temp-file "file-mode-" ".sh"))
> +	 (filename   (file-name-nondirectory file))
> +	 (path       (file-name-directory file)))
> +    (org-test-with-temp-text
> +	(concat
> +	 "#+BEGIN_SRC emacs-lisp :results file "
> +	 ":file " filename " "
> +	 ":output-dir " path " "
> +	 ":file-mode (identity #o755)
> +     nil
> +     #+END_SRC")

Rather than using org-babel-temp-file to generate a temporary file, I
think the preferred way to do this would be to use
org-test-with-temp-text-in-file.

> +      (org-babel-execute-src-block))
> +    (should (equal (file-modes file)
> +		   493))))

To my eyes, spelling this as #o755 would be clearer.

So, perhaps something like this:

(ert-deftest test-ob/file-mode ()
  "Ensure that file have correct permissions."
  (should
   (equal #o755
          (org-test-with-temp-text-in-file "
#+begin_src emacs-lisp :results file :file t.sh :file-mode (identity #o755)
nil
#+end_src"
            (org-babel-next-src-block)
            (org-babel-execute-src-block)
            (unwind-protect
                (file-modes "t.sh")
              (delete-file "t.sh"))))))


  parent reply index

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-24 17:49 John Herrlin
2020-07-30 19:47 ` Russell Adams
2020-08-01  5:21 ` Kyle Meyer [this message]
2020-08-02 10:15   ` John Herrlin
2020-08-03  3:06     ` 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://orgmode.org

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

  git send-email \
    --in-reply-to=878sez9c1i.fsf@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=jherrlin@gmail.com \
    /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

Org-mode mailing list

Archives are clonable:
	git clone --mirror https://orgmode.org/list/0 list/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 list list/ https://orgmode.org/list \
		emacs-orgmode@gnu.org
	public-inbox-index list

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.orgmode
	nntp://news.gmane.io/gmane.emacs.orgmode


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git