[PATCH] Monitor objects per monitor-index
Thorsten Wißmann
edu at thorsten-wissmann.de
Sun Oct 6 12:40:15 CEST 2013
Hi Florian,
On Sun, Oct 06, 2013 at 01:40:32AM +0200, Florian Bruhin wrote:
> * Thorsten Wißmann <edu at thorsten-wissmann.de> [2013-06-24 13:03:32 +0200]:
> > On Thu, Jun 20, 2013 at 10:40:17PM +0200, Florian Bruhin wrote:
> > > This patch adds monitor objects to be able to access them by their
> > > index if they are unnamed.
> >
> > Did you forget to add some hunks in the patch? Because this only adds a
> > link when a new monitor is added and never renames them.
> > [...]
>
> This should now all be fixed in the attached new patch. Tested it by
> adding and removing a few monitors and it works so far. It unlinks and
> re-links all id objects when a monitor is removed, as discussed in
> IRC.
>
> Also, the second patch uses the new monitor_foreach function for
> all_monitors_apply_layout as well.
It doesn't work because of the following:
> +static void monitor_foreach(void (*action)(HSMonitor*)) {
> + for (int i = 0; i < g_monitors->len; i++) {
> [...]
> +static void monitor_update_id_objects() {
> + monitor_foreach(monitor_unlink_id_object);
> + monitor_foreach(monitor_link_id_object);
> +}
> +
> [...]
> @@ -532,6 +558,7 @@ int remove_monitor(int index) {
> g_string_free(monitor->display_name, true);
> g_free(monitor);
> g_array_remove_index(g_monitors, index);
> + monitor_update_id_objects();
Now, the old monitor object never is removed, because the
foreach(unlink) iterates over all entries of the g_monitors array, so
unlink is not called for monitor as it already has been removed one line
earlier. So monitor_update_id_objects() does not make sense for
removing, because both monitor_foreach() calls iterate over exactly the
same g_monitors state.
The fix for remove_monitor is a little rearrangement:
g_string_free(monitor->display_name, true);
monitor_foreach(monitor_unlink_id_object);
g_array_remove_index(g_monitors, index);
g_free(monitor);
monitor_foreach(monitor_link_id_object);
Note that the position of g_free(monitor) does not matter as long as it
called after monitor_unlink_id_object(). (And it is not needed for
add_monitor, so I dropped it).
> From 631cddfe2325e2bbed88fca66d5e746e95188a12 Mon Sep 17 00:00:00 2001
> From: Florian Bruhin <git at the-compiler.org>
> Date: Sun, 6 Oct 2013 00:45:42 +0200
> Subject: [PATCH 2/2] Use monitors_foreach for all_monitors_apply_layout
I changed it to monitor_foreach.
> void all_monitors_apply_layout() {
> - for (int i = 0; i < g_monitors->len; i++) {
> - HSMonitor* m = monitor_with_index(i);
> - monitor_apply_layout(m);
> - }
> + monitor_foreach(monitor_apply_layout);
> }
Well spotted! All in all, it is much less code than I expected. Good
Work!
After the named modifications[1] I pushed it as:
b7d1d6d Use monitor_foreach for all_monitors_apply_layout
6006395 Add objects monitors.* to access monitors per id
Regards,
Thorsten
[1] If you have problems with me modifing your patch and commiting it
with your name, let me know.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 230 bytes
Desc: not available
URL: <https://listi.jpberlin.de/pipermail/hlwm/attachments/20131006/6231c85d/attachment.sig>
More information about the hlwm
mailing list