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

allow non ascii chars according to go spec #36

Merged
merged 2 commits into from
Feb 9, 2016
Merged

allow non ascii chars according to go spec #36

merged 2 commits into from
Feb 9, 2016

Conversation

mmlb
Copy link
Contributor

@mmlb mmlb commented Feb 9, 2016

@mmlb
Copy link
Contributor Author

mmlb commented Feb 9, 2016

See following script for demo.

#!/usr/bin/env python3

import re

# pattern = r'(?:[a-zA-Z_]\w*|[\]\)])\.(?:[a-zA-Z_]\w*)?'
pattern = r'(?:[^\W\d]\w*|[\]\)])\.(?:[^\W\d]\w*)?'
prog = re.compile(pattern)
tests = {
    'a.': True,
    '_x9.': True,
    'ThisVariableIsExported.': True,
    'αβ.': True,
    '].a': True,
    ']._x9': True,
    '].ThisVariableIsExported': True,
    '].αβ': True,
    ').a': True,
    ')._x9': True,
    ').ThisVariableIsExported': True,
    ').αβ': True,
    '9.': False,
}

for k, v in tests.items():
    match = prog.match(k) is not None
    if v == match:
        print('"%s" expected: %s, got: %s' % (k, v, match))
    else:
        print()
        print('!!!! "%s" expected: %s, got: %s !!!!' % (k, v, match))
        print()

@zchee
Copy link
Member

zchee commented Feb 9, 2016

/cc @Shougo @svanharmelen

@svanharmelen
Copy link
Contributor

Yeah... Looks good! 👍

@mmlb while your at it, could you maybe trow in a \b as well? As I just noticed that case while testing this:

6a. should not match, but it does. By adding a \b at the start we fix this as well. The regex then becomes:

(?:\b[^\W\d]\w*|[\]\)])\.(?:[^\W\d]\w*)?

@zchee
Copy link
Member

zchee commented Feb 9, 2016

and /cc @nhooyr

@mmlb
Copy link
Contributor Author

mmlb commented Feb 9, 2016

@svanharmelen do you mean that you saw this while testing in nvim? I added the '6a.': False case to my little script there and it does not match.

@svanharmelen
Copy link
Contributor

@mmlb are you sure? It should match without the \b. Check here: https://regex101.com/r/vB7rC7/1

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@mmlb @svanharmelen Thanks for assistance :)

I do not quite understand the regexp, because I can not determine... 😅
so I entrust to @svanharmelen and @Shougo judgment. 😱

@Shougo
Copy link
Collaborator

Shougo commented Feb 9, 2016

LGTM

Edit: But yes, \b is needed.

@mmlb
Copy link
Contributor Author

mmlb commented Feb 9, 2016

updated

@svanharmelen
Copy link
Contributor

LGTM! Thanks @mmlb

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@mmlb @Shougo @svanharmelen
Thanks!! LGTM.

zchee added a commit that referenced this pull request Feb 9, 2016
allow non ascii chars according to go spec
@zchee zchee merged commit 1df2aa3 into deoplete-plugins:master Feb 9, 2016
@mmlb
Copy link
Contributor Author

mmlb commented Feb 9, 2016

hopefully last of the regex PRs for a while ;)

@zchee
Copy link
Member

zchee commented Feb 9, 2016

@mmlb Thanks, I'm relieved to hear that :)

@nhooyr nhooyr mentioned this pull request Mar 11, 2016
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.

4 participants