Feature request: Make the message location configurable
unsigned-enby opened this issue · 10 comments
First off, let me say thank you for the work you have put in to cagebreak! While river is my 'main' WM, cagebreak provides a distraction free environment almost on par with the framebuffer. The one issue I've had is that messages are displayed in the top right of the screen. This isn't a 'major' issue persay, but my right eye is basically useless (ambliopia) so it's not entirely ideal. While changing it to the top left is as simple as ...
--- a/message.c 2023-11-17 16:05:19.556061285 -0600
+++ b/message.c 2023-11-17 16:08:44.706721504 -0600
@@ -266,12 +266,12 @@
wlr_output_layout_get_box(output->server->output_layout, output->wlr_output,
&output_box);
- box->x = output_box.width;
+ box->x = 0;
box->y = 0;
box->width = 0;
box->height = 0;
- message_set_output(output, buffer, box, CG_MESSAGE_TOP_RIGHT);
+ message_set_output(output, buffer, box, CG_MESSAGE_TOP_LEFT);
free(buffer);
alarm(output->server->message_config.display_time);
}
I feel it would make sense to just use the cg_message_align enum to set both the anchor point (i.e screen relative position) and the alignment (i.e message box relative position), as their respective matches aren't likely to differ(e.g if you want the message in a corner of the screen, you're probably not going to want it centered on the message). My thinking would be to 'attach' the configured enum to the cg_message_config structure, alter message_printf as needed, but keep message_set_output (and message_printf_pos) the same. I'm likely to do this for my own benifit, but I'd be more than willing to submit a pull request, along with any particular preferences of yours, if you would like to add it into cagebreak!
Hi unsigned-enby
Thank you for your feature request. We think it is a great idea to make
Cagebreak more accessible. We will release this addition as soon as
we find the time to implement and test it (modulo other features).
If you'd want to take the initiative of implementing something of the sort
and submitting a pull request, that would certainly be great and speed
things up. In this case, I think we'd want to have this as an option for
"configure_message", as you suggest. Our proposed syntax would be
configure_message position top_left (and similarly, bottom_right,
etc., i.e., it would be nice to have the "bottom" options too). What do
you think?
cheers
project-repo
Hi project-repo,
Thanks for your timely response. I just finished up implimenting this change. I went with the term anchor since 1: 'position' is already the name of a wlr_box variable in message_set_output and 2: anchor conforms with other notification utilities (at least with mako that is). However I don't think it would be confusing to change anchor to position for the user-side of things (i.e in cagebreak/config & ipc-socket).
Just as well I added in a CG_MESSAGE_NOPT enum into cg_message_align (which is now cg_message_anchor) for the sake of keybinding_configure_message & parse_message_config. Since it's an enum, setting it to -1 isn't a (good?) option, but if I'm mistaken, or if there is a better way to do that in general, I would certainly like to hear your thoughts!
Lastly, I included the ability to set the bottom options as you suggested as well as top_center , bottom_center, and center for completeness. I tested this all on both my rock 5 (aarch64) and x86_64 laptop and it all works as expected!
Best,
Luca
Hi unsigned-enby
Thanks so much! Initial tests look spot on, great job! We'll do some
further testing and create a feature branch for this asap (probably
it'll be the weekend though before we get the necessary time), so that
this can be included in the next release (which will again happen when
we can find the time to get everything ready). In the meantime, it would
be great if you could add a DCO to your pull request. Here is the
relevant documentation:
https://github.com/project-repo/cagebreak#developer-certificate-of-origin-dco
(This shows you are allowed to contribute and are fine with publication
under MIT).
Regarding the naming convention, I think I agree that changing "anchor" to
"position" on the user side of things would be a good idea, to keep
consistent with e.g. ouput position. But we can change that once we
merge your changes and have made a final decision.
In any case, once again thanks a lot!
Cheers
project-repo
Sounds good / DCO is now added to the pull request.
Hi unsigned-enby,
A few days ago, a new version of wlroots was released. It seems that
making cagebreak compatible with this new version will be trickier than
was a priori assumed... Unfortunately, that means it may take another
week or so for us to get everything up and running to a degree that we
are able to take a closer look at your PR, sorry about that. We'll get
back to you then.
Cheers,
project-repo
PS: Sorry, this should have been sent yesterday but was unfortunately delayed
due to internal comunication issues.
That's fine! Thank you for letting me know. Wether it's merged or not I appreciate your responsiveness and communication. This has been my first official pull request and you have certainly helped make it a less nerve-racking experience.
Best,
unsigned-enby
Hi unsigned-enby
Sorry for taking so long to get back to you. Wlroots 0.17 and other
year-end deadlines got in the way of looking at this more closely. We've
now done some testing and your code seems to work as advertised in all
special cases, great job!
One thing we noticed though is that there appears to be some
inconsistency in the documentation, i.e. at one point you use "align"
and at another you use "anchor". Am I right in assuming that it should
be "anchor" everywhere?
Our current plan is to do a year-end release which will include your
code along with some other things that are still pending (e.g. we might
get the "permanent" versus "peripheral" output setup to float). Does
that work for you?
Cheers
project-repo
No worries! With the end of the semester I haven't had much time myself to check for a response. As for the align vs anchor, anchor is certainly the intended/desired term. And that works for me.
Best,
Unsigned-enby
Hi unsigned-enby
This has now been merged onto the development branch. Thanks again for
putting in the effort to implement this!
Cheers
project-repo
This has been released with 2.3.0.
I am closing this issue.
Cheers
project-repo