Grant Shoshin Shangreaux <[email protected]> writes:

> I came across an issue with org-babel evaluation of Ruby blocks with
> :results `output`

Thanks for the patch!
Using inf-ruby capabilities should indeed be better.

> I have added a test, you can see the failure result here:
>
> https://builds.sr.ht/~shoshin/job/1754555

After applying your patch onto main, I am getting even more tests failing,
so something is still off.

   FAILED  test-ob-ruby/session-output-1  ((should (equal 
(org-test-with-temp-text "#+begin_src ruby :session org-test-ruby :results 
output\ns = \"1\"\ns = \"2\"\ns = \"3\"\nputs s\ns = \"4\"\n#+end_src" 
(org-babel-execute-maybe) (substring-no-properties (buffer-string))) 
"#+begin_src ruby :session org-test-ruby :results output\ns = \"1\"\ns = 
\"2\"\ns = \"3\"\nputs s\ns = \"4\"\n#+end_src\n\n#+RESULTS:\n: 3\n")) :form 
(equal "#+begin_src ruby :session org-test-ruby :results output\ns = \"1\"\ns = 
\"2\"\ns = \"3\"\nputs s\ns = \"4\"\n#+end_src\n\n#+RESULTS:\n: \n: \n: 3\n: 
:org_babel_ruby_eoe\n" "#+begin_src ruby :session org-test-ruby :results 
output\ns = \"1\"\ns = \"2\"\ns = \"3\"\nputs s\ns = 
\"4\"\n#+end_src\n\n#+RESULTS:\n: 3\n") :value nil :explanation 
(arrays-of-different-length 149 121 "#+begin_src ruby :session org-test-ruby 
:results output\ns = \"1\"\ns = \"2\"\ns = \"3\"\nputs s\ns = 
\"4\"\n#+end_src\n\n#+RESULTS:\n: \n: \n: 3\n: :org_babel_ruby_eoe\n" 
"#+begin_src ruby :session org-test-ruby :results output\ns = \"1\"\ns = 
\"2\"\ns = \"3\"\nputs s\ns = \"4\"\n#+end_src\n\n#+RESULTS:\n: 3\n" 
first-mismatch-at 119))
   FAILED  test-ob-ruby/session-output-2  ((should (equal 
(org-test-with-temp-text "#+begin_src ruby :session org-test-ruby :results 
output\nputs s\ns = \"5\"\n#+end_src" (org-babel-execute-maybe) 
(substring-no-properties (buffer-string))) "#+begin_src ruby :session 
org-test-ruby :results output\nputs s\ns = \"5\"\n#+end_src\n\n#+RESULTS:\n: 
4\n")) :form (equal "#+begin_src ruby :session org-test-ruby :results 
output\nputs s\ns = \"5\"\n#+end_src\n\n#+RESULTS:\n: 4\n: 
:org_babel_ruby_eoe\n" "#+begin_src ruby :session org-test-ruby :results 
output\nputs s\ns = \"5\"\n#+end_src\n\n#+RESULTS:\n: 4\n") :value nil 
:explanation (arrays-of-different-length 119 97 "#+begin_src ruby :session 
org-test-ruby :results output\nputs s\ns = \"5\"\n#+end_src\n\n#+RESULTS:\n: 
4\n: :org_babel_ruby_eoe\n" "#+begin_src ruby :session org-test-ruby :results 
output\nputs s\ns = \"5\"\n#+end_src\n\n#+RESULTS:\n: 4\n" first-mismatch-at 
97))
   FAILED  test-ob-ruby/session-output-5  ((should (equal 
(org-test-with-temp-text "#+begin_src ruby :session org-test-ruby :results 
output\nclass Foo\n  def moo\n    puts \"Goo\"\n  
end\nend\n\nFoo.new.moo\n#+end_src" (org-babel-execute-maybe) 
(substring-no-properties (buffer-string))) "#+begin_src ruby :session 
org-test-ruby :results output\nclass Foo\n  def moo\n    puts \"Goo\"\n  
end\nend\n\nFoo.new.moo\n#+end_src\n\n#+RESULTS:\n: Goo\n")) :form (equal 
"#+begin_src ruby :session org-test-ruby :results output\nclass Foo\n  def 
moo\n    puts \"Goo\"\n  end\nend\n\nFoo.new.moo\n#+end_src\n\n#+RESULTS:\n: 
Goo\n: :org_babel_ruby_eoe\n" "#+begin_src ruby :session org-test-ruby :results 
output\nclass Foo\n  def moo\n    puts \"Goo\"\n  
end\nend\n\nFoo.new.moo\n#+end_src\n\n#+RESULTS:\n: Goo\n") :value nil 
:explanation (arrays-of-different-length 164 142 "#+begin_src ruby :session 
org-test-ruby :results output\nclass Foo\n  def moo\n    puts \"Goo\"\n  
end\nend\n\nFoo.new.moo\n#+end_src\n\n#+RESULTS:\n: Goo\n: 
:org_babel_ruby_eoe\n" "#+begin_src ruby :session org-test-ruby :results 
output\nclass Foo\n  def moo\n    puts \"Goo\"\n  
end\nend\n\nFoo.new.moo\n#+end_src\n\n#+RESULTS:\n: Goo\n" first-mismatch-at 
142))

> I see that Org 9.8 adds some filter-prompt behavior that might make this
> patch moot.

In Org 9.8, we recently got one of the ruby tests failing as well, most
likely because of changes in the newest ruby behavior.

> I changed how the session is initiated as well, because ruby-send-string
> impacted the behavior of the prompt during initialization. I chose to
> clear the comint buffer after the prompt is configured, again to remove
> noise in the session buffer.

That makes sense.

> There's some side effects of the changes, like the contents of the
> session buffer are different. I'm not sure if the contents of the
> session are important or not, though it would be nice if the org source
> block session could be interacted with like any other inferior-ruby.

Could you elaborate?

> I am sure there is a better way to cleanly send code to the IRB
> inferior-ruby process, but I'm just beginning to understand how all the
> parts interact. Any guidance in general on org bable session behavior
> would be welcome.

In ob-R, in similar fashion, we use ess-send-string.

> I'd also like to improve the readability of ob-ruby.el, any guidance
> there is welcome too.

It is a pretty short file. Anything in particular you find not readable?

> Subject: [PATCH 1/3] [ob-ruby] Clean: factor out session and external eval
>  functions
>
> org-babel-ruby-evaluate is one large function, this extracts two
> separate functions.
> One for evaluating a code block in an external Ruby process, and
> one for session based evaluation.

We have a specific format for commit messages. Check out
https://orgmode.org/worg/org-contribute.html#commit-messages

Otherwise, I do not see anything obviously problematic in the patch
(other than tests failing).

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
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>

Reply via email to