-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
turn string tags to regexes #30
Conversation
private static final String END_SOLUTION_TAG = "// END SOLUTION"; | ||
private static final String STUB_TAG = "// STUB:"; | ||
private static final String SOLUTION_FILE_TAG = "// SOLUTION FILE"; | ||
private static final String BEGIN_SOLUTION_REGEX = ".*\\/\\/[ \\t]?BEGIN[ \\t]SOLUTION.*"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[ \\t]
seems like a smart choice since \s
contains \n
, \r
etc. and we don't really want to match those. Is there a specific reason to match for ?
instead of *
occurrences?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ?'s were relics from my first version of the regex. Changed them and added + to the spaces between words, to make sure there always is a space but not take stand on how many.
Should probably add some new test cases to Other than that (and the single note above), LGTM. |
I updated the regexes a bit and added test cases for them. |
private static final String BEGIN_SOLUTION_REGEX = ".*\\/\\/[ \\t]*BEGIN[ \\t]+SOLUTION.*"; | ||
private static final String END_SOLUTION_REGEX = ".*\\/\\/[ \\t]*END[ \\t]+SOLUTION.*"; | ||
private static final String SOLUTION_FILE_REGEX = ".*\\/\\/[ \\t]*SOLUTION[ \\t]+FILE.*"; | ||
private static final String STUB_REGEX = "(.*)\\/\\/[ \\t]*STUB:[ \\t]*(.*)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if we want to have the .*
in the beginning, maybe we want just to allow having whitespace before these tags?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I just changed it.
@@ -81,7 +81,12 @@ public void solutionFilesAreIgnoredFromStub() throws IOException { | |||
temp.toFile().deleteOnExit(); | |||
exerciseBuilder.prepareStub(temp); | |||
Path solutionFile = temp.resolve(Paths.get("src", "SolutionFile.java")); | |||
Path solutionFile2 = temp.resolve(Paths.get("src", "SolutionFileWithNoSpace.java")); | |||
Path solutionFile3 = temp.resolve(Paths.get("src", "SolutionFileWithExtraSpaces.java")); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty row
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because it makes the test more readable.
Use regex for detecting BEGIN/END SOLUTION and STUBs
This will fix #25