ManimCommunity / ManimPango

Binding for Pango, to use with Manim.
https://manimpango.manim.community
MIT License
45 stars 13 forks source link

Add support for PangoAttributes #84

Closed naveen521kk closed 2 years ago

naveen521kk commented 2 years ago

This is still a WIP and would introduce a new API which would be required for https://github.com/ManimCommunity/manim/issues/2355

lgtm-com[bot] commented 2 years ago

This pull request introduces 2 alerts when merging 08d5dca136131656742fe83b8e966f999096a4fe into ff6debddf9a2c0ec8c1a5e79d3bf3f63c9b0b4e6 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging f851e10c30cc3579573233cc3452328c5db94d06 into ff6debddf9a2c0ec8c1a5e79d3bf3f63c9b0b4e6 - view on LGTM.com

new alerts:

lgtm-com[bot] commented 2 years ago

This pull request introduces 1 alert when merging 219ce03fee7bc984597b0abbfeade2696c672827 into ff6debddf9a2c0ec8c1a5e79d3bf3f63c9b0b4e6 - view on LGTM.com

new alerts:

PhilippImhof commented 2 years ago

I will have a look at this during the weekend.

naveen521kk commented 2 years ago

This PR introduces a new class called TextAttribute. This can't be used with the current API and would require an entirely new API for this to work. TextAttribute isn't fully done and misses a lot of attributes; that's for another day.

I think it would be better to just merge this because no one would be able to review this since this would be an entire rewrite of this project.

naveen521kk commented 2 years ago

I will have a look at this during the weekend.

I didn't see this comment before posting the previous comment, I think as I told you in the previous comment we can just merge it since reviewing it would be very hard. If you found any issues with this code making an issue would be better.

PhilippImhof commented 2 years ago

OK, that's fine for me. Go for it.

PhilippImhof commented 2 years ago

Oh, and thanks :)