[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: file-mode source code block header argument
From: |
John Herrlin |
Subject: |
Re: file-mode source code block header argument |
Date: |
Sun, 02 Aug 2020 12:15:43 +0200 |
User-agent: |
mu4e 1.4.10; emacs 26.3 |
Thank you for the comments Kyle! I updated the patch accordingly. Took
your test straight of as I think it's really clean and easy to reason
about.
Best regards
Kyle Meyer <kyle@kyleam.com> writes:
> 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"))))))
0001-ob-core-file-mode-option-in-source-code-block-argume.patch
Description: Text Data