emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* ob-ruby org-babel-ruby-table-or-string numeric argument patch
@ 2015-08-10 18:00 Matthew MacLean
  2015-08-16 17:20 ` Nicolas Goaziou
  0 siblings, 1 reply; 4+ messages in thread
From: Matthew MacLean @ 2015-08-10 18:00 UTC (permalink / raw)
  To: emacs-orgmode


[-- Attachment #1.1: Type: text/plain, Size: 629 bytes --]

Yo~!

fa5fd6351605912ec75e783cb626497b1ebe471e introduced a change where
org-babel-script-escape stopped accepting numbers. This caused an issue in
ob-ruby.el where when trying to evaluate something like "2 + 2", you would
get the message:

  `org-babel-script-escape' expects a string

This broke evaluation of Ruby code blocks.

I suspect this is not the only location where this problem might arise, so
I am submitting a patch so the function simply returns numbers if they are
passed in rather than dying. (Because numbers don't need to be escaped, and
this was the previous behaviour.)

Could you review my patch? Thanks..!

[-- Attachment #1.2: Type: text/html, Size: 737 bytes --]

[-- Attachment #2: org-babel-script-escape-numbers.patch --]
[-- Type: text/x-patch, Size: 2814 bytes --]

diff --git a/lisp/ob-core.el b/lisp/ob-core.el
index e3abe97..01c4da8 100644
--- a/lisp/ob-core.el
+++ b/lisp/ob-core.el
@@ -2813,34 +2813,37 @@ block but are passed literally to the \"example-block\"."
       (error "Unterminated string in `org-babel-script-escape'"))
     (apply #'string (reverse out))))
 
-(defun org-babel-script-escape (str &optional force)
-  "Safely convert tables into elisp lists."
-  (unless (stringp str)
-    (error "`org-babel-script-escape' expects a string"))
-  (let ((escaped
-	 (cond
-	  ((and (> (length str) 2)
-		(or (and (string-equal "[" (substring str 0 1))
-			 (string-equal "]" (substring str -1)))
-		    (and (string-equal "{" (substring str 0 1))
-			 (string-equal "}" (substring str -1)))
-		    (and (string-equal "(" (substring str 0 1))
-			 (string-equal ")" (substring str -1)))))
-
-	   (concat "'" (org-babel--script-escape-inner str)))
-	  ((or force
-	       (and (> (length str) 2)
-		    (or (and (string-equal "'" (substring str 0 1))
-			     (string-equal "'" (substring str -1)))
-			;; We need to pass double-quoted strings
-			;; through the backslash-twiddling bits, even
-			;; though we don't need to change their
-			;; delimiters.
-			(and (string-equal "\"" (substring str 0 1))
-			     (string-equal "\"" (substring str -1))))))
-	   (org-babel--script-escape-inner str))
-	  (t str))))
-    (condition-case nil (org-babel-read escaped) (error escaped))))
+(defun org-babel-script-escape (val &optional force)
+  "Safely convert passed in values (including collections of
+them; tables) into elisp lists."
+  (if (numberp val)
+      val
+    (unless (stringp val)
+      (error "`org-babel-script-escape' expects a string or number"))
+    (let ((escaped
+	   (cond
+	    ((and (> (length val) 2)
+		  (or (and (string-equal "[" (substring val 0 1))
+			   (string-equal "]" (substring val -1)))
+		      (and (string-equal "{" (substring val 0 1))
+			   (string-equal "}" (substring val -1)))
+		      (and (string-equal "(" (substring val 0 1))
+			   (string-equal ")" (substring val -1)))))
+
+	     (concat "'" (org-babel--script-escape-inner val)))
+	    ((or force
+		 (and (> (length val) 2)
+		      (or (and (string-equal "'" (substring val 0 1))
+			       (string-equal "'" (substring val -1)))
+			  ;; We need to pass double-quoted strings
+			  ;; through the backslash-twiddling bits, even
+			  ;; though we don't need to change their
+			  ;; delimiters.
+			  (and (string-equal "\"" (substring val 0 1))
+			       (string-equal "\"" (substring val -1))))))
+	     (org-babel--script-escape-inner val))
+	    (t val))))
+      (condition-case nil (org-babel-read escaped) (error escaped)))))
 
 (defun org-babel-read (cell &optional inhibit-lisp-eval)
   "Convert the string value of CELL to a number if appropriate.

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: ob-ruby org-babel-ruby-table-or-string numeric argument patch
  2015-08-10 18:00 ob-ruby org-babel-ruby-table-or-string numeric argument patch Matthew MacLean
@ 2015-08-16 17:20 ` Nicolas Goaziou
  2015-08-16 19:28   ` Kyle Meyer
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Goaziou @ 2015-08-16 17:20 UTC (permalink / raw)
  To: Matthew MacLean; +Cc: emacs-orgmode

Hello,

Matthew MacLean <archenoth@gmail.com> writes:

> Could you review my patch? Thanks..!

Thank you for the patch.

> diff --git a/lisp/ob-core.el b/lisp/ob-core.el
> index e3abe97..01c4da8 100644
> --- a/lisp/ob-core.el
> +++ b/lisp/ob-core.el
> @@ -2813,34 +2813,37 @@ block but are passed literally to the \"example-block\"."
>        (error "Unterminated string in `org-babel-script-escape'"))
>      (apply #'string (reverse out))))
>  
> -(defun org-babel-script-escape (str &optional force)
> -  "Safely convert tables into elisp lists."
> -  (unless (stringp str)
> -    (error "`org-babel-script-escape' expects a string"))
> -  (let ((escaped
> -	 (cond
> -	  ((and (> (length str) 2)
> -		(or (and (string-equal "[" (substring str 0 1))
> -			 (string-equal "]" (substring str -1)))
> -		    (and (string-equal "{" (substring str 0 1))
> -			 (string-equal "}" (substring str -1)))
> -		    (and (string-equal "(" (substring str 0 1))
> -			 (string-equal ")" (substring str -1)))))
> -
> -	   (concat "'" (org-babel--script-escape-inner str)))
> -	  ((or force
> -	       (and (> (length str) 2)
> -		    (or (and (string-equal "'" (substring str 0 1))
> -			     (string-equal "'" (substring str -1)))
> -			;; We need to pass double-quoted strings
> -			;; through the backslash-twiddling bits, even
> -			;; though we don't need to change their
> -			;; delimiters.
> -			(and (string-equal "\"" (substring str 0 1))
> -			     (string-equal "\"" (substring str -1))))))
> -	   (org-babel--script-escape-inner str))
> -	  (t str))))
> -    (condition-case nil (org-babel-read escaped) (error escaped))))
> +(defun org-babel-script-escape (val &optional force)
> +  "Safely convert passed in values (including collections of
> +them; tables) into elisp lists."

First sentence in docstring has to fit in a single line.

> +  (if (numberp val)
> +      val
> +    (unless (stringp val)
> +      (error "`org-babel-script-escape' expects a string or number"))
> +    (let ((escaped

Arguably, a `cond' might look nicer:

  (cond ((numberp val) val)
        ((not (stringp val) (error ...)))
        (t (let ((escaped ...)) ...)))

Could you add a test for that in `test-org-babel/script-escape'
(test-ob.el)?


Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: ob-ruby org-babel-ruby-table-or-string numeric argument patch
  2015-08-16 17:20 ` Nicolas Goaziou
@ 2015-08-16 19:28   ` Kyle Meyer
  2015-08-16 19:44     ` Matthew MacLean
  0 siblings, 1 reply; 4+ messages in thread
From: Kyle Meyer @ 2015-08-16 19:28 UTC (permalink / raw)
  To: Matthew MacLean; +Cc: emacs-orgmode

Hi Nicolas,

This is a duplicate that has finally come through:

http://thread.gmane.org/gmane.emacs.orgmode/99888


--
Kyle

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: ob-ruby org-babel-ruby-table-or-string numeric argument patch
  2015-08-16 19:28   ` Kyle Meyer
@ 2015-08-16 19:44     ` Matthew MacLean
  0 siblings, 0 replies; 4+ messages in thread
From: Matthew MacLean @ 2015-08-16 19:44 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: emacs-orgmode

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

Yep..! Guess I know that unsubscribed mail doesn't actually go to /dev/null
like I heard it might.

The end result was commit 81a63729473baaf00c2548d8929b9388053aea05 sans the
test from http://thread.gmane.org/gmane.emacs.orgmode/99888 after a little
bit of "patch bootcamp" for someone who read README_contribute and didn't
see the relevant section on Worg.

That said, thanks for the review Nicolas--I'll keep it in mind if I send
another patch sometime.


On Sun, Aug 16, 2015 at 1:28 PM, Kyle Meyer <kyle@kyleam.com> wrote:

> Hi Nicolas,
>
> This is a duplicate that has finally come through:
>
> http://thread.gmane.org/gmane.emacs.orgmode/99888
>
>
> --
> Kyle
>

[-- Attachment #2: Type: text/html, Size: 1260 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-08-16 19:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-10 18:00 ob-ruby org-babel-ruby-table-or-string numeric argument patch Matthew MacLean
2015-08-16 17:20 ` Nicolas Goaziou
2015-08-16 19:28   ` Kyle Meyer
2015-08-16 19:44     ` Matthew MacLean

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