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

Match for pylint when parsing pylint version #2434

Merged
merged 1 commit into from Jun 8, 2015
Merged

Match for pylint when parsing pylint version #2434

merged 1 commit into from Jun 8, 2015

Conversation

pijyoi
Copy link
Contributor

@pijyoi pijyoi commented May 10, 2015

Pylint program is invoked using PYLINT, which can now be pylint, pylint3, python3-pylint. Fix the regex to just search for PYLINT.

@@ -61,7 +61,8 @@ def get_pylint_version():
shell=True if os.name == 'nt' else False)
lines = to_unicode_from_fs(process.stdout.read()).splitlines()
if lines:
match = re.match('(pylint3?|pylint-script.py) ([0-9\.]*)', lines[0])
regex = '({0}|pylint-script.py) ([0-9\.]*)'.format(PYLINT)
Copy link
Member

Choose a reason for hiding this comment

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

I think a ? is missing here after {0}, at least according to the changed line above :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understood "pylint3?" to mean to match either "pylint" or "pylint3".
PYLINT equals "pylint" or "pylint3" or "python3-pylint" depending on the
binary that was already previously found.

Was the previous behavior to allow the version string to be different from
the binary name?

  • pylint but "pylint3 1.3.1"
  • pylint3 but "pylint 1.3.1"
    Does this happen in practice?

I originally used "(python3-pylint|pylint3?|pylint-script.py)" but thought
that it was a burden to have to modify the regex each time a new binary
name cropped up.

Or simpler, why not just skip the first word? The code already assumes the
version string is on the first line.
On 12 May 2015 05:23, "Carlos Cordoba" notifications@github.com wrote:

In spyderplugins/widgets/pylintgui.py
#2434 (comment):

@@ -61,7 +61,8 @@ def get_pylint_version():
shell=True if os.name == 'nt' else False)
lines = to_unicode_from_fs(process.stdout.read()).splitlines()
if lines:

  •    match = re.match('(pylint3?|pylint-script.py) ([0-9.]*)', lines[0])
    
  •    regex = '({0}|pylint-script.py) ([0-9.]*)'.format(PYLINT)
    

I think a ? is missing here after {0}, at least according to the changed
line above :-)

Reply to this email directly or view it on GitHub
https://github.com/spyder-ide/spyder/pull/2434/files#r30083813.

Copy link
Member

Choose a reason for hiding this comment

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

I think ? is necesary to parse the output of pylint itself. I'll check that in master and I'll add it if needed.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, this was only necessary to detect the Pylint version, so I think your fix is the right one :-)

@ccordoba12 ccordoba12 added this to the v3.0 milestone May 11, 2015
@ccordoba12 ccordoba12 modified the milestones: v2.3.5, v3.0 May 31, 2015
@ccordoba12 ccordoba12 changed the title match for PYLINT when parsing pylint version Match for pylint when parsing pylint output Jun 8, 2015
ccordoba12 added a commit that referenced this pull request Jun 8, 2015
Match for pylint when parsing pylint output
@ccordoba12 ccordoba12 merged commit e755341 into spyder-ide:master Jun 8, 2015
@ccordoba12 ccordoba12 changed the title Match for pylint when parsing pylint output Match for pylint when parsing pylint version Jun 8, 2015
@pijyoi pijyoi deleted the fix-python3-pylint branch February 24, 2019 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants