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

parcellite: fix GTK theme #932

Closed

Conversation

dermetfan
Copy link
Collaborator

@dermetfan dermetfan commented Dec 1, 2019

The gtk.theme option is not picked up due to hardcoded XDG_DATA_DIRS.

Since icons do show up without this environment variable in the service unit I don't think it is necessary (anymore). According to the basic test we hope XDG_DATA_DIRS is set anyway.

cc @gleber

@rycee
Copy link
Member

rycee commented Dec 1, 2019

Yeah, seems these could safely removed. As I mentioned in the line comment I think you can remove PATH as well. If you do then you could rename the commit to

parcellite: remove obsolete backward compatibility fixes

This also fixes that the `gtk.theme` option was not picked up due to hardcoded XDG_DATA_DIRS.
@dermetfan
Copy link
Collaborator Author

Nice, I kept it to err on the safe side, but this is cleaner.

I was going to merge this myself to spare you a pointless review, but now I'm uncertain because the Travis build failed. It seems that's not an issue to address in this PR though.

@rycee
Copy link
Member

rycee commented Dec 1, 2019

Yeah, the CI error is unrelated. I'm not certain what it is about but asked in the #nixos IRC channel a while ago, hopefully somebody knows.

@rycee
Copy link
Member

rycee commented Dec 1, 2019

I rebased the commit to master in fdd65e5. Note, I wrapped the commit message to make it a bit less wide.

@rycee rycee closed this Dec 1, 2019
@dermetfan dermetfan deleted the fix/parcellite-gtk-theme branch December 1, 2019 19:07
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants