Re: [PATCH] ob-java

[ian martins <ianxm@jhu.edu>]

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

Actually I realized if I keep the commits separate and generate a patch set
instead of squashing then I can preserve authorship.

These patches, which follow patch 0001, fix the spacing and allow
non-public classes.

Thanks again for testing, debugging, and reporting.

On Wed, Oct 21, 2020 at 9:54 AM John Herrlin <jherrlin@gmail.com> wrote:

>
> ian martins <ianxm@jhu.edu> writes:
>
> >>
> >> What do you think about having a configurable list where the user can
> >> add =org-babel-java--import-maybe=? In my current use case I could then
> >> add RxJava imports to that list and the imports could be removed from
> >> the source code block.
> >
> >
> > I think this can already be done. imports can be added to the headers,
> and
> > babel allows file-wide headers, so you could add a =#+HEADER: :import
> > rx.Observable= line to the file and all source blocks would get it.  it's
> > slightly different in that =org-babel-java--import-maybe= skips imports
> > that it thinks aren't needed. also if there are any non-java source
> blocks
> > in the same file, these imports could be added to them which would be
> bad,
> > so when mixing multiple languages in the same file this wouldn't be an
> > option.
>
> Thanks for pointing that out! It work just fine!
>
> >
> > NIT
> >> Some spacing when writing =public static...=
> >>
> >
> > Thanks for fixing the spacing. I don't think I can give you credit for
> the
> > patch, though, without leaving it out until ob-java is accepted.
>
> I dont need any credits, the important part is the result!
>
> I have made a couple of more runs and I cant find anything that doesnt
> work!
>
> >
> > On Wed, Oct 21, 2020 at 1:59 AM John Herrlin <jherrlin@gmail.com> wrote:
> >
> >>
> >> I did and it looks really good. The difference in this example:
> >>
> >>     #+BEGIN_SRC java
> >>       import rx.Observable;
> >>
> >>       Observable.range(5, 3)
> >>           .subscribe((Integer i) ->   { System.out.println("Got: " +
> i); },
> >>                      (Throwable t) -> { t.printStackTrace();},
> >>                      () ->            { System.out.println("Ending
> >> stream"); });
> >>     #+END_SRC
> >>
> >> from the ones I posted yesterday is tremendous!
> >>
> >> I am not very experienced with Emacs lisp but I think it's pretty easy
> >> to understand how things works and follow the code. The comments are
> >> also of good help. I really appreciate the work you have done!
> >>
> >>
> >> What do you think about having a configurable list where the user can
> >> add =org-babel-java--import-maybe=? In my current use case I could then
> >> add RxJava imports to that list and the imports could be removed from
> >> the source code block.
> >>
> >>
> >> NIT
> >>
> >> Some spacing when writing =public static...=
> >>
> >>    #+BEGIN_SRC diff
> >>      diff --git a/lisp/ob-java.el b/lisp/ob-java.el
> >>      index 94c3f69cf..4f3904871 100644
> >>      --- a/lisp/ob-java.el
> >>      +++ b/lisp/ob-java.el
> >>      @@ -220,7 +220,7 @@ RESULT-FILE is the temp file to write the
> result."
> >>             (org-babel-java--move-past org-babel-java--class-re)
> >>             (insert "\n    public static void main(String[] args) {
> >>               System.out.print(\"success\");
> >>      -}\n\n"))
> >>      +    }\n\n"))
> >>
> >>           ;; special handling to return value
> >>           (when (eq result-type 'value)
> >>    #+END_SRC
> >>
> >>
> >>
> >> ian martins <ianxm@jhu.edu> writes:
> >>
> >> > Thanks for testing, and thanks for pointing that out. I will fix it so
> >> that
> >> > `public` is optional.
> >> >
> >> > btw, in your example you didn't have to specify `:classname` since you
> >> > defined the class name in the source block.
> >> >
> >> > btw2, did you notice that you can C-c C-c on source blocks that don't
> >> have
> >> > main methods and it'll compile without error?
> >> >
> >> > On Tue, Oct 20, 2020 at 3:17 PM John Herrlin <jherrlin@gmail.com>
> wrote:
> >> >
> >> >>
> >> >> Hey,
> >> >>
> >> >> Did some debugging and found out that my class didn't contained
> =public=
> >> >> and the patch requires it to be.
> >> >>
> >> >> This works fine:
> >> >>
> >> >>    #+HEADER: :classname Main
> >> >>    #+HEADER: :dir src
> >> >>    #+HEADER: :cmdline -classpath ./rxjava-1.3.8.jar:.
> >> >>    #+HEADER: :cmpflag -classpath ./rxjava-1.3.8.jar
> >> >>    #+BEGIN_SRC java :results output code
> >> >>      import rx.Observable;
> >> >>      public class Main {
> >> >>          public static void main(String[] args) {
> >> >>              Observable.range(5, 5)
> >> >>                  .subscribe(System.out::println);
> >> >>          }
> >> >>      }
> >> >>    #+END_SRC
> >> >>
> >> >>
> >> >>
> >> >>
> >> >> ian martins <ianxm@jhu.edu> writes:
> >> >>
> >> >> > I noticed that the tests didn't run with "make test." This updates
> the
> >> >> > patch so that they can. I didn't add java to the list of default
> >> >> languages
> >> >> > because the java tests are slow.
> >> >> >
> >> >> > On Mon, Oct 5, 2020 at 9:23 AM ian martins <ianxm@jhu.edu> wrote:
> >> >> >
> >> >> >> I wrote those examples in an org file so I could test as I wrote
> >> them,
> >> >> and
> >> >> >> then exported it to make it more readable, but the export
> resulted in
> >> >> >> source block headers being lost.  Here is the same without export:
> >> >> >> ----
> >> >> >> * Changes
> >> >> >>
> >> >> >> - support for functional mode (~:results value~)
> >> >> >> - accept variables
> >> >> >> - don't require package, class, and main definitions
> >> >> >> - write source and result tempfiles to
> >> ~org-babel-temporary-directory~,
> >> >> >> but respects the ~:dir~ header
> >> >> >> - work with tramp
> >> >> >>
> >> >> >> * Examples
> >> >> >> ** Example 1
> >> >> >> This outputs "hello."  If class and main definitions aren't given
> the
> >> >> >> code block will be wrapped in generic ones.
> >> >> >>
> >> >> >> #+begin_src java :results output silent
> >> >> >>   System.out.print("hello");
> >> >> >> #+end_src
> >> >> >>
> >> >> >> This is exactly equivalent:
> >> >> >>
> >> >> >> #+begin_src java :results output silent
> >> >> >>   public class Main {
> >> >> >>       public static void main(String[] args) {
> >> >> >>           System.out.print("hello");
> >> >> >>       }
> >> >> >>   }
> >> >> >> #+end_src
> >> >> >>
> >> >> >> ** Example 2
> >> >> >> This also outputs "hello."
> >> >> >>
> >> >> >> #+begin_src java :results value silent
> >> >> >>   return "hello";
> >> >> >> #+end_src
> >> >> >>
> >> >> >> ** Example 3
> >> >> >> This generates the class "Example" in the package "org.orgmode" in
> >> the
> >> >> >> current directory.
> >> >> >>
> >> >> >> #+begin_src java :results output silent :classname
> >> org.orgmode.Example
> >> >> >> :dir .
> >> >> >>   System.out.print("hello, org-mode");
> >> >> >> #+end_src
> >> >> >>
> >> >> >> ** Example 4
> >> >> >> The "Hey" class defines a static method but no main. C-c C-c on
> the
> >> >> >> "Hey" source block will write "./org/orgmode/Hey.java" and compile
> >> it.
> >> >> >>
> >> >> >> The "Main" class calls the "Hey" class. C-c C-c on the "Main"
> source
> >> >> >> block will write "./org/orgmode/Main.java" and compile and run it.
> >> >> >>
> >> >> >> #+begin_src java :results output silent :dir .
> >> >> >>   package org.orgmode;
> >> >> >>
> >> >> >>   public class Hey {
> >> >> >>       public static String say() {
> >> >> >>           return "hey";
> >> >> >>       }
> >> >> >>   }
> >> >> >> #+end_src
> >> >> >>
> >> >> >> #+begin_src java :results output silent :dir .
> >> >> >>   package org.orgmode;
> >> >> >>
> >> >> >>   public class Main {
> >> >> >>       public static void main(String[] args) {
> >> >> >>           System.out.print(Hey.say());
> >> >> >>       }
> >> >> >>   }
> >> >> >> #+end_src
> >> >> >>
> >> >> >> Instead of C-c C-c, we could have added tangle headers and written
> >> the
> >> >> >> source files out by tangling.
> >> >> >>
> >> >> >> ** Example 5
> >> >> >> This prints the variable from the header
> >> >> >>
> >> >> >> #+begin_src java :var msg="hello, org-mode" :results output silent
> >> >> >>   System.out.print(msg);
> >> >> >> #+end_src
> >> >> >>
> >> >> >> ** Example 6
> >> >> >> This prints "hello, org-mode." The table is provided to the method
> >> as a
> >> >> >> list of lists.
> >> >> >>
> >> >> >> #+name: table
> >> >> >> | message | hello, org-mode  |
> >> >> >>
> >> >> >> #+begin_src java :var tbl=table :results output silent
> >> >> >>   System.out.print(tbl.get(0).get(1));
> >> >> >> #+end_src
> >> >> >>
> >> >> >> ** Example 7
> >> >> >> This example returns a list.
> >> >> >>
> >> >> >> Note that you're allowed to specify imports without defining the
> >> class
> >> >> >> or main methods.
> >> >> >>
> >> >> >> #+begin_src java :results value :exports both
> >> >> >>   import java.util.Arrays;
> >> >> >>
> >> >> >>   return Arrays.asList("message", "hello, org-mode");
> >> >> >> #+end_src
> >> >> >>
> >> >> >> #+RESULTS:
> >> >> >> | message | hello, org-mode |
> >> >> >>
> >> >> >> On Mon, Oct 5, 2020 at 8:35 AM ian martins <ianxm@jhu.edu> wrote:
> >> >> >>
> >> >> >>> 1 Changes
> >> >> >>> =========
> >> >> >>>
> >> >> >>>   - support for functional mode (`:results value')
> >> >> >>>   - accept variables
> >> >> >>>   - don't require package, class, and main definitions
> >> >> >>>   - write source and result tempfiles to
> >> >> >>>     `org-babel-temporary-directory', but respects the `:dir'
> header
> >> >> >>>   - work with tramp
> >> >> >>>
> >> >> >>>
> >> >> >>> 2 Examples
> >> >> >>> ==========
> >> >> >>> Some examples follow.  See the tests for more examples.  I'll
> write
> >> >> >>> proper docs after review.
> >> >> >>>
> >> >> >>> 2.1 Example 1
> >> >> >>> ~~~~~~~~~~~~~
> >> >> >>>
> >> >> >>>   This outputs "hello."  If class and main definitions aren't
> given
> >> the
> >> >> >>>   code block will be wrapped in generic ones.
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | System.out.print("hello");
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>   This is exactly equivalent:
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | public class Main {
> >> >> >>>   |     public static void main(String[] args) {
> >> >> >>>   |         System.out.print("hello");
> >> >> >>>   |     }
> >> >> >>>   | }
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>
> >> >> >>> 2.2 Example 2
> >> >> >>> ~~~~~~~~~~~~~
> >> >> >>>
> >> >> >>>   This also outputs "hello."
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | return "hello";
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>
> >> >> >>> 2.3 Example 3
> >> >> >>> ~~~~~~~~~~~~~
> >> >> >>>
> >> >> >>>   This generates the class "Example" in the package
> "org.orgmode" in
> >> >> the
> >> >> >>>   current directory.
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | System.out.print("hello, org-mode");
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>
> >> >> >>> 2.4 Example 4
> >> >> >>> ~~~~~~~~~~~~~
> >> >> >>>
> >> >> >>>   The "Hey" class defines a static method but no main. C-c C-c on
> >> the
> >> >> >>>   "Hey" source block will write "./org/orgmode/Hey.java" and
> compile
> >> >> it.
> >> >> >>>
> >> >> >>>   The "Main" class calls the "Hey" class. C-c C-c on the "Main"
> >> source
> >> >> >>>   block will write "./org/orgmode/Main.java" and compile and run
> it.
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | package org.orgmode;
> >> >> >>>   |
> >> >> >>>   | public class Hey {
> >> >> >>>   |     public static String say() {
> >> >> >>>   |         return "hey";
> >> >> >>>   |     }
> >> >> >>>   | }
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | package org.orgmode;
> >> >> >>>   |
> >> >> >>>   | public class Main {
> >> >> >>>   |     public static void main(String[] args) {
> >> >> >>>   |         System.out.print(Hey.say());
> >> >> >>>   |     }
> >> >> >>>   | }
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>   Instead of C-c C-c, we could have added tangle headers and
> written
> >> >> the
> >> >> >>>   source files out by tangling.
> >> >> >>>
> >> >> >>>
> >> >> >>> 2.5 Example 5
> >> >> >>> ~~~~~~~~~~~~~
> >> >> >>>
> >> >> >>>   This prints the variable from the header
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | System.out.print(msg);
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>
> >> >> >>> 2.6 Example 6
> >> >> >>> ~~~~~~~~~~~~~
> >> >> >>>
> >> >> >>>   This prints "hello, org-mode." The table is provided to the
> >> method as
> >> >> >>>   a list of lists.
> >> >> >>>
> >> >> >>>    message  hello, org-mode
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | System.out.print(tbl.get(0).get(1));
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>
> >> >> >>> 2.7 Example 7
> >> >> >>> ~~~~~~~~~~~~~
> >> >> >>>
> >> >> >>>   This example returns a list.
> >> >> >>>
> >> >> >>>   Note that you're allowed to specify imports without defining
> the
> >> >> class
> >> >> >>>   or main methods.
> >> >> >>>
> >> >> >>>   ,----
> >> >> >>>   | import java.util.Arrays;
> >> >> >>>   |
> >> >> >>>   | return Arrays.asList("message", "hello, org-mode");
> >> >> >>>   `----
> >> >> >>>
> >> >> >>>    message  hello, org-mode
> >> >> >>>
> >> >> >>
> >> >>
> >>
>

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

[-- Attachment #2: 0003-ob-java.el-Allow-non-public-classes.patch --]
[-- Type: text/x-patch, Size: 3253 bytes --]

From 0f754613f7b9616e6cc1c2bd3b61a6552796ea9d Mon Sep 17 00:00:00 2001
From: Ian Martins <ianxm@jhu.edu>
Date: Thu, 22 Oct 2020 06:55:59 -0400
Subject: [PATCH 3/3] ob-java.el: Allow non-public classes

* lisp/ob-java.el: Don't require class definitions to be declared
public.

* testing/lisp/test-ob-java.el: Add test with non-public class.
---
 lisp/ob-java.el              | 12 ++++++------
 testing/lisp/test-ob-java.el | 12 ++++++++++++
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/lisp/ob-java.el b/lisp/ob-java.el
index 65b8c3eba..eaeb78844 100644
--- a/lisp/ob-java.el
+++ b/lisp/ob-java.el
@@ -125,22 +125,22 @@
 Look through BODY for the package and class.  If found, put them
 together into a fully qualified class name and return.  Else just
 return class name.  If that isn't found either, default to Main."
-  (let ((package (if (string-match "package \\\([^ ]*\\\);" body)
+  (let ((package (if (string-match org-babel-java--package-re body)
                      (match-string 1 body)))
-        (class (if (string-match "public class \\\([^ \n]*\\\)" body)
+        (class (if (string-match org-babel-java--class-re body)
                    (match-string 1 body))))
     (or (and package class (concat package "." class))
         (and class class)
         (and package (concat package ".Main"))
         "Main")))
 
-(defconst org-babel-java--package-re "^[[:space:]]*package .*;$"
+(defconst org-babel-java--package-re "^[[:space:]]*package[[:space:]]+\\\([[:alnum:]_\.]+\\\);$"
   "Regexp for the package statement.")
-(defconst org-babel-java--imports-re "^[[:space:]]*import .*;$"
+(defconst org-babel-java--imports-re "^[[:space:]]*import[[:space:]]+\\\([[:alnum:]_\.]+\\\);$"
   "Regexp for import statements.")
-(defconst org-babel-java--class-re "^public class [[:alnum:]_]+[[:space:]]*\n?[[:space:]]*{"
+(defconst org-babel-java--class-re "^[[:space:]]*\\\(?:public[[:space:]]+\\\)?class[[:space:]]+\\\([[:alnum:]_]+\\\)[[:space:]]*\n?[[:space:]]*{"
   "Regexp for the class declaration.")
-(defconst org-babel-java--main-re "public static void main(String\\(?:\\[]\\)? args\\(?:\\[]\\)?).*\n?[[:space:]]*{"
+(defconst org-babel-java--main-re "public static void main(String\\\(?:\\[]\\\)?[[:space:]]+[^ ]+\\\(?:\\[]\\\)?).*\n?[[:space:]]*{"
   "Regexp for the main method declaration.")
 (defconst org-babel-java--any-method-re "public .*(.*).*\n?[[:space:]]*{"
   "Regexp for any method.")
diff --git a/testing/lisp/test-ob-java.el b/testing/lisp/test-ob-java.el
index 090c40084..cb03cadac 100644
--- a/testing/lisp/test-ob-java.el
+++ b/testing/lisp/test-ob-java.el
@@ -137,6 +137,18 @@ public class Simple {
 #+end_src"
    (should (string= "42" (org-babel-execute-src-block)))))
 
+(ert-deftest ob-java/simple-with-non-public-class ()
+  "Hello world program that defines a non-public class."
+  (org-test-with-temp-text
+      "#+begin_src java :results output silent
+class Simple {
+    public static void main(String[] args) {
+        System.out.print(42);
+    }
+}
+#+end_src"
+   (should (string= "42" (org-babel-execute-src-block)))))
+
 (ert-deftest ob-java/simple-with-class-and-package ()
   "Hello world program that defines a class and package."
   (org-test-with-temp-text
-- 
2.25.1


[-- Attachment #3: 0002-ob-java.el-Fix-spacing-in-generated-main-method.patch --]
[-- Type: text/x-patch, Size: 870 bytes --]

From 2b459d59d006c190c6b7513e17378b67c8162bf9 Mon Sep 17 00:00:00 2001
From: John Herrlin <jherrlin@gmail.com>
Date: Thu, 22 Oct 2020 06:52:22 -0400
Subject: [PATCH 2/3] ob-java.el: Fix spacing in generated main method

* lisp/ob-java.el: Correct Indentation of closing curly braces when
the default main method is used.
---
 lisp/ob-java.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/ob-java.el b/lisp/ob-java.el
index e704c5552..65b8c3eba 100644
--- a/lisp/ob-java.el
+++ b/lisp/ob-java.el
@@ -218,7 +218,7 @@ RESULT-FILE is the temp file to write the result."
       (org-babel-java--move-past org-babel-java--class-re)
       (insert "\n    public static void main(String[] args) {
         System.out.print(\"success\");
-}\n\n"))
+    }\n\n"))
 
     ;; special handling to return value
     (when (eq result-type 'value)
-- 
2.25.1