bhoov / manim-notebook

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

executeCommand() doesn't execute the checkpoint_paste() command #18

Open mitkonikov opened 2 days ago

mitkonikov commented 2 days ago

Terminals: GitBash and Conda through Powershell and GitBash Issue replicated on 2 different PCs both Windows 11 one Enterprise and one Education (Version 23H2) IPython version: 8.21.0

I've downloaded the repository, npm installed, npm compiled everything and attached the debugger.

Simple Manim Scene:

from manimlib import *

class SubTensors(InteractiveScene):
    def construct(self):
        ## Interactions
        circle = Circle(radius = 2)
        self.play(DrawBorderThenFill(circle))

Detects the preview cell. Run the command:

manimgl sub_tensor.py SubTensors -se 5

And when I click on the Preview Manim Cell, it clears the console, writes checkpoint_paste() but doesn't run it at all.

With the debugger, I can see that with both the executeCommand and terminal.sendText, the command is not executed. If then I manually click ENTER in terminal, the command is executed, however it's now too late, the clipboard is already reset to the previous state and throws an error because it tries to run something that is not python code.

Since this happens with different terminals, I believe it's something due to the IPython console. Because if we just click Preview Manim Cell, without running the manimgl command, the command checkpoint_paste() is executed properly (even though, in this case, it's an error, since it's not recognized as a Powershell/Bash/Conda command).

What I have already tried:

Nothing worked. Any ideas as to why would this happen? I'm not familiar with the IPython console; it may have some weird interaction behaviour.

VladimirFokow commented 2 days ago

Hello!

@Splines , for info: the relevant code section

The problem is: checkpoint_paste() is written in the terminal, but is NOT executed.


@mitkonikov I really don't know the reason... Could you record a video?

Here are just some thoughts:

bhoov commented 1 day ago

I would also appreciate a video to understand. Right now, I have a suspicion that you are starting the manimgl ... -se ... command from a terminal other than the integrated VSCode terminal. Right now, our extension sends the checkpoint_paste() command only to the integrated, active VSCode terminal (i don't think we have tested multiple terminal windows open in the same vscode instance)

Can you try running the command in the integrated VSCode terminal and see if things work as expected?

This workflow will be much improved once #2 is solved, and @VladimirFokow is very close to getting that essential feature integrated

mitkonikov commented 1 day ago

Answers to the previous questions:

  • even if always just send_text - it doesn't execute in the IPython shell, but executes in the bash shell? That's weird

this is true, it executes it everywhere, except the IPython shell. But notice that the IPython shell could be doing some string trimming or smth else itself. That is why my suspicion is in the IPython shell.

Here's a video I made going through everything, including the preparation, debugger, different terminals. I forgot to show you that it works if we are executing the command on a terminal without running the manimgl command at all. Sorry for the lengthy video, I just wanted to show you everything how it behaves.

https://youtu.be/ahBg4jx75Ds

mitkonikov commented 1 day ago

Here's an image. I wanted to see if it would execute the checkpoint_paste() even inside conda terminal/env, without running manimgl. It executes it immediately.

image

mitkonikov commented 1 day ago

OMG. I fixed it.. I think..

const PREVIEW_COMMAND = `\x0C checkpoint_paste()\x1b`; // \x0C is Ctrl + L

When VSCode is writing text in the command line, the cursor is not being updated to the end. So, IPython shell thinks that the cursor is in the beginning, thus not executing the command but rather adding a new line. This behaviour is explained in this issue. So, after adding an ESC symbol at the end of the PREVIEW_COMMAND, IPython then sees that \n character should execute the command.

VladimirFokow commented 1 day ago

omg, wow

VladimirFokow commented 1 day ago

for reference, here is the 3b1b's command - there I didn't know what was the: "\x7F" * col if clear else "", # Bad hack Maybe for this, maybe not..

mitkonikov commented 1 day ago

According to this, 7F is the Delete character. This could be the same fix. Because the Delete character is a special one, it could just reset the cursor to the end. Let me try it.

However, tell me if this works on your machines too. You can fix it or tell me to make a PR, it's just one line change.

VladimirFokow commented 1 day ago

Yes, I'll test and can push, or you can make a PR if you want to. I feel your solution is better than with x7F

mitkonikov commented 1 day ago

For me, 7F doesn't work at the beginning or at the end of the PREVIEW_COMMAND.

If I put \x7f at the beginning of the PREVIEW_COMMAND nothing changes (same command, no execution), however at the end, for some reason, it deletes the last character. I suspect that the cursor is not at the end, but one character behind. That could be some VSCode issue, not updating the cursor for the last character properly. I'm not sure though.

This is after executing: \x0C checkpoint_paste()\x7f image

VladimirFokow commented 1 day ago

your solution works for me 🚀

RickLuiken commented 1 day ago

Hi, I found this issue as well. I just tried the proposed solution by @mitkonikov. It seems to work most of the time, however when the window get a bit laggier (when you have a few updaters running), it stops working.

As for the \x7f in 3b1b's code, I guess it's there to clear whatever is in the terminal already. You see him getting the amount of chars in the terminal window, and adding \x7f that amount of times to clear the screen

mitkonikov commented 1 day ago

@RickLuiken Can you send some sample code, so I can try?

What do you mean a few updaters? What kind of updaters?

mitkonikov commented 1 day ago

As for the \x7f in 3b1b's code, I guess it's there to clear whatever is in the terminal already. You see him getting the amount of chars in the terminal window, and adding \x7f that amount of times to clear the screen

This is true. Maybe this functionally is going to be needed in this extension as well. If there has been some error, that made it so the IPython shell has stopped executing. The characters will maybe remain in the terminal window? This has to be tested as well.

mitkonikov commented 1 day ago

Hi, I found this issue as well. I just tried the proposed solution by @mitkonikov. It seems to work most of the time, however when the window get a bit laggier (when you have a few updaters running), it stops working.

I replicated this by just waiting for 4 seconds.

This is my code:

from manimlib import *

class SubTensors(InteractiveScene):
    def construct(self):
        ## Interactions
        circle = Circle(radius = 2)
        self.play(DrawBorderThenFill(circle))
        self.wait(4)

        ## Remove
        self.remove(circle)

When I execute the first block, the wait for 4 seconds, makes the terminal busy, so if I don't wait until the end, and preview the first block again, the command will be written in the shell, but the ESC key is overlooked somehow. This is why I think, in case of executing, if the user clicks the preview again, it should first send a CTRL+C or something similar in order to stop executing and afterwards send the preview command. Let's try injecting CTRL+C in the PREVIEW_COMMAND..?

RickLuiken commented 1 day ago
from manimlib import *

class LaggyScene(Scene):

    def construct(self):
        ## create a bunch of boxes
        grid = Square().get_grid(100, 100, buff=0)
        grid.set_height(5)
        grid.set_stroke(WHITE)
        self.play(ShowCreation(grid))

        ## move the boxes using updaters
        position = ValueTracker(0)

        grid.add_updater(lambda m, position=position: m.move_to((position.get_value(), 0, 0)))

        self.play(position.animate.set_value(2), run_time=5)

        ## moves boxes back
        self.play(position.animate.set_value(0), run_time=5)

Here's a minimal example. I start running with manimgl laggyscene.py LaggyScene -se 10. Then run the second cell using the preview command. This one works fine with @mitkonikov's fix. After this animation, manim becomes a bit laggy. Running the third cell using the preview command then fails.

mitkonikov commented 1 day ago

Wow, I can replicate it as well. Even waiting for all of the commands to finish, the third block's command is not executed. Even with my fix.. :(

mitkonikov commented 1 day ago

This is why I think, in case of executing, if the user clicks the preview again, it should first send a CTRL+C

Sending just CTRL+C as \x03 like this:

const PREVIEW_COMMAND = `\x03\x0C checkpoint_paste()\x1b`; // \x0C is Ctrl + L

it stops the execution, but then it doesn't execute the new checkpoint_paste() command. You have to click once again, to execute the new checkpoint paste command.

It's so weird.

And this doesn't fix @RickLuiken's issue.

RickLuiken commented 1 day ago

One way of fixing it is delaying the newline by doing:

terminal.sendText(PREVIEW_COMMAND, false);
setTimeout(() => terminal.sendText(""), 100);

This is also done in this ipython vscode extension. However this adds the problem that we don't know how long the delay should be. 100 ms seems to solve my issues, but maybe even laggier scenes require more delay?

mitkonikov commented 1 day ago

This to me, suggests that the cursor is not being updated to the last position in time. But, as I can see, the VSCode's Terminal API doesn't allow for anything like "cursor position change" or waiting for something like this. I don't believe that the delay is even in ms, it should be very fast. Can you try pushing it to 1-10ms? I will try this on my machine and report the findings.

mitkonikov commented 1 day ago
terminal.sendText(PREVIEW_COMMAND, false);
setTimeout(() => terminal.sendText(""), 100);

This fix for me works. Even with only 50ms delay. However, going down to 10ms, it fails. So, it does need some time for the cursor to be fixed apparently. The delay shouldn't matter much, but we could add a config for it as well. By default, 100ms seems to be plenty.

VSCode has similar issues when writing to files. A delay there is required.

RickLuiken commented 1 day ago

On my slower laptop, 50ms seems to work sometimes. I've not seen 100ms fail. Adding a setting to change the delay and setting the default to 100ms seems okay to me!

EDIT: just after I send this, 100ms also started failing on my laptop 🙃