vyperlang / blackadder

MIT License
14 stars 5 forks source link

Fork black #15

Closed AbnerZheng closed 2 years ago

AbnerZheng commented 2 years ago

This PR fork from the project black(https://github.com/psf/black), and rewrote its grammar file to support Vyper. The reason why I can't use Vyper's ast is that these ast lose comments node, and this is the exact reason why black, flake8 need to implement its parser as well. This is the first PR of the epic. You could run test_simple_format to see what has been achieved, and the content below '# output' in Valut.vy is the result of reformatter.

  1. https://stackoverflow.com/questions/7456933/python-ast-with-preserved-comments
AbnerZheng commented 2 years ago

@fubuloubu Could you please take a look?

fubuloubu commented 2 years ago

@fubuloubu Could you please take a look?

Will do, but this is a big PR and will take me time to review

Perhaps @charles-cooper can lend their thoughts too

AbnerZheng commented 2 years ago

The first two commits are coping code from black, so you could review from the third commit of the PR. Actually it doesn't change a lot.

AbnerZheng commented 2 years ago

@fubuloubu Just clean and reorder the commits, and force-pushed. Now the first two commits are adding .gitignore and coping code from black. And you can review from the third commit, from which I made the modification to support vyper.

AbnerZheng commented 2 years ago

Though this method works, but maintaining a grammar from lib2to3 is quite annoying. I prefer it working as google's reformatter tool yapf, which reformat the python file depending on ast.

https://github.com/google/yapf/blob/main/yapf/pyparser/pyparser.py#L68

charles-cooper commented 2 years ago

i think ideally, we would use vyper's built-in grammar -- https://github.com/vyperlang/vyper/blob/4d32d17a004b1211464bee4fcba6efeea8677b64/tests/grammar/test_grammar.py

AbnerZheng commented 2 years ago

Though this method works, but maintaining a grammar from lib2to3 is quite annoying. I prefer it working as google's reformatter tool yapf, which reformat the python file depending on ast.

How do you think? @fubuloubu

https://github.com/google/yapf/blob/main/yapf/pyparser/pyparser.py#L68

After diving into the new parser of yapf, I am sorry that the new parser is far from complete and not work. So depending on the ast and tokenize information is also quite a large project.

AbnerZheng commented 2 years ago

i think ideally, we would use vyper's built-in grammar -- https://github.com/vyperlang/vyper/blob/4d32d17a004b1211464bee4fcba6efeea8677b64/tests/grammar/test_grammar.py

Sounds great, I could rewrite the grammar rule of black according to the grammar. But I have a problem, I don't find the rule is actually used when generating vyper ast, so there is no guarantee the grammar define the exact rule of supported vyper grammar.

charles-cooper commented 2 years ago

Sounds great, I could rewrite the grammar rule of black according to the grammar. But I have a problem, I don't find the rule is actually used when generating vyper ast, so there is no guarantee the grammar define the exact rule of supported vyper grammar.

that's sharp of you! the lark grammar is experimental. we do, however, test that vyper programs can be parsed with the lark grammar, so you can consider that as a guarantee that valid vyper programs can be parsed with the lark grammar.

spinoch commented 2 years ago

Hey @AbnerZheng . Thanks for your efforts here. Since you closed this PR I assume you won't be working on this anymore, do you have any more insights about this issue?