Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

#250 - Added support for multi-lined config entries #281

Merged
merged 7 commits into from
Sep 18, 2017

Conversation

shacharoo
Copy link
Member

@shacharoo shacharoo commented Sep 8, 2017

Thanks for submitting a PR!

Please make sure to check for the following items:

  • Add unit tests and integration tests where applicable.
    If you've added a error code or changed an error code behavior,
    you should probably add or change a test case file under tests/test_cases/ and add
    it to the list under tests/test_definitions.py.
    If you've added or changed a command line option,
    you should probably add or change a test in tests/test_integration.py.
  • Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".
    Make sure to include the PR number after you open and get one.

Please don't get discouraged as it may take a while to get a review.

Solves #250 .

@@ -7,9 +7,9 @@
from re import compile as re


try: # Python 3.x
try: # Python 2.x
from ConfigParser import RawConfigParser
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, I would just use the backport from Python 3. The Python 2 version fo ConfigParser is awful.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Nurdok , what say you?

Copy link
Member

Choose a reason for hiding this comment

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

Since we crossed to chasm of package dependencies, I think you should use whichever package is best for the job, even if it adds a dependency.

# Dealing with split-lined configurations. The part might begin
# with a whitespace and/or a comment marker.
part = part.strip()
if not part or part.startswith(('#', ';')):
Copy link
Member

Choose a reason for hiding this comment

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

For example, if you use the backport, this goes away.

Copy link
Member

@Nurdok Nurdok left a comment

Choose a reason for hiding this comment

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

Use the backported pacakge as we discussed.

@@ -1 +1,2 @@
snowballstemmer==1.2.1
configparser==3.5.0
Copy link
Member

Choose a reason for hiding this comment

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

You also need to add this to setup.py, only for Python 2.x.


select_string = ('D100,\n'
' #D103,\n'
' D204, D300')
Copy link
Member

Choose a reason for hiding this comment

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

Also add a line that has in-line comment, such as:

D301,  # we want to select this
D302

setup.py Outdated
@@ -28,6 +28,7 @@
install_requires=[
'snowballstemmer',
'six',
'configparser;python_version<"3.3"',
Copy link
Member

Choose a reason for hiding this comment

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

This will break on old enough versions of pip and setuptools which, if I remember correctly, is anyone using Ubuntu between 14.04 and 16.04

Copy link
Member Author

Choose a reason for hiding this comment

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

@Nurdok and I were just discussing it. I saw this and thought about doing it in the setup.py, but it's reeeeeeeeeeeeally gross. Have you encountered that issue before and have an example / boilerplate? 😜

Copy link
Member Author

Choose a reason for hiding this comment

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

Bump, @sigmavirus24 ?

setup.py Outdated

# Do not update the version manually - it is managed by `bumpversion`.
version = '2.0.1rc'


REQUIRES = [
'snowballstemmer',
Copy link
Member

Choose a reason for hiding this comment

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

Indentation seems weird here.
Should be:

REQUIRES = [
    'snowballstemmer',
    'six',
]

Copy link
Member

Choose a reason for hiding this comment

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

Also, why is REQUIRED capital cased? It's not a constant.

@@ -277,6 +277,29 @@ def foo():
assert 'file does not contain a pydocstyle section' not in err


def test_multiple_lined_config_file(env):
"""Blah."""
Copy link
Member

Choose a reason for hiding this comment

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

Oops :)

@Nurdok Nurdok merged commit 6c1cf99 into PyCQA:master Sep 18, 2017
@shacharoo shacharoo deleted the Feature/support-multi-lined-configs branch September 18, 2017 12:14
@@ -17,6 +17,7 @@ New Features
* Added support for Python 3.6 (#270).
* Specifying an invalid error code prefix (e.g., ``--select=D9``) will print
a warning message to ``stderr`` (#253, #279).
* Configuration files now support multiple-lined entries (#250, #281).
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest "multiple line entries" here (not lined).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants