Skip to content

Conversation

@theteleforce
Copy link
Contributor

Motivation

In some Windows CLIs, the command

manim file.py ...

will generate the argv

C:\\path\\to\\manim.exe file.py ...

This would cause _from_command_line() to misleadingly return False, which completely prevents users from calling manim from the command line, as certain key fields like input_file would not be populated.

Explanation for Changes

The problem is fixed simply by adding checks for "manim.exe" and "manimcm.exe" to _from_command_line().

Testing Status

The issue was first noticed while running manim from the terminal on PyCharm Community 2019.2.6 with Python 3.8. Before patching, the command returned a FileNotFoundError; after patching, the command worked as documented.

Acknowledgement

In some Windows CLIs, the command 

`manim file.py ...`

will generate the argv

`C:\\path\\to\\manim.exe file.py ...`

This would cause _from_command_line() to misleadingly return False, which completely prevents users from calling manim from the CLI, as certain key fields like `input_file` would not be populated.
@leotrs
Copy link
Contributor

leotrs commented Aug 23, 2020

@AetherBreeze thank you very much for reporting and fixing this bug, we do appreciate it!

@leotrs
Copy link
Contributor

leotrs commented Aug 23, 2020

Seeing as this is such an important part of manim, we should totally add a test for this cc. @huguesdevimeux

Also, I think either @Aathish04 or @naveen521kk work on Windows, yes? How is it that we missed this? Is it a problem of using different command prompts?

EDIT: I'm merging this now since it's so important, but I'd still like to discuss my questions above!

@leotrs leotrs merged commit 06602b7 into ManimCommunity:master Aug 23, 2020
@leotrs
Copy link
Contributor

leotrs commented Aug 23, 2020

@AetherBreeze congratulations on your first contribution to ManimCommunity! We look forward to more from you :)

@naveen521kk
Copy link
Member

How is it that we missed this? Is it a problem of using different command prompts?

I actually didn't face any problems on that. I don't know what really happens maybe I should look into it.

@huguesdevimeux
Copy link
Member

Seeing as this is such an important part of manim, we should totally add a test for this cc. @huguesdevimeux

Also, I think either @Aathish04 or @naveen521kk work on Windows, yes? How is it that we missed this? Is it a problem of using different command prompts?

EDIT: I'm merging this now since it's so important, but I'd still like to discuss my questions above!

Added this to #193

@huguesdevimeux huguesdevimeux mentioned this pull request Aug 23, 2020
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants