html2image
html2image copied to clipboard
Added method to remove temporary files created
Hello,
I added the method _remove_tmp_file
which is supposed to remove the temporary files created when we supply a HTML string to screenshot()
instead of providing a file. This prevents too much files from accumulating in the temporary directory.
Feel free to change the name of the method or to tell your opinions about this PR :)
Hello, thanks for opening a PR, something like this is indeed missing from the package.
It could almost be merged as-is, but I think one should be able to choose whether or not they want the files to be automatically deleted. I can think of two reasons:
- If something went wrong with the screenshot process, taking a look at the HTML files in the temporary directory by opening them in a browser can be a way to see what the pages should look like.
- There could be some use-cases where one have to take a lot of screenshots and would like the process to finish as soon as possible. In this case, the deletion time (even if insignificant compared to the time it takes to take a screenshot) could adds up while taking screenshots, whereas the files could be deleted later without adding time to the screenshot-taking process.
Here is how I would add the possibility to toggle the automatic deletion:
- add an
auto_remove_tmp_files
(or a similar name) parameter to theHtml2Image
class'__init__
with a default value ofTrue
- add an attribute with the same name to the
Html2Image
class - add a simple
if
statement surrounding the call of the_remove_tmp_file
method - and finally add docstring and mention the attribute/parameter in the readme
On a side note : as-is, this method only deletes HTML files and leaves CSS files untouched. It is not necessarily bad however : a CSS file can be loaded one time and be used by an infinite number of HTML files, so deleting them may sometimes be counter-productive.
Still, being able to delete everything in the tmp folder using one command would be nice, it could be a function that would have to be called manually.
Anyway, thanks again for your PR. I wrote everything that came to my mind to make sure that I won't forget about it.
If you wish to add anything that I mentioned in this comment, feel free to do so. If not, I'll add it later myself to this PR.
I'm currently focusing on making this package compatible with Chrome's and Firefox's CDP interface (which is a way to control a web browser via websockets), so any unrelated changes will come after that, sooner or later.
I added an improvement commit:
- method is now called
_remove_temp_file
(for better consistency withtemp_path
) - an argument
keep_temp_file
is now present in the class constructor and initialized to False - added documentation in docstring and README
However, I am unsure to understand what you mean with CSS files: from my perspective, it looks like that only HTML files are created, since the CSS string we pass to the screenshot
function is embedded into the HTML file. Are there particular circumstances in which temporary CSS files (and anything else than HTML) are supposed to be created?