On Thu, Nov 12, 2020 at 10:46 PM Kyle Meyer <[email protected]> wrote: > > ian martins writes: > > > Subject: [PATCH 2/2] ob-java.el: Allow for more whitespace in java code > > > > * lisp/ob-java.el (defconst *-re): Updated regexps to allow for more > > whitespace in the content of java code blocks, and removed some > > redundancies. > > Sorry, more change log nitpicking from me (which is even less fun to do > than other nitpicking because I dislike the practice of including change > logs in commit messages).
No problem. Gathering the list of changed names is easy (I use emacs). I thought the long list would make the entry less useful, but I can see how it makes it more searchable this way. Of course, people could search the code diffs instead and then the commit logs could just be written for people. > Please name each variable in full. Here's the relevant bit from the > guidelines that Emacs's CONTRIBUTE points to: > > If you mention the names of the modified functions or variables, > it’s important to name them in full. Don’t abbreviate function or > variable names, and don’t combine them. Subsequent maintainers will > often search for a function name to find all the change log entries > that pertain to it; if you abbreviate the name, they won’t find it > when they search. > > https://www.gnu.org/prep/standards/html_node/Style-of-Change-Logs.html#Style-of-Change-Logs > > We should probably link to that in worg's org-contribute.org. Thanks for providing the reference. I'll add a link to worg if there isn't one. > > * testing/lisp/test-ob-java.el (ob-java/simple-with-main-whitespace): > > Added test case with lots of whitespace. > > Is this related to Jarmo's report at > <https://orgmode.org/list/[email protected]>? If so, it'd be good > to include a Reported-by trailer as well as a link. Yes. The updated patch includes Reported-by. That is just text in the commit message, not a git option, right? > > -(defconst org-babel-java--package-re > > "^[[:space:]]*package[[:space:]]+\\\([[:alnum:]_\.]+\\\);$" > > +(defconst org-babel-java--package-re > > "^[[:space:]]*package[[:space:]]+\\\([[:alnum:]_\.]+\\\)[[:space:]]*;$" > > "Regexp for the package statement.") > > -(defconst org-babel-java--imports-re > > "^[[:space:]]*import[[:space:]]+\\\([[:alnum:]_\.]+\\\);$" > > +(defconst org-babel-java--imports-re > > "^[[:space:]]*import[[:space:]]+\\\([[:alnum:]_\.]+\\\)[[:space:]]*;$" > > "Regexp for import statements.") > > -(defconst org-babel-java--class-re > > "^[[:space:]]*\\\(?:public[[:space:]]+\\\)?class[[:space:]]+\\\([[:alnum:]_]+\\\)[[:space:]]*\n?[[:space:]]*{" > > +(defconst org-babel-java--class-re > > "^[[:space:]]*\\\(?:public[[:space:]]+\\\)?class[[:space:]]+\\\([[:alnum:]_]+\\\)[[:space:]]*{" > > "Regexp for the class declaration.") > > -(defconst org-babel-java--main-re "public static void > > main(String\\\(?:\\[]\\\)?[[:space:]]+[^ > > ]+\\\(?:\\[]\\\)?).*\n?[[:space:]]*{" > > +(defconst org-babel-java--main-re > > "public[[:space:]]+static[[:space:]]+void[[:space:]]+main[[:space:]]*([[:space:]]*String[[:space:]]*.*[[:space:]]*)[[:space:]]*.*[[:space:]]*{" > > "Regexp for the main method declaration.") > > -(defconst org-babel-java--any-method-re "public .*(.*).*\n?[[:space:]]*{" > > +(defconst org-babel-java--any-method-re > > "public[[:space:]]+.*[[:space:]]*([[:space:]]*.*[[:space:]]*)[[:space:]]*.*[[:space:]]*{" > > "Regexp for any method.") > > Not speaking Java, I don't have anything actually valuable to say about > this change, but I wouldn't complain if these regular expressions were > switched over to rx (or at least tamed a bit in terms of line length). Thanks for the suggestion. I hadn't seen `rx' before. It's beautiful. I converted it. That was a joy.
From 4784ea1e926da014e30bbbaa241b3779a14119f4 Mon Sep 17 00:00:00 2001 From: Ian Martins <[email protected]> Date: Thu, 12 Nov 2020 05:18:48 -0500 Subject: [PATCH 2/2] ob-java.el: Allow for more whitespace in java code * lisp/ob-java.el (org-babel-java--package-re) (org-babel-java--imports-re, org-babel-java--class-re) (org-babel-java--main-re, org-babel-java--any-method-re): Updated regexps to allow for more whitespace in the content of java code blocks. Convert regexps to `rx' to improve clarity. * testing/lisp/test-ob-java.el (ob-java/simple-with-main-whitespace): Added test case with excessive whitespace. Reported-by: Jarmo Hurri <[email protected]> <https://orgmode.org/list/[email protected]> --- lisp/ob-java.el | 35 ++++++++++++++++++++++++++++++----- testing/lisp/test-ob-java.el | 18 ++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/lisp/ob-java.el b/lisp/ob-java.el index 92e873f0d..4cf80433f 100644 --- a/lisp/ob-java.el +++ b/lisp/ob-java.el @@ -77,15 +77,40 @@ like javac -verbose." :package-version '(Org . "9.5") :type 'symbol) -(defconst org-babel-java--package-re "^[[:space:]]*package[[:space:]]+\\\([[:alnum:]_\.]+\\\);$" +(defconst org-babel-java--package-re (rx line-start (0+ space) "package" + (1+ space) (group (1+ (in alnum ?_ ?.))) ; capture the package name + (0+ space) ?\; line-end) "Regexp for the package statement.") -(defconst org-babel-java--imports-re "^[[:space:]]*import[[:space:]]+\\\([[:alnum:]_\.]+\\\);$" +(defconst org-babel-java--imports-re (rx line-start (0+ space) "import" + (1+ space) (group (1+ (in alnum ?_ ?.))) ; capture the fully qualified class name + (0+ space) ?\; line-end) "Regexp for import statements.") -(defconst org-babel-java--class-re "^[[:space:]]*\\\(?:public[[:space:]]+\\\)?class[[:space:]]+\\\([[:alnum:]_]+\\\)[[:space:]]*\n?[[:space:]]*{" +(defconst org-babel-java--class-re (rx line-start (0+ space) (opt (seq "public" (1+ space))) + "class" (1+ space) + (group (1+ (in alnum ?_))) ; capture the class name + (0+ space) ?{) "Regexp for the class declaration.") -(defconst org-babel-java--main-re "public static void main(String\\\(?:\\[]\\\)?[[:space:]]+[^ ]+\\\(?:\\[]\\\)?).*\n?[[:space:]]*{" +(defconst org-babel-java--main-re (rx line-start (0+ space) "public" + (1+ space) "static" + (1+ space) "void" + (1+ space) "main" + (0+ space) ?\( + (0+ space) "String" + (0+ space) (1+ (in alnum ?_ ?\[ ?\] space)) ; "[] args" or "args[]" + (0+ space) ?\) + (0+ space) (opt "throws" (1+ (in alnum ?_ ?, ?. space))) + ?{) "Regexp for the main method declaration.") -(defconst org-babel-java--any-method-re "public .*(.*).*\n?[[:space:]]*{" +(defconst org-babel-java--any-method-re (rx line-start + (0+ space) (opt (seq (1+ alnum) (1+ space))) ; visibility + (opt (seq "static" (1+ space))) ; binding + (1+ (in alnum ?_ ?\[ ?\])) ; return type + (1+ space) (1+ (in alnum ?_)) ; method name + (0+ space) ?\( + (0+ space) (0+ (in alnum ?_ ?\[ ?\] ?, space)) ; params + (0+ space) ?\) + (0+ space) (opt "throws" (1+ (in alnum ?_ ?, ?. space))) + ?{) "Regexp for any method.") (defconst org-babel-java--result-wrapper "\n public static String __toString(Object val) { if (val instanceof String) { diff --git a/testing/lisp/test-ob-java.el b/testing/lisp/test-ob-java.el index e14975412..50d3ef5b0 100644 --- a/testing/lisp/test-ob-java.el +++ b/testing/lisp/test-ob-java.el @@ -128,6 +128,24 @@ public static void main(String args[]) { #+end_src" (should (string= "42" (org-babel-execute-src-block))))) +(ert-deftest ob-java/simple-with-main-whitespace () + "Hello world program that defines a main function with the square brackets after `args'." + (org-test-with-temp-text + "#+begin_src java :results output silent +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 () "Hello world program that defines a class." (org-test-with-temp-text -- 2.25.1
