[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ob-java.el: Allow for more whitespace
From: |
ian martins |
Subject: |
Re: [PATCH] ob-java.el: Allow for more whitespace |
Date: |
Sat, 14 Nov 2020 04:40:28 -0500 |
On Thu, Nov 12, 2020 at 10:46 PM Kyle Meyer <kyle@kyleam.com> 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
> <87o8k68w05.fsf@iki.fi">https://orgmode.org/list/87o8k68w05.fsf@iki.fi>? 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.
0002-ob-java.el-Allow-for-more-whitespace-in-java-code.patch
Description: Text Data