Skip to content

Commit

Permalink
Auto merge of #831 - micbou:replace-comments-strings-empty-lines, r=p…
Browse files Browse the repository at this point in the history
…uremourning

[READY] Fix multiline comments and strings issues

When `collect_identifiers_from_comments_and_strings` is `0`, we are replacing comments and strings with empty strings before extracting the identifiers. If one of these comments or strings span multiple lines, the current line number `line_num` may become invalid and, in that case, the identifier completer will fail to add the previous identifier on the `CurrentIdentifierFinished` event:

![ignore-comments-current-identifier-finished](https://user-images.githubusercontent.com/10026824/30140028-dca4c58e-9371-11e7-9231-f8ef35130ed7.gif)
As you can see, the `test` identifier is properly extracted before the comment but not after.

Another issue with multiline comments and strings is that, when adding an identifier under the cursor on the `InsertLeave` event, we only remove comments and strings on the current line. This is incorrect if the current line is in the middle of a multiline comment or string:

![ignore-comments-insert-leave](https://user-images.githubusercontent.com/10026824/30140274-b8af9fee-9373-11e7-94ec-78cf00585c39.gif)
The `test` identifier is extracted even though it's inside a comment.

For now, I am updating the tests to showcase both issues. I'll update the PR with the fixes once the builds failed.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/831)
<!-- Reviewable:end -->
  • Loading branch information
zzbot authored Sep 9, 2017
2 parents 927a5f2 + 74648f8 commit 2e7a596
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 6 deletions.
7 changes: 5 additions & 2 deletions ycmd/completers/all/identifier_completer.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,13 @@ def _RemoveSmallCandidates( candidates, min_num_candidate_size_chars ):

def _GetCursorIdentifier( collect_from_comments_and_strings,
request_data ):
line = request_data[ 'line_value' ]
filepath = request_data[ 'filepath' ]
contents = request_data[ 'file_data' ][ filepath ][ 'contents' ]
filetype = request_data[ 'first_filetype' ]
if not collect_from_comments_and_strings:
line = identifier_utils.RemoveIdentifierFreeText( line, filetype )
contents = identifier_utils.RemoveIdentifierFreeText( contents, filetype )
contents_per_line = SplitLines( contents )
line = contents_per_line[ request_data[ 'line_num' ] - 1 ]
return identifier_utils.IdentifierAtIndex(
line,
request_data[ 'column_codepoint' ] - 1,
Expand Down
8 changes: 7 additions & 1 deletion ycmd/identifier_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from builtins import * # noqa

import re
from ycmd.utils import SplitLines

C_STYLE_COMMENT = "/\*(?:\n|.)*?\*/"
CPP_STYLE_COMMENT = "//.*?$"
Expand Down Expand Up @@ -170,8 +171,13 @@ def IdentifierRegexForFiletype( filetype ):
return FILETYPE_TO_IDENTIFIER_REGEX.get( filetype, DEFAULT_IDENTIFIER_REGEX )


def ReplaceWithEmptyLines( regex_match ):
return '\n' * ( len( SplitLines( regex_match.group( 0 ) ) ) - 1 )


def RemoveIdentifierFreeText( text, filetype = None ):
return CommentAndStringRegexForFiletype( filetype ).sub( '', text )
return CommentAndStringRegexForFiletype( filetype ).sub(
ReplaceWithEmptyLines, text )


def ExtractIdentifiersFromText( text, filetype = None ):
Expand Down
20 changes: 20 additions & 0 deletions ycmd/tests/identifier_completer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,19 @@ def GetCursorIdentifier_LineEmpty_test():

def GetCursorIdentifier_IgnoreIdentifierFromCommentsAndStrings_test():
eq_( '', ic._GetCursorIdentifier( False, BuildRequestWrap( '"foobar"', 4 ) ) )
eq_( '', ic._GetCursorIdentifier( False,
BuildRequestWrap( '/*\n'
' * foobar\n'
' */', 5, 2 ) ) )


def GetCursorIdentifier_CollectIdentifierFromCommentsAndStrings_test():
eq_( 'foobar', ic._GetCursorIdentifier( True,
BuildRequestWrap( '"foobar"', 4 ) ) )
eq_( 'foobar', ic._GetCursorIdentifier( True,
BuildRequestWrap( '/*\n'
' * foobar\n'
' */', 5, 2 ) ) )


def PreviousIdentifier_Simple_test():
Expand Down Expand Up @@ -178,13 +186,25 @@ def PreviousIdentifier_IgnoreIdentifierFromCommentsAndStrings_test():
ic._PreviousIdentifier( 2, False, BuildRequestWrap( '"foo"\n',
column_num = 1,
line_num = 2 ) ) )
eq_( '',
ic._PreviousIdentifier( 2, False, BuildRequestWrap( '/*\n'
' * foo\n'
' */',
column_num = 2,
line_num = 3 ) ) )


def PreviousIdentifier_CollectIdentifierFromCommentsAndStrings_test():
eq_( 'foo',
ic._PreviousIdentifier( 2, True, BuildRequestWrap( '"foo"\n',
column_num = 1,
line_num = 2 ) ) )
eq_( 'foo',
ic._PreviousIdentifier( 2, True, BuildRequestWrap( '/*\n'
' * foo\n'
' */',
column_num = 2,
line_num = 3 ) ) )


def FilterUnchangedTagFiles_NoFiles_test():
Expand Down
9 changes: 6 additions & 3 deletions ycmd/tests/identifier_utils_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,13 @@ def RemoveIdentifierFreeText_PythonComments_test():


def RemoveIdentifierFreeText_CstyleComments_test():
eq_( "\n bar",
iu.RemoveIdentifierFreeText( "/* foo\n */ bar" ) )

eq_( "foo \nbar \nqux",
iu.RemoveIdentifierFreeText( "foo \nbar /* foo */\nqux" ) )

eq_( "foo \nbar \nqux",
eq_( "foo \nbar \n\nqux",
iu.RemoveIdentifierFreeText( "foo \nbar /* foo \n foo2 */\nqux" ) )


Expand Down Expand Up @@ -90,10 +93,10 @@ def RemoveIdentifierFreeText_NoMultilineString_test():


def RemoveIdentifierFreeText_PythonMultilineString_test():
eq_( "\nzoo",
eq_( "\n\n\nzoo",
iu.RemoveIdentifierFreeText( "\"\"\"\nfoobar\n\"\"\"\nzoo" ) )

eq_( "\nzoo",
eq_( "\n\n\nzoo",
iu.RemoveIdentifierFreeText( "'''\nfoobar\n'''\nzoo" ) )


Expand Down

0 comments on commit 2e7a596

Please sign in to comment.