What change would like to see?
Migrating over to Java's built-in regular expressions instead of using oro.jar. Ask Eberhard for help.
Why?
So we don't need to continue to include an obsolete regular expression engine.
Would this cause any incompatibilities with previous versions? If so, how can these be mitigated?
Comment #1
Posted on Jan 9, 2010 by Massive LionHi,
I have been playing with the code a bit, trying to make more suitable for creating an ubuntu package. In order to get rid of the "oro.jar" I had to rewrite processing.app.preproc.PdePreprocessor.
The only noticeable change would be that I created a local version of the scrubComments(String) method. The original code calls Sketch.scrubComments(String). But nothing else uses Sketch.scrubComments(String) so why should this not be moved to the preprocessor class?
Eberhard
- PdePreprocessor.java 13.21KB
Comment #2
Posted on Jan 10, 2010 by Massive LionThe file I posted previously had 2 bugs.
1) It reported an IllegalArgumentException instead of a RuntimeException when the
user entered unbalanced /* ... */ multiline comments.
2) It had a problem generating the prototypes when there was a comma in a comment
just before the function definition
Both bugs are fixed in the attached file.
Eberhard
- PdePreprocessor.java 13.45KB
Comment #3
Posted on Jan 28, 2010 by Happy RhinoCan we put scrubComments() back in Sketch.java? This will make it easier for me to merge any corrections that Processing might make to the function, and to provide any changes we make back to Processing.
Also, it would be great to have more information about the tests you've run on this. Either the ones I was using before http://code.google.com/p/arduino/source/browse/#svn/branches/junit/app/preproc/test/data or others.
Comment #4
Posted on Feb 10, 2010 by Massive LionHere is a new version of the preprocessor. It now passes all the (valid) tests from
http://code.google.com/p/arduino/source/browse/#svn/branches/junit/app/preproc/test/data.
It fails on t14.cpp, but so does the current implementation!
I would leave scrubComments() in PdePreprocessor. We want a working preprocessor for Arduino Sketches. If procsessing updates the version in Sketch.scrubComments() it will very likely incompatible with the requirements of processing Arduino Sketches.
Eberhard
- PdePreprocessor.java 14.8KB
Comment #5
Posted on Feb 13, 2010 by Massive LionNew Version in reaction to issue 205 http://code.google.com/p/arduino/issues/detail?id=205
- PdePreprocessor.java 14.8KB
Comment #6
Posted on Feb 14, 2010 by Massive LionAnd another update caused by issue205
Functions can return references too!
- PdePreprocessor.java 14.64KB
Comment #7
Posted on Feb 19, 2010 by Happy RhinoAny chance you can submit a more streamlined patch (one that makes the switch from the old oro.jar regular expressions to java.regex ones, but without other modifications to the code)? The other changes may be useful, but make it more difficult to understand what's going on here. Maybe we can implement them separately?
Comment #8
Posted on Aug 11, 2010 by Happy Rhino(No comment was entered for this change.)
Comment #9
Posted on Oct 1, 2010 by Helpful Kangaroo@dmellis, you can get the patch in attachment, some notes:
1) I can confirm that the patch passes all tests in the old repository (except t14.cpp but i dont know if it's valid).
2) Eberhard make some changes in regexes related to comments:
// single and multi-line comment
// p += "|" + "(//\\s*?$)|(/\\*\\s*?\\*/)"; <- E. version
p += "|(//.*?$)|(/\\*[^*]*(?:\\*(?!/)[^*]*)*\\*/)"; <- actual
but seems that the actual version is the correct one, since the Eberhard one never matches comments.
3) Eberhard made another change in prototype:
"[\w\[\]\*]+\s+[&\[\]\*\w\s]+\([&,\[\]\*\w\s]\)(?=\s\{)" <- E. version "[\w\[\]\*]+\s+[\[\]\*\w\s]+\([,\[\]\*\w\s]\)(?=\s\{)" <- actual
he added the two amps that matches argument passed by reference like:
void func(MyStruct &var);
this one seems correct to me and I keeped it.
4) I've not tested the method firstStatement(...). It seems a recently added method that wasn't ported yet. I tried an implementation but is not tested and can (probably) fail. Can you give me some tests to check that?
- regexp.patch 7.79KB
Comment #10
Posted on Oct 1, 2010 by Helpful KangarooOk, use the one attached here, this patch solves also the firstStatement(...) issue. I checked with some examples (using the old method and the new) and both gives the same results.
- regexp.patch 7.75KB
Comment #11
Posted on Oct 4, 2010 by Happy RhinoChristian: looks good, I applied it: http://github.com/arduino/Arduino/commit/fa4d0582970e50c574ecfdcdbcfeae3754ca2095
Status: Fixed
Labels:
Type-Enhancement
Priority-Medium
Component-IDE
OpSys-All
Milestone-0022