search

CensoredUsername / unrpyc

A ren'py script decompiler
Other
821 stars 149 forks source link

Dev: Cleanup and pep8 #213

Closed madeddy closed 2 months ago

madeddy commented 3 months ago

Not much in terms of code changes beyond removal of a few unused imports, vars, a leftover comment (init.py L489) and some added parens for code stacking. However a lot of formatting.

CensoredUsername commented 3 months ago

Huh, I never knew of the pep8 standard of at least 2 spaces before an inline comment.

CensoredUsername commented 3 months ago

Went through it all, there's only 2 things I'm not really a fan of that I marked above. Thanks!

madeddy commented 3 months ago

Huh, I never knew of the pep8 standard of at least 2 spaces before an inline comment.

I was also surprised at the first time i read it. Among others. Personally i don't care if its 1, 2 or 3 but more or none spaces is imho counterproductive. As said, the linter is useful IMO, but it gives just the options to block the error code in question generally, turn off lint for the whole file or with 'NOQA' at every line with a error i don't care about. A section 'NOQA' would be good.

BTW, fun fact: unrpyc code breaks NEARLY every line spacing rule: E301: expected 1 blank line, found 0 E302: expected 2 blank lines, found 0 E303: too many blank lines (3) E304: blank lines found after function decorator <- This one not E305: expected 2 blank lines after end of function or class E306: expected 1 blank line before a nested definition

😁 You have your own system with the code section comments e.g. # API or # Machinery so i don't touched this. I do not want to be a grammar nazi, but some rules make imho sense. At least to avoid chaos code.

madeddy commented 3 months ago

So, i made some changes according your comments. If you want other corrections just tell. It's no problem.

CensoredUsername commented 3 months ago

BTW, fun fact: unrpyc code breaks NEARLY every line spacing rule:

TBH, those things are a bit too rigid at times. There's a lot I do agree with in general (empty lines after functions / classes etc) but sometimes you want to group some items together (or at least, separate them from some others clearly) and then things like too many empty lines between are just annoying.

I think it's fine now, can you change it from a work in progress?

madeddy commented 3 months ago

sometimes you want to group some items together (or at least, separate them from some others clearly)

The same. I like to do this for code parts but also imports. And then the formatter mucks it up.

change it from a work in progress

Will do. I thought a draft brings some advantages but i noticed none.

CensoredUsername commented 3 months ago

Will do. I thought a draft brings some advantages but i noticed none.

It just disallows me from merging it in the web UI.