Skip to content
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

AnnotationBear: Return precise string/comment info #1332

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aptrishu
Copy link
Member

Earlier the return used to be either a tuple of
source ranges of strings/comments now it is a
tuple of namedtuples with fields
"start_delimiter_range end_delimiter_range content_range full_range"
of singleline strings/multiline strings/singlieline comments/multiline comments
Also, It modifies KeywordBear, QuotesBear,
IndentationBear and their tests.

@gitmate-bot
Copy link
Collaborator

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!

@gitmate-bot
Copy link
Collaborator

Comment on e017fa2.

Body of HEAD commit contains too long lines. Commit body lines should not exceed 72 characters.

GitCommitBear, severity NORMAL, section commit.

@gitmate-bot
Copy link
Collaborator

Comment on e017fa2.

Shortlog of the HEAD commit contains 54 character(s). This is 4 character(s) longer than the limit (54 > 50).

GitCommitBear, severity NORMAL, section commit.

@aptrishu
Copy link
Member Author

aptrishu commented Jan 23, 2017

CC @jayvdb.
Please take a look at the pickle error in coala-ci log in circleCI.
namedtuple is creating a problem with pickle IMO.. should I use dict instead. Though namedtuple is easy to use..

self.get_singleline_strings)
if end_position and _range:
strings_range.append(_range)
end_position, start_delim, end_delim =\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a space after the equals, here and below

multiline_comment_range = []
fields = ('start_delimiter_range end_delimiter_range'
' content_range full_range')
singleline_string = namedtuple('singleline_string', fields)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with dict..

@aptrishu
Copy link
Member Author

@jayvdb It's still failing, I have put namedtuples outside the class.
If we use pickle from dill (https://github.com/uqfoundation/dill) it would be easily done without any such issues.

@aptrishu
Copy link
Member Author

All errors are resolved..

@sils
Copy link
Member

sils commented Feb 3, 2017

@aptrishu is this still wip? With that in the title you aren't getting any reviews and this is so important :( we need to merge it within hours

@sils sils added this to the 0.10 milestone Feb 3, 2017
@aptrishu aptrishu changed the title WIP AnnotationBear: Return precise string/comment info AnnotationBear: Return precise string/comment info Feb 3, 2017
@aptrishu
Copy link
Member Author

aptrishu commented Feb 3, 2017

Ohh I forgot to remove WIP from the tittle, Though I had labelled it pending review. so, it must be showing in review queue.

@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

1 similar comment
@gitmate-bot
Copy link
Collaborator

Hey! This pull request hasn't been updated for a while :/ It would be nice if we could get this going again!

AbsolutePosition(text, 0),
AbsolutePosition(text, len(text[0]) - 2))

compare_content = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains unused source code.

PyUnusedCodeBear, severity NORMAL, section flakes.

The issue can be fixed by applying the following patch:

--- a/tests/general/AnnotationBearTest.py
+++ b/tests/general/AnnotationBearTest.py
@@ -62,7 +62,6 @@
             AbsolutePosition(text, 0),
             AbsolutePosition(text, len(text[0]) - 2))
 
-        compare_content = []
 
         compare_start = SourceRange.from_absolute_position(
             'F',

@@ -1,3 +1,5 @@
import sys
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains unused source code.

PyUnusedCodeBear, severity NORMAL, section flakes.

The issue can be fixed by applying the following patch:

--- a/bears/general/AnnotationBear.py
+++ b/bears/general/AnnotationBear.py
@@ -1,4 +1,3 @@
-import sys
 
 from coalib.bearlib.languages.LanguageDefinition import LanguageDefinition
 from coalib.bears.LocalBear import LocalBear

@@ -1,3 +1,5 @@
import sys
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains unused source code.

Origin: PyUnusedCodeBear, Section: flakes.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp255z_p7_/bears/general/AnnotationBear.py
+++ b/tmp/tmp255z_p7_/bears/general/AnnotationBear.py
@@ -1,4 +1,3 @@
-import sys
 
 from coalib.bearlib.languages.LanguageDefinition import LanguageDefinition
 from coalib.bears.LocalBear import LocalBear

AbsolutePosition(text, 0),
AbsolutePosition(text, len(text[0]) - 2))

compare_content = []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains unused source code.

Origin: PyUnusedCodeBear, Section: flakes.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp255z_p7_/tests/general/AnnotationBearTest.py
+++ b/tmp/tmp255z_p7_/tests/general/AnnotationBearTest.py
@@ -62,7 +62,6 @@
             AbsolutePosition(text, 0),
             AbsolutePosition(text, len(text[0]) - 2))
 
-        compare_content = []
 
         compare_start = SourceRange.from_absolute_position(
             'F',

@aptrishu aptrishu force-pushed the annot2 branch 4 times, most recently from fa5e192 to e6468ee Compare October 6, 2017 07:23
Earlier the return used to be either a tuple
of source ranges of strings/comments now it
is a tuple of dicts with keys
'start_delimiter_range', 'end_delimiter_range'
'content_range', 'full_range' of singleline
strings/multiline strings/singleline
comments/multiline comments.
@aptrishu
Copy link
Member Author

aptrishu commented Oct 8, 2017

@Makman2 @userzimmermann review needed. :P

Copy link
Member

@userzimmermann userzimmermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aptrishu I took a quick look again and... IMO this needs a complete redesign

cc @sils


for string_range in ranges:
if (file[string_range.start.line-1][string_range.start.column-1] ==
temp_range = string_range.full_range
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is no need to use a different variable here.
Earlier, do

-    ranges = annotation_dict['strings']
+    ranges = [x.full_range for x in annotation_dict['strings']]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.