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

Make mouse support configurable #494

Merged
merged 5 commits into from
Feb 21, 2024
Merged

Conversation

Mroik
Copy link
Contributor

@Mroik Mroik commented Feb 15, 2024

Implements request of issue #402 for mouse support to be configurable through joshuto.toml instead of it being a cargo feature.

Copy link
Owner

@kamiyaa kamiyaa left a comment

Choose a reason for hiding this comment

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

Ideally I would like turning off mouse support to be able to select text.
As it is, when mouse is enabled or disabled, text cannot be selected via mouse.

So some programmatic way to use MouseTerminal

@Mroik
Copy link
Contributor Author

Mroik commented Feb 20, 2024

I really dislike this solution, made me change the signature of some functions to pass context. I don't think this is avoidable due to the fact that Screen was an alias had different types based on the cargo features.

@Mroik Mroik requested a review from kamiyaa February 20, 2024 23:06
@kamiyaa
Copy link
Owner

kamiyaa commented Feb 20, 2024

I really dislike this solution, made me change the signature of some functions to pass context. I don't think this is avoidable due to the fact that Screen was an alias had different types based on the cargo features.

An alternative solution could be to keep the mouse feature, have it enabled by default and can be changed via config (the work you did).
What do you think?

@Mroik
Copy link
Contributor Author

Mroik commented Feb 20, 2024

An alternative solution could be to keep the mouse feature, have it enabled by default and can be changed via config (the work you did).

Not sure what you mean, as it is right now in this branch it is done only through the config (there wouldn't be a need for a cargo feature).
What I was referring to is this. I don't think we can avoid an enum given that the previous implementation for the mouse support toggle via cargo feature achieved it through different types.

Ideally I would like turning off mouse support to be able to select text.
As it is, when mouse is enabled or disabled, text cannot be selected via mouse.

After the latest commits this is achieved tho, what you wanted should be the current behaviour.

@Mroik
Copy link
Contributor Author

Mroik commented Feb 20, 2024

Not sure if I explained myself, sorry if I wasn't too clear 😅

@kamiyaa
Copy link
Owner

kamiyaa commented Feb 21, 2024

Okay, this is fine. I am content with this haha.

@kamiyaa kamiyaa merged commit f6d1f71 into kamiyaa:main Feb 21, 2024
4 checks passed
@kamiyaa
Copy link
Owner

kamiyaa commented Feb 21, 2024

Thanks!

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