(Flagrantly abusing the Patches category once again, to discuss another proposed set of changes that will (if we reach consensus) result in an MR...)
As I've already been fiddling with icons lately, I figure it makes me the logical choice to address this, and now the right time to address it.
mypy reminded me of one issue with the icon-packaging solution from MR #33: directory names like 16x16
and 48x48
aren't actually legal Python package names, whether or not they contain an __init__.py
file. Oops.
And the thing is, the only reason those directories are named like that is because they're the install sources for the freedesktop icon files, which all have to be named mcomix.png
, and all installed to different paths depending on size.
The installation already has a set of directories set up to deal with that, in fact, in the mime/icons/
hierarchy. Which is outside of the package itself, and therefore won't cause any problems using directory names like 48x48
.
So, I propose to get rid of the mcomix/images/nxn/
directories from the package space. Instead, the various icon sizes that mcomix
uses internally (from the Python code) can all be stored one directory up, as files named mcomix/images/mcomix_n.png
.
The installable icon set, OTOH, can be moved into mime/icons/
alongside the mimetype icons, and setup.py
can source them from there when installing to freedesktop dirs.
It'll mean that the repo contains two copies of a few of the icon files (every mcomix/images/mcomix_*.png
will be a duplicate of some mime/icons/*/mcomix.png
file, but given their tiny size that really doesn't seem worth losing sleep over. And we don't even need to duplicate all of the icon sizes, currently mcomix.icons
only uses the 16px, 32px, and 48px ones.
Basically, I propose...
for i in 16 22 24 32 48 64; do
git mv mcomix/images/${i}x${i}/mcomix.png \
mime/images/icons/${i}x%{i}/
done
for i in 16 32 48; do
cp mime/icons/${i}x${i}/mcomix.png \
mcomix/images/mcomix_${i}.png
done
Then, update mcomix/icons.py
and setup.py
to look for the files they consume under new names and/or paths, so everything can still be accessed.
Finally, clean up what's left over...
for i in 16 22 24 32 48 64; do
git rm mcomix/images/${i}x${i}/__init__.py
done
Makefile
in MR #35, either before or after it's merged....Thoughts, @oddegamra, @aaku? IMHO it makes the most sense. The Python package doesn't need those freedesktop subdirectories anyway, they're just needless clutter that can stay in mime/icons/
.
Whoops, make that first script...
Bueller? ...Bueller? ...Bueller?
Actually, @oddegamra? @aaku? Santa Claus?
I think those images being in a Python package in the first place is somewhat related to the way setuptools/pkg_resources worked in the past. Loading resource files from known locations required these files to be placed in directories that could be located using Python module mechanisms. I haven't looked into this very much, but if such hacks are can be avoided nowadays, that would be the best solution. If not, then your proposal would probably be the next best solution.
I think it'll work out.
Setuptools already installs the MIME icons from a directory outside the package, so installing the application icons from the same place is no different. And the icons inside the package will still be there for
mcomix.icons
to load usingpkgutil.get_data()
(hence the duplicate copies) — there's just no need to follow freedesktop's annoying naming rules, for those, since they're incompatible with Python's package-naming scheme.I'll whip up an MR, that'll be the real test.
This can be closed, as it was dealt with in @oddegamra's commit e671dce and in my MR #43.