Menu

#56 Proposal re: icon organization

Git
closed-accepted
nobody
None
5
2024-05-18
2023-07-12
FeRD
No

(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...

  • First,do this:
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
  • ...And I'll also have to make a bunch of related tweaks to the new icon 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/.

Discussion

  • FeRD

    FeRD - 2023-07-12

    Whoops, make that first script...

    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
    
     
  • FeRD

    FeRD - 2023-07-14

    Bueller? ...Bueller? ...Bueller?

    Actually, @oddegamra? @aaku? Santa Claus?

     
  • Oddegamra

    Oddegamra - 2023-07-14

    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.

     
    • FeRD

      FeRD - 2023-07-14

      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 using pkgutil.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.

       
  • FeRD

    FeRD - 2024-05-12

    This can be closed, as it was dealt with in @oddegamra's commit e671dce and in my MR #43.

     
  • Oddegamra

    Oddegamra - 2024-05-18
    • status: open --> closed-accepted
     

Log in to post a comment.