Skip to content

Commit

Permalink
Fix TextClip.list("color") returning an incorrect list of byte object…
Browse files Browse the repository at this point in the history
…s, not strings (#1119)

* Fix TextClip.list("color") returning an incorrect list of byte objects, not strings, causing TextClip.find("", "color") to fail as well.
  • Loading branch information
tburrows13 authored Apr 4, 2020
1 parent 7353216 commit 4106b4c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 9 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed <!-- for any bug fixes -->
- When using `VideoClip.write_videofile()` with `write_logfile=True`, errors would not be properly reported [#890]
- `TextClip.list("color")` now returns a list of bytes, not strings [#1119]
- `TextClip.search("colorname", "color")` does not crash with a TypeError [#1119]


## [v1.0.2](https://github.com/zulko/moviepy/tree/v1.0.2) (2020-03-26)
Expand Down
14 changes: 7 additions & 7 deletions moviepy/video/VideoClip.py
Original file line number Diff line number Diff line change
Expand Up @@ -1269,8 +1269,8 @@ def __init__(

@staticmethod
def list(arg):
"""Returns the list of all valid entries for the argument of
``TextClip`` given (can be ``font``, ``color``, etc...) """
"""Returns a list of all valid entries for the ``font`` or ``color`` argument of
``TextClip``"""

popen_params = {"stdout": sp.PIPE, "stderr": sp.DEVNULL, "stdin": sp.DEVNULL}

Expand All @@ -1280,23 +1280,23 @@ def list(arg):
process = sp.Popen(
[get_setting("IMAGEMAGICK_BINARY"), "-list", arg], **popen_params
)
result = process.communicate()[0]
result = process.communicate()[0].decode()

This comment has been minimized.

Copy link
@mgaitan

mgaitan Apr 5, 2020

Collaborator

would be better to pass encoding="utf8" in the popen_params

This comment has been minimized.

Copy link
@tburrows13

tburrows13 Apr 5, 2020

Author Collaborator

Good point. I'll make a new PR.

lines = result.splitlines()

if arg == "font":
return [l.decode("UTF-8")[8:] for l in lines if l.startswith(b" Font:")]
return [l[8:] for l in lines if l.startswith(" Font:")]
elif arg == "color":
return [l.split(b" ")[0] for l in lines[2:]]
return [l.split(" ")[0] for l in lines[5:]]

This comment has been minimized.

Copy link
@mgaitan

mgaitan Apr 5, 2020

Collaborator

what does that slicing mean? could you add a comment with an example?

else:
raise Exception("Moviepy:Error! Argument must equal " "'font' or 'color'")
raise Exception("Moviepy Error: Argument must equal 'font' or 'color'")

@staticmethod
def search(string, arg):
"""Returns the of all valid entries which contain ``string`` for the
argument ``arg`` of ``TextClip``, for instance
>>> # Find all the available fonts which contain "Courier"
>>> print ( TextClip.search('Courier', 'font') )
>>> print(TextClip.search('Courier', 'font'))
"""
string = string.lower()
Expand Down
18 changes: 16 additions & 2 deletions tests/test_TextClip.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,22 @@
from .test_helper import FONT


def test_fontlist():
TextClip.list("font")
def test_list():
fonts = TextClip.list("font")
assert isinstance(fonts, list)
assert isinstance(fonts[0], str)

colors = TextClip.list("color")
assert isinstance(colors, list)
assert isinstance(colors[0], str)
assert "blue" in colors


def test_search():
blues = TextClip.search("blue", "color")
assert isinstance(blues, list)
assert isinstance(blues[0], str)
assert "blue" in blues


def test_duration():
Expand Down

0 comments on commit 4106b4c

Please sign in to comment.