fixes a bug causing extra newlines after comments.

[ernop]

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:

    #
    import ipdb;ipdb.set_trace()
    print 5

before change:

$python test.py 
> test.py(3)<module>()
             1 #
             [extra line]
             2 import ipdb;ipdb.set_trace()
---->        3 print 5

after change:

$python test.py 
> test.py(3)<module>()
             1 #
             2 import ipdb;ipdb.set_trace()
---->        3 print 5

[ernop]

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.

[takluyver]

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 pycolor test.py, I don't see this bug - I wonder what the difference is.

[ernop]

Yes, it looks like that bug fix in 2.6 causes an extra newline.

test.py:
#com

output of tokenize.generate_tokens(open('test.py','r').readline):
python 2.4:
(53, '#com\n', (1, 0), (1, 5), '#com\n')
(0, '', (2, 0), (2, 0), '')

python 2.6
(53, '#com', (1, 0), (1, 4), '#com\n')
(54, '\n', (1, 4), (1, 5), '#com\n')
(0, '', (2, 0), (2, 0), '')

so now it looks to me like skipping the whole section inserting newlines here would fix the problem.

[takluyver]

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?

[ernop]

@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.

[takluyver]

Oh, I think I see. Are these lines writing in the newline when it handles the next token:

if newpos > oldpos:
    owrite(self.raw[oldpos:newpos])

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.

[takluyver]

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:

In [1]: if True:
   ...:     # comment
   ...:     1/0
   ...:     
---------------------------------------------------------------------------
ZeroDivisionError                         Traceback (most recent call last)
/home/thomas/Code/virtualenvs/ipy-trunk/<ipython-input-1-97101fdeac14> in <module>()
      1 if True:
      2     # comment

----> 3     1/0
      4 

ZeroDivisionError: integer division or modulo by zero

[takluyver]

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.

[fperez]

On Tue, Jan 24, 2012 at 2:05 PM, Thomas
reply@reply.github.com
wrote:

@fperez : Do you think we should skip adding a test for this

I'm OK with no tests for this. Unfortunately it's a reality with
ipython: it's such a strongly interactive tool that testing all of it
automatically is extremely hard, so we'll always have to live with
some corners that only get tested 'in the field'.

[fperez]

@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.

[ernop]

I deleted the commented out code. Is that enough?

[fperez]

Since @takluyver was reviewing, I'll leave the final call to him. But from where I stand, it looks good to go. Thanks!

[takluyver]

OK, I'm merging it. Thanks @ernop .