fix(lib): prevent filename collision (#429)

* fix(lib): prevent filename collision

Apparently, ManimCE can produce two different animations with the same name (i.e., the same hash). As documented, ManimGL would any produce files with the same name so this fix was needed.

Closes #428

* chore(lib): update comment

chore(lib): update comment

* chore(tests): add test

* chore(tests): remove redundant underscore

* chore(docs): add changelog entry
This commit is contained in:
Jérome Eertmans
2024-05-18 10:17:25 +02:00
committed by GitHub
parent 993acf0e3f
commit b08073983b
4 changed files with 57 additions and 5 deletions

View File

@ -10,6 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
(unreleased)= (unreleased)=
## [Unreleased](https://github.com/jeertmans/manim-slides/compare/5.1.7...HEAD) ## [Unreleased](https://github.com/jeertmans/manim-slides/compare/5.1.7...HEAD)
(unreleased-fixed)=
### Fixed
- Fix combining assets from multiple scenes to avoid filename collision.
[#429](https://github.com/jeertmans/manim-slides/pull/429)
(v5.1.7)= (v5.1.7)=
## [v5.1.7](https://github.com/jeertmans/manim-slides/compare/v5.1.6...v5.1.7) ## [v5.1.7](https://github.com/jeertmans/manim-slides/compare/v5.1.6...v5.1.7)

View File

@ -324,15 +324,19 @@ class PresentationConfig(BaseModel): # type: ignore[misc]
f.write(self.model_dump_json(indent=2)) f.write(self.model_dump_json(indent=2))
def copy_to( def copy_to(
self, folder: Path, use_cached: bool = True, include_reversed: bool = True self,
folder: Path,
use_cached: bool = True,
include_reversed: bool = True,
prefix: str = "",
) -> "PresentationConfig": ) -> "PresentationConfig":
"""Copy the files to a given directory.""" """Copy the files to a given directory."""
for slide_config in self.slides: for slide_config in self.slides:
file = slide_config.file file = slide_config.file
rev_file = slide_config.rev_file rev_file = slide_config.rev_file
dest = folder / file.name dest = folder / f"{prefix}{file.name}"
rev_dest = folder / rev_file.name rev_dest = folder / f"{prefix}{rev_file.name}"
slide_config.file = dest slide_config.file = dest
slide_config.rev_file = rev_dest slide_config.rev_file = rev_dest

View File

@ -404,8 +404,28 @@ class RevealJS(Converter):
full_assets_dir.mkdir(parents=True, exist_ok=True) full_assets_dir.mkdir(parents=True, exist_ok=True)
for presentation_config in self.presentation_configs: num_presentation_configs = len(self.presentation_configs)
presentation_config.copy_to(full_assets_dir, include_reversed=False)
if num_presentation_configs > 1:
# Prevent possible name collision, see:
# https://github.com/jeertmans/manim-slides/issues/428
# With ManimCE, this can happen when caching is disabled as filenames are
# 'uncached_000x.mp4'
# With ManimGL, this can easily occur since filenames are just basic integers...
num_digits = len(str(num_presentation_configs - 1))
def prefix(i: int) -> str:
return f"s{i:0{num_digits}d}_"
else:
def prefix(i: int) -> str:
return ""
for i, presentation_config in enumerate(self.presentation_configs):
presentation_config.copy_to(
full_assets_dir, include_reversed=False, prefix=prefix(i)
)
dest.parent.mkdir(parents=True, exist_ok=True) dest.parent.mkdir(parents=True, exist_ok=True)

View File

@ -153,6 +153,28 @@ class TestConverter:
file_contents = Path(out_file).read_text() file_contents = Path(out_file).read_text()
assert "manim" in file_contents.casefold() assert "manim" in file_contents.casefold()
@pytest.mark.parametrize("num_presentation_configs", (1, 2))
def test_revealjs_multiple_scenes_converter(
self,
tmp_path: Path,
presentation_config: PresentationConfig,
num_presentation_configs: int,
) -> None:
out_file = tmp_path / "slides.html"
RevealJS(
presentation_configs=[
presentation_config for _ in range(num_presentation_configs)
]
).convert_to(out_file)
assert out_file.exists()
assets_dir = Path(tmp_path / "slides_assets")
assert assets_dir.is_dir()
got = sum(1 for _ in assets_dir.iterdir())
expected = num_presentation_configs * len(presentation_config.slides)
assert got == expected
@pytest.mark.parametrize("frame_index", ("first", "last")) @pytest.mark.parametrize("frame_index", ("first", "last"))
def test_pdf_converter( def test_pdf_converter(
self, frame_index: str, tmp_path: Path, presentation_config: PresentationConfig self, frame_index: str, tmp_path: Path, presentation_config: PresentationConfig