Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't touch system-wide files when run as user. #180

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

loathingKernel
Copy link

When Numix Folders application is run as a regular user, and the themes are installed system-wide, instead of replacing files in system-wide, create a new theme which inherits the base theme specified by the user. This helps with package managers that expect for files installed in /usr to remain untouched. Also helps with package upgrades of the base themes.

Although it works, I haven't tested it thoroughly yet, so I consider it work-in-progress. It is quite slow for now, possibly because most string manip is done in bash or inefficiently. Still, I would like to get some more eyes on it.

@palob palob added the merge label Mar 10, 2018
@palob
Copy link
Member

palob commented Mar 10, 2018

Thanks for your first steps into this direction. It's needed in order to address #101, #159 and #162 and it would obviate the need for #166.

@loathingKernel loathingKernel changed the title Don't touch system-wide files when run as user. WIP - Don't touch system-wide files when run as user. Mar 11, 2018
@loathingKernel
Copy link
Author

loathingKernel commented Mar 12, 2018

I have an issue with large icons (96 and 128 I believe) when using custom colors. If someone more knowledgeable of the theme could take a look at what is possibly missing, it would be greatly appreciated.

Edit: It seems to be unrelated to this PR as the file lists generated in both cases are identical.

@benburrill
Copy link

Why use ~/.icons as the destination dir as opposed to the more xdg-ish ~/.local/share/icons? Is ~/.icons more compatible or something?

@loathingKernel
Copy link
Author

There wasn't any specific reason I chose ~/.icons over the xdg equivalent, just force of habit.

@loathingKernel loathingKernel changed the title WIP - Don't touch system-wide files when run as user. Don't touch system-wide files when run as user. Nov 25, 2019
Copy link
Contributor

@Foggalong Foggalong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very belatedly I'm testing this now and I generally like it other than that it's quite slow during the symlink creation stage. Not sure what part of the changes is the bottleneck there but some testing needed.

Also yeah, ~/.icons needs switching for ~/.local/share/icons just to remain standards compliant but that's an easy enough switch.

@Foggalong
Copy link
Contributor

Bug which arises with this method is that any symlinks not included in the created "Numix Folders" theme aren't being picked up by the inheritance system. Not sure why that's the case but further testing there needed also.

@loathingKernel
Copy link
Author

loathingKernel commented Mar 21, 2020

The bottleneck is the amount of symlinks and that find is invoked all the time. That combined with that fact that it is doing string manipulation in bash (which is still faster than invoking external programs) makes up most of the execution time. I don't think that the current solution as it is can do any better, it will require a change of method, possibly a rewrite. If so, I would suggest to move it to Python or something similar for better code clarity and possibly performance.

The issue with the custom folder colours not being used in large sized icons is due to those icons being symlinked from the system-wide Numix folder as they don't exist in the Numix-Folders. A solution could be to copy those files, either in the script or by including them in Numix-Folders.

A solution could also be to just copy everything to the local folder on top of each other and be done with it. I will also be faster, but the script should be run then on every update and more storage space will be required.

On a second look, there are a few more tricks that can be done to fix all of these.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

4 participants