Skip to content

Conversation

@SirJamesClarkMaxwell
Copy link
Contributor

@SirJamesClarkMaxwell SirJamesClarkMaxwell commented Oct 24, 2025

It adds a sanity check in CodeMobject constructor during the coloring of patrs the text that style has "color: " string, if not it sets color to None by default. This PR is inspired by the post on the help-formu https://discord.com/channels/581738731934056449/1431310500125212753

Overview: What does this pull request change?

Motivation and Explanation: Why and how do your changes improve the library?

Links to added or changed documentation pages

Further Information and Comments

Reviewer Checklist

  • The PR title is descriptive enough for the changelog, and the PR is labeled correctly
  • If applicable: newly added non-private functions and classes have a docstring including a short summary and a PARAMETERS section
  • If applicable: newly added functions and classes are tested

@henrikmidtiby
Copy link
Contributor

I have now looked a bit closer to your solution. It works in my tests, but I think that there is an even better solution for the issue.

My test case is the following code

from manim import *

class CodeStyle(Scene):
    def construct(self):
        styles = VGroup()
        for style in Code.get_styles_list():
            print(style)
            styles.add(
                Code(
                    code_string="""
            # This is a comment.
            var = 3
            print(var)
            """,
                    formatter_style=style,
                )
            )
        self.add(styles.arrange_in_grid())

If I run this code on the main branch (4dd2937) I get the following error for the style algol.

ValueError: Color font-weight: bold; text-decoration: underline not found

I expect that this is the original issue that you tried to solve.

With the current version of this PR, everything seems to work ok.
But if you inspect the extracted color values, you find values like this (for the style state-dark)

None
#BBB
#777; font-style: italic
#BBB
#7686BB; font-weight: bold
#BBB
#CCC
#BBB
#4FB8CC
#BBB
#CCC
#7686BB; font-weight: bold
#CCC
#BBB

These values are then interpreted by ManimColor, which is robust enough to handle that gracefully.
It would be better if only the color values were extracted.

This is what the code suggested by maejam does. Now the extracted color codes are as follows:

None
#BBB
#777
#BBB
#7686BB
#BBB
#CCC
#BBB
#4FB8CC
#BBB
#CCC
#7686BB
#CCC
#BBB

Code suggested by maejam
@henrikmidtiby henrikmidtiby added pr:easy review There is nothing particular (i.e, it's about a general/small thing) to know for review! issue:bug Something isn't working... For use in issues and removed issue:bug Something isn't working... For use in issues labels Oct 25, 2025
@henrikmidtiby henrikmidtiby changed the title fixed problem: default value of color in styles in CodeMobject Better parsing of color styles in CodeMobject Oct 25, 2025
@SirJamesClarkMaxwell
Copy link
Contributor Author

SirJamesClarkMaxwell commented Oct 25, 2025

@henrikmidtiby I am okay with maejam solutions. In fact, it should be his PR, not mine. In this scenario, I only spotted the place of the bug. Do I need to do something with this PR to make it ready to merge?

@henrikmidtiby
Copy link
Contributor

@SirJamesClarkMaxwell I think it should be your PR, as you identified the issue and the first step towards a solution.
Right now we are awaiting the continuous integration tests. When they are completed I can merge the PR.

@henrikmidtiby henrikmidtiby self-requested a review October 25, 2025 20:20
Copy link
Contributor

@henrikmidtiby henrikmidtiby left a comment

Choose a reason for hiding this comment

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

LGTM

@github-project-automation github-project-automation bot moved this from 🆕 New to 👍 To be merged in Dev Board Oct 26, 2025
@henrikmidtiby henrikmidtiby merged commit 3e8f41c into ManimCommunity:main Oct 26, 2025
24 checks passed
@github-project-automation github-project-automation bot moved this from 👍 To be merged to ✅ Done in Dev Board Oct 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:easy review There is nothing particular (i.e, it's about a general/small thing) to know for review!

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants