Titles: Rewrite XML parsing/mods, add style_tools #3816
Conversation
| ard = style_to_dict(s) | ||
| set_if_existing(ard, "font-style", self.font_style) | ||
| set_if_existing(ard, "font-family", self.font_family) | ||
| set_if_existing(ard, "font-size", f"{self.font_size_pixel}px") |
There was a problem hiding this comment.
Oh, yes, purely by coincidence this PR also introduces the first f-string info the OpenShot codebase, making it officially require Python 3.6 or later to run.
It's time. Python 3.5 has actually been end-of-life since September. And f-strings are just too damn convenient not to use!
src/windows/title_editor.py
Outdated
| def find_in_list(self, l, value): | ||
| '''when passed a partial value, function will return the list index''' | ||
| for item in l: | ||
| for i, item in enumerate(l): | ||
| if item.startswith(value): | ||
| return l.index(item) | ||
| return i |
There was a problem hiding this comment.
I also goosed the efficiency of find_in_list() a bit, since it's still used in a couple of parts of the code.
src/windows/title_editor.py
Outdated
| set_if_existing(ard, "font-family", self.font_family) | ||
| set_if_existing(ard, "font-family", f"'{self.font_family}'") |
There was a problem hiding this comment.
The old code wasn't doing this either, and it admittedly seems to work fine without it, but technically font family names with a space are supposed to be inside single quotes, in CSS. (No real harm in doing it for all font family names, even one-word ones.)
- New model design uses only a single column, all data for each template is stored on the same QStandardItem under different roles - TitleRoles.PathRole references the file path (former column 2) - Dialog now preserves user-supplied filename across launches of the external Advanced Editor process - TitlesModel is now a QObject, parented to MainWindow
|
LGTM! For some reason I thought this was already merged in!? Any-who, it looks great. Nice work! |
Update
Just pushed a few more commits that re-work the title editor data handling and template parsing. Now:
style_to_dict()is used for all style parsing (find_in_list()is now gone)TitlesModelwrapper class is now aQObject, and gets parented toMainWindowQStandardItemModel-based data model now only has one column. Each "row" in the model holds a self-containedQStandardItemwith all of the data for a given template.TitleRoles.PathRoleas the roleOriginal description
This PR grew out of my testing for the previous three PRs.
There was a lot of redundant code in the Title Editor source, most of it to do with the manipulation of the SVG file's contents to update the
style=attribute on various nodes.I hit on one major efficiency that really streamlined things.
The code was already doing the work to
split()the style attribute's string on;, to extract individual properties. But then it was doing a ton of laborious substring matching and whatnot to find the specific CSS properites it wanted to modify.What I realized was, just like the property list could be split apart using
;, the properties themselves could be split at the first:to form key-value pairs, and then naturally the whole thing could be loaded into adictwhich makes manipulation trivial.So, this PR adds
classes/style_tools.py, a module containing three functions:That last one (which I left out the docstring for, oops) has a pretty obvious name, and does what it says: It updates the value of a dictionary key ONLY IF that key already exists in the dictionary. If not, then it won't insert a new key. Some of the code in the Title Editor was concerned with only updating existing style attributes where they already existed.
Armed with the new style tools, I then rewrote the update-handling methods of the TitleEditor class to use them. The result is a drop of almost 100 lines of code, not to mention a ton of inefficiency and redundancy, from
title_editor.py