Fix flake8 failures. #5
Conversation
ericsnowcurrently
force-pushed
the
lint
branch
from
a2f3d83
to
ab1985e
Compare
ptvsd/debugger.py
Outdated
| import ptvsd.wrapper | ||
| import pydevd | ||
| sys.argv[1:0] = ['--port', str(port_num), '--client', '127.0.0.1', '--file', filename] | ||
| # XXX docstring |
There was a problem hiding this comment.
We prefer "TODO:" over "XXX" (which I'm fairly sure our "no bad language" scanner is going to flag...)
ptvsd/futures.py
Outdated
| traceback.print_exception(self._exc_info[0], self._exc_info[1], self._exc_info[2], file=sys.__stderr__) | ||
| exctype, exc, tb = self._exc_info | ||
| traceback.print_exception(exctype, exc, tb, | ||
| file=sys.__stderr__) |
There was a problem hiding this comment.
print_exception(*self._exc_info, file...)?
ptvsd/ipcjson.py
Outdated
| content = json.dumps(payload).encode('utf-8') | ||
| headers = ('Content-Length: %d\r\n\r\n' % (len(content), )).encode('ascii') | ||
| headers = ('Content-Length: {}\r\n\r\n'.format(len(content)) | ||
| ).encode('ascii') |
There was a problem hiding this comment.
Seeing a lot of unnecessary code wrapping that isn't really beneficial. Feel free to up the limit to 120 chars per line (but selectively wrap lines shorter where important information would be past 80)
There was a problem hiding this comment.
Yeah, normally I would split things up rather than wrap. For this patch I was trying to resist the urge to do more in-depth cleanup of the code, hence the relatively minimal change here. That said, I'll skim through the code and re-format spots where readability is suffering.
As to a limit of 120, I didn't realize you can set the line length for flake8 (i.e. via setup.cfg). That's cool. :) Regardless, in general I'm a big fan of the 80 character line length. In my experience it makes a big difference in readability. (I suffered through 2 years of Go code where line length regularly gets into the 120 range and beyond.)
ptvsd/ipcjson.py
Outdated
| # XXX docstring | ||
| # base protocol defined at: | ||
| # https://github.com/Microsoft/language-server-protocol/ | ||
| # blob/master/protocol.md#base-protocol |
There was a problem hiding this comment.
Wrapping URLs is going to make them non-clickable in any editor that supports it. Better to let it run off the screen for most people. (I think it's safe to assume VS Code is the editor for people touching these files :) )
ptvsd/wrapper.py
Outdated
| ('python-BaseException\t{}\t{}\t{}' | ||
| ).format(2 if break_raised else 0, | ||
| 1 if break_uncaught else 0, | ||
| 0)) |
There was a problem hiding this comment.
This is hideous :) Maybe we should format the message on a separate line? (Oh for an f-strings backport...)
ptvsd/wrapper.py
Outdated
| pydevd_comm.CMD_STEP_INTO, | ||
| pydevd_comm.CMD_STEP_OVER, | ||
| pydevd_comm.CMD_STEP_RETURN, | ||
| } |
There was a problem hiding this comment.
I'll take an opinion from @int19h about whether to dedent the closing brace/paren in these contexts. I prefer to dedent, but consistency and team preference wins (and I'm not expecting to be actively developing this code enough to be really upset either way)
There was a problem hiding this comment.
I'd prefer to either dedent it, or else put it on the same line as the last item.
There was a problem hiding this comment.
I'm okay with dedenting if that is the way we do it here. :) Obviously I prefer to put the closing "brace" on its own line at the same indent level as the last item. :) It's visually consistent with how code blocks are indented in Python so I find it less jarring visually than dedenting.
Regardless, I would definitely prefer to not put it at the end of the line of the last item. Doing that makes adding or removing items a pain.
ericsnowcurrently
force-pushed
the
lint
branch
from
ab1985e
to
f9a1228
Compare
(based on PR #4)
This change cleans up all the failures reported by running flake8 on the code base. I made an effort to not make any other changes.
You can skip the base PR by reviewing the following:
https://github.com/Microsoft/ptvsd/pull/5/files/9a61a4e..HEAD