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
fixes a bug causing extra newlines after comments. #1247
Conversation
I can see the extra line in python 2.6 and 2.7 - does it exist other places? It doesn't seem to be intentional cause it kind of messes up the code view, particularly when doing longer listings. |
So is it the case that tokenizer produces an extra newline token (with no token text) at the end of a comment? Does it relate to this Python bug in any way? http://bugs.python.org/issue1230484 Also, using |
Yes, it looks like that bug fix in 2.6 causes an extra newline. test.py: output of tokenize.generate_tokens(open('test.py','r').readline): python 2.6 so now it looks to me like skipping the whole section inserting newlines here would fix the problem. |
I'm not quite sure why that's causing an extra newline. It looks like it should write '#com', then '\n' once, then carry on to the next line. What am I missing? |
@takluyver , I think it's the python tokenize function that's adding the extra newline. In the python output side it's lines 221-223 that find this newline & insert a newline to the output. Since this is changed in python, I think pycolorize can just not do this anymore. |
Oh, I think I see. Are these lines writing in the newline when it handles the next token:
In that case, I think this might be the best way to handle it. Could you have a look at making a test case for this that fails at present? Have a look in IPython/utils/tests for examples of existing tests. |
In quick testing, I can confirm that this seems to solve the problem. Automated testing looks like it would be quite tricky, since all the relevant code is skipped when colours are disabled. @fperez : Do you think we should skip adding a test for this, or modify the code to let us test it (e.g. add an optional parameter to override the fast path with colour turned off)? Minimal manual test case:
|
One more point, @ernop : If we leave the code commented out, it should have a comment clearly explaining why we left it there. But I think this can simply be deleted - it looks like it's fairly clear that it works. |
On Tue, Jan 24, 2012 at 2:05 PM, Thomas
I'm OK with no tests for this. Unfortunately it's a reality with |
@ernop, it seems like we're almost done here. Do you want to address @takluyver's last comment and just delete that dead code? Then we can merge. The longer this lingers, the higher the chances of a conflict. |
I deleted the commented out code. Is that enough? |
Since @takluyver was reviewing, I'll leave the final call to him. But from where I stand, it looks good to go. Thanks! |
OK, I'm merging it. Thanks @ernop . |
fixes a bug causing extra newlines after comments in colorized code.
fixes a bug causing extra newlines after comments in colorized code.
I noticed that the formatter for code in ipdb inserts extra newlines after comments, & traced it a line in pycolorize that inserts newlines even when the input tokentext is empty.
I'm not sure if it's due to the way the tokenizer works, or what, but a single line modification to PyColorize, to not insert newlines in this case solved the problem.
an example:
test.py:
before change:
after change: