Closed VladimirFokow closed 1 week ago
By the way, on another branch
–– I've added the run_scene
command, BUT:
I'm thinking that that change is under CC-BY-NC-SA-4.0 license (so I haven't made a PR from that branch yet),
because it's adapted from repo 3b1b/videos
(license discussion)
Maybe this workflow can be re-written, to be free from that license? Although Grant is saying he's mainly worried about others using "videos, and the assets" and "directly using content" which I'm not doing here, but still bound by that license..
Update (from that discussion) : an adaptation of the workflow itself - can be without the CC license
Neat stuff! I'm currently traveling and I won't be able to look into this until likely early next week, but thank you for the contribution!
Wow, thanks for the quick implementation. The extension.ts
file already gets quite large, maybe it'd be useful to extract the individual commands to separate files for better readability? Maybe there's also some shared behavior between commands that could also be outsourced?
The extension.ts file already gets quite large
On the other hand, spreading things over different places will make it harder to follow each one.
extension.ts
is straightforward - it just defines commands 1 by 1,
and inside each command you just read linearly from top to bottom to understand 100% what it's doing.
Right now there isn't much overlap,
there may be some shared parts in the future, similar to manim_plugins.py
, e.g. send_terminus_command
is used in several places:
https://github.com/3b1b/videos/blob/master/sublime_custom_commands/manim_plugins.py
But after all, I don't have strong opinions - I'd support what's fastest for everyone involved.
On the other hand, spreading things over different places will make it harder to follow each one.
I think quite the contrary is true ;) For me, it's easier to follow if the file is manageable and only contains of the things I need to look at (and no noise) — high cohesion, low coupling and separation of responsibilities. But maybe that's not too much of a problem yet in this early stage of the extension.
Of course, this is up to bhoov to decide in the end. I just think that it would make sense to watch out for this in the long-term run and to generally aim for high code quality right from the start to avoid aggregation of costs later on.
extension.ts
is straightforward - it just defines commands 1 by 1, and inside each command you just read linearly from top to bottom to understand 100% what it's doing.
I am in general in favor of this style by @VladimirFokow , provided we all use VSCode's code folding functionality to fold away each individual function that we don't work on, I would prefer to keep everything in one file. The editor ends up looking pretty clean. In your contributions for new functionality, please add a descriptive comment to each new command in a similar fashion
Note that it might be worth to first merge #7, then move the code that is adjusted in this PR (#5) to the respective merge #8.previewCode()
method.
Ended up integrating this into my local branch and doing some cleanup. I changed no functionality, works great. Thanks @VladimirFokow !
Great! I'm happy to have contributed! 👍
Addresses #3, and also makes some improvements.
Added steps 1. and 3. Now the order is:
Also added:
some error checking (e.g. you can't run >1 commands at the same time, but it's fast and dirty: it only waits for 0.5 sec. It doesn't actually detect when the animation has stopped playing! This can be an idea for future improvement)
if nothing is selected -> run the whole line where the cursor is
if something is selected (e.g. multiple lines) - extend selection to the start and end of respective lines (much more convenient - no need to accurately drag the selection to the edges)
after running
checkpoint_paste()
- the cursor now automatically goes down 1 line (for convenience, if you want to execute lines in order - now there's no need to manually move the cursor down 1 line after each run)added runing
cmd+L
beforecheckpoint_paste()