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

Make vimlparser.py (much more) PEP8 compliant #122

Merged
merged 19 commits into from
Aug 1, 2019
Merged

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Jul 19, 2019

@blueyed blueyed changed the title Make vimlparser.py (much more) PEP8 compliant [WIP] Make vimlparser.py (much more) PEP8 compliant Jul 19, 2019
@tyru
Copy link
Member

tyru commented Jul 19, 2019

Is there any chance to lint this in CI? 😎

@blueyed
Copy link
Contributor Author

blueyed commented Jul 19, 2019

Sure.. :)

I am using the following config locally (which is OK for now I guess):

[flake8]
per-file-ignores =
    # undefined names for StringReader, VimLParser, Compiler
    py/vimlfunc.py:F821
    py/vimlparser.py:E501

With this PR it results in:

py/vimlparser.py:2192:79: F821 undefined name 'token'
py/vimlparser.py:2192:93: F821 undefined name 'token'
py/vimlparser.py:4249:46: W605 invalid escape sequence '\e'

(two real issues (#121, and #110 (review)))

@blueyed
Copy link
Contributor Author

blueyed commented Jul 19, 2019

There you go: https://travis-ci.org/vim-jp/vim-vimlparser/jobs/561117833

@blueyed
Copy link
Contributor Author

blueyed commented Jul 19, 2019

I think it's ready, but needs to be rebased after fixing the two issues mentioned above.

@blueyed blueyed changed the title [WIP] Make vimlparser.py (much more) PEP8 compliant [RFC/blocked] Make vimlparser.py (much more) PEP8 compliant Jul 19, 2019
@blueyed blueyed changed the title [RFC/blocked] Make vimlparser.py (much more) PEP8 compliant [RFC] Make vimlparser.py (much more) PEP8 compliant Jul 19, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Jul 19, 2019

Rebased.

Makefile Outdated Show resolved Hide resolved
@tyru
Copy link
Member

tyru commented Jul 19, 2019

maybe this change is conflicted to @mattn (and me?), so I can't merge this soon.

@blueyed
Copy link
Contributor Author

blueyed commented Jul 19, 2019

maybe this change is conflicted to @mattn (and me?), so I can't merge this soon.

Sure, happy to rebase this later again.

py/vimlparser.py Outdated

# TODO: under construction
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Slightly better now, but has wrong indent.

@tyru
Copy link
Member

tyru commented Jul 24, 2019

integrate #132 / #131

Ah sorry, now I think I was requesting too much to @blueyed and it makes this PR bloated.
I think this should be in another PR.

After #119 and #125, please rebase master again 🙇
(But currently @mattn may be busy now because he is VimConf 2019 stuff... 😎 please hang tight)

@blueyed
Copy link
Contributor Author

blueyed commented Jul 24, 2019

Ah sorry, now I think I was requesting too much to @blueyed and it makes this PR bloated.
I think this should be in another PR.

We can do it later.
I still think that we basically only need to run flake8 on the output (via Python 3) to be good, and that's done here already.

please hang tight

No worries, just let me know when it is a bit calmer, and I am happy to rebase it then.

@tyru
Copy link
Member

tyru commented Jul 31, 2019

No worries, just let me know when it is a bit calmer, and I am happy to rebase it then.

Now it's time to rebase 😎

@blueyed blueyed changed the title [RFC] Make vimlparser.py (much more) PEP8 compliant Make vimlparser.py (much more) PEP8 compliant Jul 31, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Jul 31, 2019

Rebased, should be good.
Feel free to squash - but I've added / kept existing "gen" steps to see what is being done.

@tyru tyru merged commit 280f30c into vim-jp:master Aug 1, 2019
@tyru
Copy link
Member

tyru commented Aug 1, 2019

Thank you 🎉🍺🎉

@blueyed blueyed deleted the pep8 branch August 1, 2019 19:23
@blueyed
Copy link
Contributor Author

blueyed commented Aug 1, 2019

You are welcome, glad to help! 🍻

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

Successfully merging this pull request may close these issues.

2 participants