Auto script - #34
Conversation
|
I just have 1 question: For MissionScript, why does parenSplit split at '\(' and not '('? |
|
This is because http://stackoverflow.com/questions/3481828/how-to-split-a-string-in-java/3481842#3481842 |
|
Oh maybe we should have it split on a semicolon instead of a new line so you could write multiple on a single line. |
|
How about a semicolon or a newline? I do not think we have much of a need for multiple statements on one line and I do not want to require semicolons for this. |
|
Okay sounds good, feel free to make the change, or let me know if I should. |
|
I think it is fine without semicolons. |
|
Having a comment on a blank line breaks the parser. |
|
I think the best way to fix the comment without a preceding function function on that line is to use the presence of a |
|
Where does it break? I think you could fix it by checking if the line starts with // before line 12. I'm guessing it's trying to parse the comment? |
|
No, it breaks because there is no open parenthesis on that line. |
This is because the SmartDashboard does not support typing newlines in a text field
|
I have fixed the blank line issue. |
|
Oh okay sorry, yes you're right. |
| return null; | ||
| } | ||
|
|
||
| if(!(returnValue instanceof Command)){ |
There was a problem hiding this comment.
Instead of checking what is returned, we should check the function signature. This can be done with getReturnType().
There was a problem hiding this comment.
Having done more research, I think the best way is to use instanceof Command. By using instanceof Command, a subclass of Command will also be accepted. Also, null instanceof T is always false [1], so we can remove the null check.
| } catch (NumberFormatException e) { | ||
| System.err.println("Failed to parse parameter as boolean or double: \"" | ||
| + parameter + "\""); | ||
| return null; |
There was a problem hiding this comment.
We should throw an exception here.
A ParseException or rethrowing the NumberFormatException is probably appropriate.
| } else { | ||
| System.err.format("Too many opening parentheses in line %d: \"%s\"", lineIndex + 1, | ||
| lineWithoutComment); | ||
| return null; |
There was a problem hiding this comment.
Here is also a good place to throw an excerption.
Probably a ParseException.
|
|
||
| } catch (NoSuchMethodException e) { | ||
| System.err.println("Failed to find method: " + signature + ""); | ||
| return null; |
There was a problem hiding this comment.
Here is also a good place to throw an excerption.
Probably a ParseException, or maybe an IllegalArgumentException.
|
@nedearb Can you review this, specifically the exceptions. |
|
Looks fine, but have you tested it? |
|
A better way of integrating the MissionScript parser with our missions is to make a |
3 participants
No description provided.