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.

Sign up for GitHub

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion spyderplugins/widgets/pylintgui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 :-)

match = re.match(regex, lines[0])
if match is not None:
return match.groups()[1]

Expand Down