Skip to content

Conversation

@01micko
Copy link

@01micko 01micko commented Jan 30, 2022

This is my take on adding features to 'tweaks

Notes:

  • hard coded adwaita for cursors and gtk as it's builtin to gtk. Not much can be done about that
  • icon themes and everything else works pretty well
  • cursor theme isn't changing reliably so this is WIP

tweaks

If you get time @johanmalm or @Consolatis you could rewrite this correctly - I'm pretty much a novice in C, certainly know enough to be dangerous!

@01micko 01micko marked this pull request as draft January 30, 2022 06:44
@johanmalm
Copy link
Member

Just had a click look.

find_themes() is getting quite hard to follow. It might be better to create a separate find_cursor_themes() to deal with associated edge-cases there.

Would we not to set XCURSOR_THEME in ~/.config/labwc/environment for it to have an effect outside of GTK applications?

Some comments in the code regarding the hard-coded Adwaita would be helpful. I still get a /usr/share/themes/Adwaita/ on my box. What's the case you're trying to deal with by hard-coding it? Are we wanting add to Adwaita even if an theme directory does not exist? If so, it might be easier to keep find_themes() clean (i.e. no Adwaita check). Then after it's been called, check if the vector contains Adwaita - and if not, then explicitly add it.

@01micko
Copy link
Author

01micko commented Jan 31, 2022

Just had a click look.

find_themes() is getting quite hard to follow. It might be better to create a separate find_cursor_themes() to deal with associated edge-cases there.

Ok, agreed.

Would we not to set XCURSOR_THEME in ~/.config/labwc/environment for it to have an effect outside of GTK applications?

Ok, I'll have to walk through that and see if it's set and either edit it or append it

Some comments in the code regarding the hard-coded Adwaita would be helpful. I still get a /usr/share/themes/Adwaita/ on my box. What's the case you're trying to deal with by hard-coding it?

As far as I understood it was the default builtin gtk theme and it didn't show up in my tests so I hard coded it in. But now I suppose some distros separate it out (which makes sense).

Are we wanting add to Adwaita even if an theme directory does not exist?

Yes

If so, it might be easier to keep find_themes() clean (i.e. no Adwaita check). Then after it's been called, check if the vector contains Adwaita - and if not, then explicitly add it.

Ok :)

I'll find some time next week end to clean this up. For now I'll leave it as a 'draft' and am willing to accept anyone's critique.

Thanks for looking.

@01micko 01micko force-pushed the feature-icons-cursors branch from 8aaf1bd to 49ccb67 Compare February 4, 2022 10:23
@01micko 01micko requested review from a user and johanmalm February 4, 2022 10:24
@01micko
Copy link
Author

01micko commented Feb 4, 2022

Just had a click look.

find_themes() is getting quite hard to follow. It might be better to create a separate find_cursor_themes() to deal with associated edge-cases there.

Declined that, but find_themes() is simplified from my last effort.

Would we not to set XCURSOR_THEME in ~/.config/labwc/environment for it to have an effect outside of GTK applications?

Yes, and did so, but fails to be reloaded on my system. Need confirmation.

Some comments in the code regarding the hard-coded Adwaita would be helpful.

Done.

I still get a /usr/share/themes/Adwaita/ on my box. What's the case you're trying to deal with by hard-coding it? Are we wanting add to Adwaita even if an theme directory does not exist? If so, it might be easier to keep find_themes() clean (i.e. no Adwaita check). Then after it's been called, check if the vector contains Adwaita - and if not, then explicitly add it.

I don't have the GTK theme directory (Adwaita) so hard coding is the only option (I could see), but I softened it accordingly. Code comments amended accordingly.


I also took the liberty to adjust README reflecting the changes and add other comments to the code.

@01micko 01micko marked this pull request as ready for review February 4, 2022 10:39
@01micko 01micko changed the title Add icon themes and cursor themes [WIP] Add icon themes and cursor themes Feb 4, 2022
@01micko 01micko marked this pull request as draft April 24, 2022 10:36
@01micko
Copy link
Author

01micko commented Apr 24, 2022

Converted to draft until I find time to fix conflicts.

@johanmalm
Copy link
Member

Whoops. Forgot about this PR. Will have a look now to see if I can merge.

@01micko
Copy link
Author

01micko commented Apr 24, 2022

Nah, I'll fix it and force push when ready. :-)

@01micko 01micko closed this Apr 24, 2022
@01micko 01micko deleted the feature-icons-cursors branch April 24, 2022 11:05
@johanmalm
Copy link
Member

Whoops... read your note too late. Hopefully that's okay. Did a quick fix on a segfault.

@01micko
Copy link
Author

01micko commented Apr 24, 2022

All good, thanks for the help 😃

@johanmalm
Copy link
Member

Felt guilty 😄
Had completely forgotten about that PR. Short term memory is not what it used to be

@01micko
Copy link
Author

01micko commented Apr 24, 2022

This is open source. We help each other, that's what it's about, and you've been a big help to me and I thank you for that. I hope I've been somewhat of some help to you 😺

@johanmalm
Copy link
Member

Absolutely.
Love the cat-smiley to go with lab+puppy

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