Skip to content

Add option for hanging closing brackets #94

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

Jump to bottom
Merged
merged 3 commits into from
Jul 15, 2018
Merged

Add option for hanging closing brackets #94

merged 3 commits into from
Jul 15, 2018

Conversation

jakar
Copy link
Contributor

@jakar jakar commented Oct 24, 2017

The main idea is discussed at length in PyCQA/pycodestyle#103.

It allows this style:

my_list = [
    1, 2, 3,
    4, 5, 6,
    ]

result = some_function_that_takes_arguments(
    'a', 'b', 'c',
    'd', 'e', 'f',
    )

A similar option was added to pycodestyle / pep8 awhile ago. See PyCQA/pycodestyle#207.

blueyed
blueyed reviewed Oct 24, 2017
View reviewed changes
.travis.yml Outdated
@@ -15,5 +15,5 @@ before_script:
- pip install --user vim-vint
- vim --version
script:
- make test
- make test_slow
Copy link
Member

Choose a reason for hiding this comment

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

Slow/fast should only make a difference if the tests are hanging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like it should, but now every test is failing. I only made that change in an attempt to debug. I plan on reverting it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I was planning on force-pushing the previous commit, not reverting. But now I'm not sure if that's a good idea.

Copy link
Member

Choose a reason for hiding this comment

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

Both should work, we can easily squash-merge it later.

@jakar
Copy link
Contributor Author

jakar commented Oct 24, 2017

I am failing hard with these tests. They all pass on my local machine, with VIMRUNNER_REUSE_SERVER=1 and unset. Any suggestions?

@blueyed
Copy link
Member

blueyed commented Oct 24, 2017

Would have to look into this myself, leave it like this for now?!

@jakar
Copy link
Contributor Author

jakar commented Oct 24, 2017

Yeah, let's leave it like this for now. It took me about a year to finally get around to making those small test changes and opening this PR. So I guess you could say that I'm not in a hurry.

@blueyed
Copy link
Member

blueyed commented Oct 24, 2017

Yeah, I also feel sorry for the inherited test suite that we have to live with.

Thanks for bringing it up in the first place.

I plan to spend some Sunday afternoon here, with some ideas to fix #91 - but I am currently too far off, so it may take some Sundays.

jakar added 3 commits May 8, 2018 11:59
@jakar
Add option for hanging closing brackets
37be909 
The main idea is discussed at length in PyCQA/pycodestyle#103.
@jakar
Document python_pep8_indent_hang_closing
d4a1f9b
@jakar
Add tests for python_pep8_indent_hang_closing
3ac9093 
This might be overkill. It reruns the current "vim" set of tests once
for each value of the new setting. Of course, this makes the test suite
take longer to run. I couldn't think of a good way to factor out the
relevant test cases without creating a mess.
@jakar jakar force-pushed the feature/hang-closing branch from e04d5ac to 3ac9093 Compare May 8, 2018 16:23
@jakar
Copy link
Contributor Author

jakar commented May 8, 2018

Finally rebased, and now the tests pass. For good measure, I also ran them on my local machine (real X) and in a bionic container (fake X). Everything passes.

No idea why they failed before. I could never reproduce it.

@blueyed blueyed merged commit 19c53ee into Vimjas:master Jul 15, 2018
@blueyed
Copy link
Member

blueyed commented Jul 15, 2018

Thanks! (better late than never :D)

@blueyed blueyed mentioned this pull request Jul 26, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blueyed blueyed blueyed left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jakar @blueyed