AnnotationBear: Use unescaped_search_for #996
Conversation
|
Thanks for your contribution! Reviewing pull requests take really a lot of time and we're all volunteers. Please make sure you go through the following check list and complete them all before pinging someone for a review.
As you learn things over your Pull Request please help others on the chat and on PRs to get their stuff right as well! |
|
Your commit structure is not correct. see https://coala.io/commit. |
| @@ -4,6 +4,7 @@ | |||
| from coalib.results.Result import Result, RESULT_SEVERITY | |||
There was a problem hiding this comment.
Blank line before Fixes.. 😉
There was a problem hiding this comment.
ahh right, though this is a really awkward place for that review dont you think? ;) .
shouldn't it be on that main PR page?
There was a problem hiding this comment.
I saw many times review like this
There was a problem hiding this comment.
well a line note (although i dont know if they still call it that), should be for that line only right? though occasionally i guess its fine, so i'll leave it there :P
| text[position + 1:])) | ||
| except StopIteration: | ||
| end_match = None | ||
| print(end_match) |
There was a problem hiding this comment.
We need this? I don't know what should be printed when the bear is used.
| text[position + 1:])) | ||
| except StopIteration: | ||
| end_match = None | ||
| end_end = position + end_match.span()[1] if end_match else -1 |
There was a problem hiding this comment.
If end match is none, then this will give an error saying span() not found in None
There was a problem hiding this comment.
Sorry, nevermind. Just saw the if
There was a problem hiding this comment.
It would be easier to just put the end_end calculation inside the try/cath. Why do another if?
| unescaped_search_for("\n", text[position + 1:])) | ||
| except StopIteration: | ||
| newline_match = None | ||
| newline = position + newline_match.span()[1] if newline_match else -1 |
There was a problem hiding this comment.
Same end position and newline can be inside the try/catch
There was a problem hiding this comment.
they can't be right? they'll raise errors at different times and different behavior is expected at those times
| @@ -287,4 +304,4 @@ class NoCloseError(Exception): | |||
|
|
|||
| def __init__(self, annotation, code): | |||
| Exception.__init__(self, annotation + " has no closure") | |||
| self.code = code | |||
| self.code=code | |||
There was a problem hiding this comment.
The code does not comply to PEP8.
PEP8Bear, severity NORMAL, section autopep8.
The issue can be fixed by applying the following patch:
--- a/bears/general/AnnotationBear.py
+++ b/bears/general/AnnotationBear.py
@@ -304,4 +304,4 @@
def __init__(self, annotation, code):
Exception.__init__(self, annotation + " has no closure")
- self.code=code
+ self.code = code
| unescaped_search_for("\n", text[position + 1:])) | ||
| newline = position + newline_match.span()[1] | ||
| except StopIteration: | ||
| newline = -1 |
There was a problem hiding this comment.
I think you should put this try except construction into a function!
|
ok small change, big merge? |
Find function doesn't ignore escape sequences hence we use unescaped_search_for to ignore the sequences which are escaped. Fixes #993
|
ack e68e893 |
|
@rultor merge |
Fixes #993