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

Improve find cursor #936

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Improve find cursor #936

merged 1 commit into from
Aug 19, 2024

Conversation

ChangeCaps
Copy link
Contributor

@ChangeCaps ChangeCaps commented Aug 15, 2024

This pr does two things. When load_cursor now splits the $XCURSOR_PATH variable on both ':' and ' ', on my system $XCURSOR_PATH is separated by spaces. Second, cursor::Handle::new now checks $XCURSOR_THEME if getting Xcursor.theme from the resource_database fails.

@psychon
Copy link
Owner

psychon commented Aug 15, 2024

First: Due to #934 asking for all of this code to be replaced by the xcursor crate, I don't feel like we should spend too much time on this here. Sorry.

When load_cursor now splits the $XCURSOR_PATH variable on both ':' and ' ', on my system $XCURSOR_PATH is separated by spaces

Why is it that way on your system? I am pretty sure that xcursor-rs also only splits on :. But that's just another random Rust crate. IMO the "source of truth" is libXcursor. I find its source code always hard to navigate, but I am pretty sure that it also just splits on :.

So, does your space-separates $XCURSOR_PATH actually work with anything? Did I miss something?

Second, cursor::Handle::new now checks $XCURSOR_THEME if getting Xcursor.theme from the resource_database.

That seems to be done by libXcursor (it prefers the env var over the resource database), but does not seem to be done by xcursor-rs.

@ChangeCaps
Copy link
Contributor Author

Why is it that way on your system? I am pretty sure that xcursor-rs also only splits on :. But that's just another random Rust crate. IMO the "source of truth" is libXcursor. I find its source code always hard to navigate, but I am pretty sure that it also just splits on :.

So, does your space-separates $XCURSOR_PATH actually work with anything? Did I miss something?

You didn't, it was a brain-fart on my part... I ran echo $XCURSOR_PATH which apparently replaces semicolons with spaces in the output. I have reverted the change.

Second, cursor::Handle::new now checks $XCURSOR_THEME if getting Xcursor.theme from the resource_database.

That seems to be done by libXcursor (it prefers the env var over the resource database), but does not seem to be done by xcursor-rs.

As of 070e2bb $XCURSOR_THEME is now tried first.

@psychon
Copy link
Owner

psychon commented Aug 17, 2024

Could you squash this into a single commit and perhaps rebase onto master? Thanks.

#934 is now dealt with and the code you are touching is still there. Sorry, previously I thought that this code would go away with that change.

Fallback to $XCURSOR_THEME when database doesn't have a cursor theme

Revert splitting on ' '

Try $XCURSOR_THEME before Xcursor.theme
@ChangeCaps
Copy link
Contributor Author

ChangeCaps commented Aug 19, 2024

Could you squash this into a single commit and perhaps rebase onto master? Thanks.

All done!

Copy link
Owner

@psychon psychon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mergify mergify bot merged commit 8d43dd0 into psychon:master Aug 19, 2024
11 checks passed
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