Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CLI bugs #1431

Merged
merged 12 commits into from
May 2, 2021
Merged

Fix CLI bugs #1431

merged 12 commits into from
May 2, 2021

Conversation

jsonvillanueva
Copy link
Member

@jsonvillanueva jsonvillanueva commented May 1, 2021

Changelog / Overview

  • Fixed conflict with -f which was previously assigned to both --show_in_file_browser and --format by removing -f from --format. A warning is issued that -f will soon move to --format.
  • Added back in flags to render the files as gif/last frame. Deprecated them in favor of --format.
  • Fixed the broken --output_file/-o option.
  • Fixed an issue where the -qh quality option was interpreted as -q -h, prompting the help page.

Motivation

The format of the Scene(s) to render should be easily decidable under a single option: -f or --format.
As such, rendered media can now be selected via manim -f png [FILE], or more explicitly manim --format=gif [FILE]

Testing Status

Further Comments

It'd be nice to have this in the release so that we're not knowingly releasing a buggy new CLI, but I wish I had more time to implement test cases for this PR. If test cases are preferable, we can make a patch release later.

Checklist

  • I have read the Contributing Guidelines
  • I have written a descriptive PR title (see top of PR template for examples)
  • I have written a changelog entry for the PR or deem it unnecessary

Reviewer Checklist

  • The PR title is descriptive enough
  • The PR is labeled correctly
  • The changelog entry is completed if necessary
  • Newly added functions/classes either have a docstring or are private
  • Newly added functions/classes have tests added and (optional) examples in the docs
  • Newly added documentation builds, looks correctly formatted, and adds no additional build warnings

Fixed output_file option

Removed -h help flag from manim/manim render
@jsonvillanueva jsonvillanueva added pr:bugfix Bug fix for use in PRs solving a specific issue:bug pr:deprecation Deprecation, or removal of deprecated code labels May 1, 2021
@jsonvillanueva jsonvillanueva changed the title First draft at fixing CLI bugs Fix CLI bugs May 1, 2021
@behackl behackl mentioned this pull request May 1, 2021
Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the code, LGTM. (Didn't test locally yet, if someone else manages to use these changes successfully, we can merge it.)

Copy link
Member

@behackl behackl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I noticed, please have a look. I think we can merge this after this discussion is resolved.

manim/cli/render/render_options.py Outdated Show resolved Hide resolved
Co-authored-by: Benjamin Hackl <devel@benjamin-hackl.at>
manim/_config/utils.py Show resolved Hide resolved
Copy link
Member

@naveen521kk naveen521kk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@naveen521kk naveen521kk merged commit 7248cf5 into ManimCommunity:master May 2, 2021
@jsonvillanueva jsonvillanueva deleted the click-fix branch May 2, 2021 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:bugfix Bug fix for use in PRs solving a specific issue:bug pr:deprecation Deprecation, or removal of deprecated code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants