Navigation Menu

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

rofi-theme-selector: Store config in XDG_CONFIG_HOME if set #781

Merged
merged 2 commits into from Jun 3, 2018

Conversation

NeQuissimus
Copy link
Contributor

@NeQuissimus NeQuissimus commented Mar 11, 2018

When XDG_CONFIG_HOME is set and you use rofi-theme-selector, the new theme is config is stored in ${HOME}/.config/rofi/config.rassi but rofi reads from ${XDG_CONFIG_HOME}/rofi/config.rasi

This PR saves the config in the correct location.

@codecov-io
Copy link

codecov-io commented Mar 11, 2018

Codecov Report

Merging #781 into next will increase coverage by 0.1%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             next     #781     +/-   ##
=========================================
+ Coverage   73.06%   73.16%   +0.1%     
=========================================
  Files          37       37             
  Lines        9864     9869      +5     
=========================================
+ Hits         7207     7221     +14     
+ Misses       2657     2648      -9
Impacted Files Coverage Δ
lexer/theme-lexer.l 90.22% <0%> (-0.9%) ⬇️
source/widgets/textbox.c 81.52% <0%> (-0.59%) ⬇️
lexer/theme-parser.y 95.85% <0%> (ø) ⬆️
source/history.c 85.89% <0%> (+0.18%) ⬆️
test/helper-pidfile.c 60.86% <0%> (+1.77%) ⬆️
source/widgets/widget.c 74.1% <0%> (+3.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7f83fa3...ba08df8. Read the comment docs.

@patrick-motard
Copy link
Contributor

Good catch. I'm also wondering why this script doesn't look in $HOME/.config/rofi/themes for themes in the first place. I also noticed a PR in @DaveDavenport 's official theme repo that includes install scripts for the themes. It seems like a good idea to get these scripts all using the same location.

It seems like we've been using XDG so far, but haven't completely been consistent. Not sure if that's the case or not. All I know is that I use ~/.config/{application}/ for nearly all of my dotfiles. It seems to be what most applications do.

@sardemff7
Copy link
Collaborator

XDG_CONFIG_HOME is almost never set, so it was just overlooked when writing the selector script I guess. (rofi uses GLib which handles all that itself so it doesn’t have to care, so to speak.)

As for themes location, (“packaged”) themes are data, so ~/.local/share is a better place for them, and that’s what the theme selector cares about.
rofi looks in ~/.config/rofi too, for people having their own custom theme in their dotfile repo, but that is not considered a “packaged” theme, which is why the theme selector should not check there.

@sardemff7
Copy link
Collaborator

As for the fix, I think the common idiom is ${XDG_CONFIG_HOME:-${HOME}/.config}. IOW, if it’s set, it must be valid.

@patrick-motard
Copy link
Contributor

@sardemff7 thank you for the info. That's pretty cool that there's an idea of a "packaged" theme and a "non packaged" or "user's custom theme" concept. I've not heard of this in the documentation. Is there documentation on how to both publish and import a "packaged" theme?

Having that knowledge would help me a lot. I'm trying to learn as much as I can about themes and configuration and get it documented as I go along. I've created a page about themes in the wiki and have been adding to it as I learn new things. My goal is for it to be a complete reference for working with these concepts in rofi post 1.4.

Side note, what does "IOW" mean?

@patrick-motard
Copy link
Contributor

It would also be very cool if the rofi-theme-selector supported looking at user themes as an option. I would really like that as an optional feature.

@sardemff7
Copy link
Collaborator

IOW means “in other words”.

By “packaged” theme, I mean a theme installed by project A to be used by project B in a well-known location. Something the user should not touch (besides running whatever installer).

If the user wants the theme selector to find a theme, just treat it as a packaged theme and put it in the good location. A symlink should work, so you can keep the real file in ~/.config/rofi.

@carnager
Copy link
Collaborator

carnager commented Jun 3, 2018

While I agree that the script should honuor XDG_CONFIG_HOME, this needs some updates.

First of all, the if..else is not needed. Use ${XDG_CONFIG_HOME:-${HOME}/.config} instead.
In addition to that XDG_DATA_HOME should be honoured too, so the same applies for that variable.

@carnager carnager merged commit ca1ae5d into davatorium:next Jun 3, 2018
@NeQuissimus NeQuissimus deleted the XDG_CONFIG_HOME branch June 3, 2018 13:18
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants