Skip to content

Custom color themes and list command#2

Merged
Achno merged 3 commits intoAchno:mainfrom
abs3ntdev:main
Jul 29, 2024
Merged

Custom color themes and list command#2
Achno merged 3 commits intoAchno:mainfrom
abs3ntdev:main

Conversation

@abs3ntdev
Copy link
Contributor

Hello. I added the ability to add custom themes in a yaml file and added a command to list available themes. i wanted to use this program personally but i didn't want to have to open a pr and wait for merge any time i wanted to add an unsupported theme or change a color.

I look for $XDG_CONFIG_HOME/gowall/config.yml or $HOME/.config/gowall/config.yml if you want to standardize your config file or use a flag to pass in the config file path eventually that will need to change.

I made the parser just read hex strings and convert them to color.Color. you might want to consider doing that in your code so its easier to add themes to the codebase without converting them.

let me know if you dont like any of this or want something changed

example config file

themes:
  - name: "theme1"
    colors:
      - "#F5E0DC"
      - "#F2CDCD"
      - "#F5C2E7"
      - "#CBA6F7"
      - "#F38BA8"
      - "#EBA0AC"
      - "#FAB387"
      - "#F9E2AF"
      - "#A6E3A1"
      - "#94E2D5"
      - "#89DCEB"
      - "#74C7EC"
      - "#89B4FA"
      - "#B4BEFE"
      - "#CDD6F4"
      - "#BAC2DE"
      - "#A6ADC8"
      - "#9399B2"
      - "#7F849C"
      - "#6C7086"
      - "#585B70"
      - "#45475A"
      - "#313244"
      - "#1E1E2E"
      - "#181825"
      - "#11111B"
  - name: "othertheme"
    colors:
      - "#F73253"
      - "#FA39DF"
      - "#005382"
      - "#123456"

@Achno
Copy link
Owner

Achno commented Jul 29, 2024

Just woke up ,

I look for $XDG_CONFIG_HOME/gowall/config.yml or $HOME/.config/gowall/config.yml
I made the parser just read hex strings and convert them to color.Color

I was going to do that as well because it was beginning to get annoying to add the themes by hand manually, good job on that.

I don't really like the error handling specifically the continue keyword in general

ex.

	if tw.Name == "" || len(tw.Colors) == 0 {
			// skip invalid color
			continue
		}

Is it possible to refactor this to something else to somehow skip an iteration without continue ? If not dont think too much about it

Also please note that your pr is a bit big for me and its my first time reviewing pr's so its going to take a a day or so to merge after changes and testing :) Thanks for contributing.

Though i have to say this is a much needed change and a really cool one !

@Achno Achno added the Feature Further information is requested label Jul 29, 2024
@Achno Achno self-assigned this Jul 29, 2024
@Achno Achno merged commit d9e5420 into Achno:main Jul 29, 2024
@Achno
Copy link
Owner

Achno commented Jul 29, 2024

Decided to merge anyway i will refactor the continue keyword if i can

Refactored

		if !valid {
			continue
		}
		themes[strings.ToLower(theme.Name)] = theme

into

		if valid  && !themeExists(theme.Name) {
			themes[strings.ToLower(theme.Name)] = theme
		}

removing a continue statement and making sure no duplicate theme can be loaded , because that was possible with the previous code

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

Labels

Feature Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants