Repository navigation
Fix save-html --no-images ignoring -o - #583
vishal-tambi wants to merge 4 commits into
Conversation
Breakthrough
left a comment
Breakthrough
left a comment
There was a problem hiding this comment.
with --no-images, save-html ignores -o. CliContext.save_images_result starts as (None, None) instead of None, so save_html takes its output folder from a save-images run that never happened. I can fix that in a separate PR if you'd like.
Thanks for kicking this off. We should change that behaviour to fix this properly here. By -o do you mean the global or local directory option (or both)?
|
Just the global |
vishal-tambi
changed the title
Should we add one as well? |
Breakthrough
left a comment
Breakthrough
left a comment
There was a problem hiding this comment.
LGTM, will merge after you take a look at last set of comments/questions. Thanks for the PR!
| self.stats_manager: StatsManager | None = None | ||
| self.save_images: bool = False # True if the save-images command was specified | ||
| self.save_images_result: ty.Any = (None, None) # Result of save-images used by save-html | ||
| self.save_images_result: ty.Any = None # Result of save-images used by save-html |
There was a problem hiding this comment.
Good find, this is quite subtle. Should we make the type hint here ty.any | None?
This branch has not been deployed
2 participants
save-html --no-imagesignored the global-oand wrote the HTML to the working directory, which is whytest_cli_save_htmlleftgoldeneye-Scenes.htmlin the repo root.CliContext.save_images_resultstarted as(None, None)instead ofNone, sosave_htmltook its output folder from asave-imagesrun that never happened. It now starts asNone, and the test checks that the file lands in the-ofolder.