Manim-Notebook / manim-notebook

Simple commands to replicate the manim dev workflow in VSCode
MIT License
5 stars 0 forks source link

Fix IPython cell detection (multiple ones in one datastream) #64

Closed Splines closed 1 day ago

Splines commented 1 day ago

This fixes #52. At least I hope I've fixed the problem identified by @VladimirFokow in the first log. Please tell me if this PR also fixes the other problems of that issue. If not, I will take a look at them as well.

The issue could be identified by comparing logs on different machines. Here only the most relevant parts:

On Linux (WSL) ``` 2024-10-31 10:49:18.109 [trace] [manimShell.ts:514:28] [unknown] ๐Ÿงพ Terminal data: In [1]: checkpoint_paste()In [1]: checkpoint_paste() 2024-10-31 10:49:18.109 [debug] [manimShell.ts:536:32] [unknown] ๐Ÿ“ฆ IPython cell 1 detected 2024-10-31 10:49:18.109 [debug] [manimShell.ts:475:24] [unknown] ๐Ÿ•’ While waiting for command to finish, iPythonCellCount=1, currentExecutionCount=1 2024-10-31 10:49:18.109 [debug] [manimShell.ts:475:24] [unknown] ๐Ÿ•’ While waiting for command to finish, iPythonCellCount=1, currentExecutionCount=1 2024-10-31 10:49:18.474 [trace] [manimShell.ts:514:28] [unknown] ๐Ÿงพ Terminal data: 1 2 In [2]: In [2]:  2024-10-31 10:49:18.474 [debug] [manimShell.ts:536:32] [unknown] ๐Ÿ“ฆ IPython cell 2 detected ```
On MacOS ``` 2024-10-30 23:20:39.611 [trace] [manimShell.ts:514:28] [unknown] ๐Ÿงพ Terminal data: In [1]: checkpoint_paste() In [1]: checkpoint_paste() 1 2 In [2]: In [2]:  2024-10-30 23:20:39.612 [debug] [manimShell.ts:536:32] [unknown] ๐Ÿ“ฆ IPython cell 1 detected 2024-10-30 23:20:39.612 [debug] [manimShell.ts:475:24] [unknown] ๐Ÿ•’ While waiting for command to finish, iPythonCellCount=1, currentExecutionCount=1 ```

See the difference? On Linux I get two separate data streams and each one only contains one IPython Cell identifier. This is not the case for MacOS. Therefore, I changed the RegEx to globally match multiple occurrences in a data stream and then just take the maximum number I have found, in this example: 2.

Note the fixed approach is coherent with

https://github.com/bhoov/manim-notebook/blob/b4ec9bbe242a3d120d2674091c046f1962f29809/src/manimShell.ts#L478

since this will evaluate to true if the seen cell count is higher than the current cell count. So even if we skip over cells via this new tactic, we will definitely end the old ones when a cell higher than the current execution count is seen.

VladimirFokow commented 1 day ago