end-4 / dots-hyprland

i hate minimalism so...
https://end-4.github.io/dots-hyprland-wiki/en/
GNU General Public License v3.0
3.98k stars 267 forks source link

Consistently use XDG base directiories #381

Closed Cu3PO42 closed 4 months ago

Cu3PO42 commented 6 months ago

Hi! First off: thank you for the wonderful work you've done in this project! Everything looks so extremely polished.

This PR serves two main functions:

First, it consistently uses the XDG base directory spec. Before, it was already used in many places (via the GLib functions get_user_xyz_dir), but not everywhere. Other locations used the default values for these paths directly (e.g. ~/.config and ~/.cache). While it is rare to see a system that changes these defaults, it could happen and the inconsistent handling would break your config.

Further, it moves some files from XDG_CONFIG_HOME and XDG_CACHE_HOME to XDG_STATE_HOME. This happens for two different reasons:

I have done my best to seperate these changes into smaller commits that make sense individually.

I did not spot any regressions, but I'm not familiar with all of the features, so it's possible I missed some. In particular, much of the dynamic color feature does not currently/not yet work on my NixOS setup, therefore my tests pertaining it are rather superficial.

clsty commented 6 months ago

However, some files stored there (such as firstrun.txt, todo.json) really should be kept.

install.sh will overwrite everything except user_options.js in the ~/.config/ags folder. Please post a list of files that should not be overwritten by install.sh for this PR. If it's merged, I'll handle the rest.


There's a problem though. I found that on my Arch Linux,

echo "1$XDG_CONFIG_HOME 2$XDG_CACHE_HOME 3$XDG_STATE_HOME"

Outputs 1 2 3 which means those vars are empty.

According to Arch Wiki, it seems that these vars are not set by default.

To solve this problem, if it's in a bash script, we can use ${XDG_CONFIG_HOME:-~/.config} to provide a fallback value for it. Though this is in js file for AGS, I'm not sure will it work.

Cu3PO42 commented 6 months ago

I don't believe install.sh currently poses any problem here. These files (firstrun.txt, ...) I was referring to were previously placed in XDG_CACHE_HOME (which may be deleted, albeit not by install.sh), but are now moved to XDG_STATE_HOME. This directory was not used by illogical-impulse prior to this PR and should not be affected by install.sh.

I do appreciate your help and offer, though.


As far as these variables not being defined, yes, that can happen and is perfectly compliant with the spec. The fix you propose is already included in this PR. If you check the first lines of the shell scripts I changed, you will find I have added the block

XDG_CONFIG_HOME="${XDG_CONFIG_HOME:-$HOME/.config}"
XDG_CACHE_HOME="${XDG_CACHE_HOME:-$HOME/.cache}"
XDG_STATE_HOME="${XDG_STATE_HOME:-$HOME/.local/state}"

This is done simply so we don't have to include the default value everywhere and keep the rest of the file easier to read.

As far as code in AGS is concerned, it doesn't need any particular workaround: GLib.get_user_*_dir() checks and uses the environment variable if it is defined and uses the same defaults otherwise.

clsty commented 6 months ago

As far as code in AGS is concerned, it doesn't need any particular workaround: GLib.getuser*_dir() checks and uses the environment variable if it is defined and uses the same defaults otherwise.

As far as I know, the environment variables of a process are only passed to its sub-process, which means if AGS is running some bash scripts containing those default values for XDGs, AGS will not able to know these values, since AGS is their parent process.

I don't know well about AGS. Maybe you are right.

Cu3PO42 commented 6 months ago

As far as I know, the environment variables of a process are only passed to its sub-process

You are right that these variables are not passed to AGS. These variables aren't even passed to child processes: that only happens to variables marked with export. These fallback definitions as I have written them will not affect any other processes, they are relevant for the individual scripts.

However, this is fully intended and not a problem. Every script has its own fallback definitions and AGS does not explicitly need the fallbacks. The behavior of the built-in functions GLib.get_user_*_dir does the necessary computation on its own: if XDG_*_HOME is defined, it will use that variable, otherwise it will use the default values defined by the specification. These are ~/.config, ~/.cache, and ~/.local/state. These are also the defaults defined in the scripts. Therefore, the same, correct directories will be used even if XDG_*_HOME are not set.

Cu3PO42 commented 4 months ago

Hi! I just rebased this on current main and fixed any issues that cropped up (that I could find).

Is there anything I could do to help get this merged?

Cu3PO42 commented 4 months ago

Unfortunately, I have just found a potential issue with how the SCSS is handled. I'll look into it and patch it asap.

Cu3PO42 commented 4 months ago

It turns out this line in the dart-sass docs is only mildly accurate:

This option (abbreviated -I) adds an additional load path for Sass to look for stylesheets. It can be passed multiple times to provide multiple load paths. Earlier load paths will take precedence over later ones.

The current directory always wins over any load paths. Therefore I had to move _material.scss to a new folder; currently scss/fallback. It is applied whenever no other colors have been generated yet. I don't know if there's any sensible default beyond "whatever is currently in the repository".

end-4 commented 4 months ago

ugh when i try this styling gets all messed up

Cu3PO42 commented 4 months ago

Thanks for trying it! It seems fine on my end:

image

Could you check if ~/.cache/ags/user/generated/style.css contains some error message?

end-4 commented 4 months ago

Thanks for trying it! It seems fine on my end:

image

Could you check if ~/.cache/ags/user/generated/style.css contains some error message?

/* Error: Can't find stylesheet to import.
 *   ,
 * 2 | @import 'musicmaterial';
 *   |         ^^^^^^^^^^^^^^^
 *   '
 *   .config/ags/scss/_music.scss 2:9  @import
 *   .config/ags/scss/main.scss 27:9   root stylesheet */

body::before {
  font-family: "Source Code Pro", "SF Mono", Monaco, Inconsolata, "Fira Mono",
      "Droid Sans Mono", monospace, monospace;
  white-space: pre;
  display: block;
  padding: 1em;
  margin-bottom: 1em;
  border-bottom: 2px solid black;
  content: "Error: Can't find stylesheet to import.\a   \2577 \a 2 \2502  @import 'musicmaterial';\a   \2502          ^^^^^^^^^^^^^^^\a   \2575 \a   .config/ags/scss/_music.scss 2:9  @import\a   .config/ags/scss/main.scss 27:9   root stylesheet";
}
Cu3PO42 commented 4 months ago

No need to be sorry! I'm thankful you're developing this!

I believe I know the issue and why it's working for me and not for you. I forgot to create ~/.local/state/ags/scss if it doesn't already exist. I must have inadvertently done that and not realized that step is missing. I'll push a fix for this.

end-4 commented 4 months ago

seems good, i'll merge

end-4 commented 4 months ago

is there anything else that needs to be done to make sure nix users are happy? (except providing a flake.nix since i'm not capable)

Cu3PO42 commented 4 months ago

Thanks for merging!

I have a lot of things working right now in a Nix environment. You can simply run nix run github:Cu3PO42/gleaming-glacier/next#illogical-impulse. Some system requirements need to be satisfied externally, for example I can't put a user into groups from a simple Nix package. This also only starts AGS in a configured state, it does not set up Hyprland or anything else.

If one also (manually) applied your other configs, I imagine dynamic color generation would work. In my setup it works only for AGS, but not for Hyprland, since my Hyprland config is also read-only. I'm still working on making more things work and as I discover ways to do that, I'll discuss and submit more patches.

EDIT: The code for my Nix package is here (technically also here and here).