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

ActiveWindowName won't respect max_chars #286

Closed
greweln opened this issue Dec 3, 2023 · 4 comments
Closed

ActiveWindowName won't respect max_chars #286

greweln opened this issue Dec 3, 2023 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@greweln
Copy link

greweln commented Dec 3, 2023

Describe the bug

The widget displays full length text ignoring the max_chars value.
...

To Reproduce

  • On the same workspace, st + firefox (with some tabs).

  • While keeping the focus on the Firefox window switch tabs.It will display the full URL ignoring the max_chars value. If the URL is long enough will even "push " the next widgets out of the screen.

  • Switching the focus between st/firefox(while having the tab with the long URL displayed) works as expected.

Expected behavior

...

Screenshots

screenshot

@greweln greweln added the bug Something isn't working label Dec 3, 2023
@sminez
Copy link
Owner

sminez commented Dec 17, 2023

Hey @greweln, sorry about the delay on this one! I think the fix might be embarrassingly simple...

https://github.com/sminez/penrose/blob/develop/crates/penrose_ui/src/bar/widgets/simple.rs#L129

Pretty sure that all that is required is to change that line from

self.inner.set_text(x.window_title(*id)?)

to

self.set_text(x.window_title(*id)?)

I've just pushed that change on a branch (issue-286) are you able to try that out and verify if it fixes the issue for you?

@sminez sminez self-assigned this Dec 17, 2023
@greweln
Copy link
Author

greweln commented Dec 17, 2023

@sminez
I think it should be borrowed here:
self.set_text(&x.window_title(*id)?)
right?

@sminez
Copy link
Owner

sminez commented Dec 18, 2023

The fact that those two signatures don't match makes me all kinds of sad 😞

And yeah, that should either be a reference or the signature should be updated so it matches Text::set_text

sminez added a commit that referenced this issue Jan 3, 2024
* #286 calling the version of set_text that truncates to our character limit

* #286 should be a &str not a String or impl Into<String>
@sminez
Copy link
Owner

sminez commented Jan 3, 2024

fixed in #287

@sminez sminez closed this as completed Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants