Html export - #104
Conversation
|
This is absolutely fantastic, thank you so much @wjs018. Everything seems to be up to the project's standards after a full review, I really appreciate your effort in that regard. This will make updating the documentation & licenses really easy for me. I left a few comments in the code review, but they are very minor so I will accept this as-is and work on those separately. Thank you again for your efforts with this, much appreciated. |
Breakthrough
left a comment
There was a problem hiding this comment.
Follows project's standards quite well, clean implementation. Just a few minor comments, but I can address those pending your responses.
Thank you for both tackling this feature, and taking so much care with your implementation (I am very pleased with the quality of the code in this pull request!).
| '%d' % duration.get_frames(), duration.get_timecode(), '%.3f' % duration.get_seconds()]) | ||
|
|
||
|
|
||
| def write_scene_list_html(output_html_filename, scene_list, cut_list=None, css=None, css_class='mytable', |
There was a problem hiding this comment.
Would like to have this function in it's own .py file, just because it has nothing to do with the SceneManager class itself. That being said, this is something I can tackle myself - just a small comment :)
There was a problem hiding this comment.
That makes sense. I put it there because this is also where the write_scene_list function is that writes the csv file. I basically tried to make these two functions the same. One minor thing is that the write_scene_list function takes a file handle, but the html version just takes a filepath. This is because the simpletable code does the opening of the file and I thought it easier to have this discrepancy than rewriting the simpletable code at the time. If these two functions get consolidated, it might make sense to standardize the file handling as well.
There was a problem hiding this comment.
Good points @wjs018, I'll make a note to see if I can modify write_scene_list to also just take the output file path as a string, instead of a file handle, to make it consistent. Also good point in that write_scene_list and write_scene_list_html belong together - perhaps I will move them into a new sub-module (e.g. export.py or scene_export.py - I'm not sure which is better, or if there's a better name - feel free to choose or suggest one!).
As discussed in #17 this allows for exporting the scene list and optionally include the images from the
save-imagescommand in the html output.