Isolating 'seamless_restart' into a separate patch

[acid-bong]

Hi there, hello. I wanna port the seamless restart feature to my build of dwm, but it seems to be heavily dependent on dusk and dwm-flexipatch's own features.

First, I straight up copied seamless_restart.c (and cut out any lines that for patches I'm not using), added to dwm.c only lines activated by SEAMLESS_RESTART and expanded getatomprop() to work with three arguments (so it doesn't output errors on arguments number mismatch). There's still one error that I don't know how to deal with -- TAGMASK type:

seamlessrestart.c: In function 'getmonitortags':
dwm.c:60:53: error: subscripted value is neither array nor pointer nor vector
   60 | #define LENGTH(X)               (sizeof X / sizeof X[0])
      |                                                     ^
dwm.c:64:40: note: in expansion of macro 'LENGTH'
   64 | #define TAGMASK                 ((1 << LENGTH(tags)) - 1)
      |                                        ^~~~~~
seamlessrestart.c:172:48: note: in expansion of macro 'TAGMASK'
  172 |                 m->tagset[m->seltags] = tags & TAGMASK;
      |                                                ^~~~~~~
seamlessrestart.c: In function 'getclienttags':
dwm.c:60:53: error: subscripted value is neither array nor pointer nor vector
   60 | #define LENGTH(X)               (sizeof X / sizeof X[0])
      |                                                     ^
dwm.c:64:40: note: in expansion of macro 'LENGTH'
   64 | #define TAGMASK                 ((1 << LENGTH(tags)) - 1)
      |                                        ^~~~~~
seamlessrestart.c:244:26: note: in expansion of macro 'TAGMASK'
  244 |         c->tags = tags & TAGMASK;
      |                          ^~~~~~~

This part seems to rely on your implementation of the number of tags, which I personally don't wanna implement. How can I adapt it to the vanilla implementation? Do I need to copy more stuff from this commit of yours where you introduced it first?

Reference:

• the current state of the patch: https://git.sr.ht/~acid-bong/dwm/commit/0f2f068fcebc3bddb31a3f0b157362d76112e7f1 (after that I added the stuff from your attachx.c which I forgot about)
• normal state of my dwm: https://git.sr.ht/~acid-bong/dwm

[bakkeby]

Right, so in your config you have a variable tags:

static const char *tags[] = { "α", "β", "γ", "δ", "ε" };

and in the problematic functions in seamlessrestart.c you have:

	if (nitems) {
		tags = *(Atom *)p;
		m->tagset[m->seltags] = tags & TAGMASK;
	}

The macro of TAGMASK will expand to:

		m->tagset[m->seltags] = tags & ((1 << LENGTH(tags)) - 1);

and tags is now referring to the local atom variable rather than your array of tag icons, hence the error.

So if you rename the local variables to something else then it will compile fine, e.g.

diff --git a/seamlessrestart.c b/seamlessrestart.c
index 4db3f93..b4dba17 100644
--- a/seamlessrestart.c
+++ b/seamlessrestart.c
@@ -157,7 +157,7 @@ getmonitortags(Monitor *m)
        unsigned long dl, nitems;
        unsigned char *p = NULL;
        char atom[22] = {0};
-       Atom da, monitor_tags = None, tags;
+       Atom da, monitor_tags = None, seltags;

        sprintf(atom, "_DWM_MONITOR_TAGS_%u", m->num);
        monitor_tags = XInternAtom(dpy, atom, False);
@@ -168,8 +168,8 @@ getmonitortags(Monitor *m)
        }

        if (nitems) {
-               tags = *(Atom *)p;
-               m->tagset[m->seltags] = tags & TAGMASK;
+               seltags = *(Atom *)p;
+               m->tagset[m->seltags] = seltags & TAGMASK;
        }

        XFree(p);
@@ -237,11 +237,11 @@ setclienttags(Client *c)
 int
 getclienttags(Client *c)
 {
-       Atom tags = getatomprop(c, clientatom[ClientTags], AnyPropertyType);
-       if (tags == None)
+       Atom clienttags = getatomprop(c, clientatom[ClientTags], AnyPropertyType);
+       if (clienttags == None)
                return 0;

-       c->tags = tags & TAGMASK;
+       c->tags = clienttags & TAGMASK;
        return 1;
 }

[acid-bong]

Ight, name clashing it is.

I renamed them to montags and clitags respectively, and it resolved the conflict. It wasn't enough to keep the layouts on restart, so I added the rest of that commit of yours (already pushed to my sourcehut: https://git.sr.ht/~acid-bong/dwm/tree/restart).

Except this is where bigger issues appeared (one of the reasons is that I copid the stuff mostly blindly):

1. alwayscenter feature worked differently: from my floating rules only Kotatogram (Telegram fork) and Gajim were spawned centered, the rest -- pinentry-gnome3 and terminal apps -- in the top left corner, as if the alwayscenter patch was never applied. I assume it's because of those nx, ny and -9999 stuff, can't say for sure.
2. On restart it does keep the positions of the windows, but if there's multiple of them -- on killing (or sometimes on restart, which is quit(1)) they completely forget what tiling and floating are and start to float randomly (but manage to still change shape).
3. Unrelated to floating: I have forementioned Kotatogram and Gajim in the systray (and in the background), and for some reason Gajim gets killed. For comparison, preserveonrestart does absolutely nothing to the background processes.

Any tips on how to continue after that?

[bakkeby]

It can get messy very quickly. Besides preserving information before restarting the restoring of client state, etc., also depends on the order things happen in manage.

I would have created a standalone patch for seamless restart, but there is not that much that can be preserved on a bare dwm besides nmaster, the layout and whether the bar is shown or not. The preserveonrestart and restoreafterrestart patches primarily just deals with what tags are assigned to what clients.

As for 1. I'd say that this is related to that the window float position is stored for clients and thus the alwayscenter calculations do not come into effect. I think you would expect the window to remain in place rather than snapping back to center when you restart.

As for 2. do you perhaps have something that automatically starts dwm again if you quit? Otherwise could be that the layout is actually floating upon restart. You mentioned an issue with preserving layouts on restart. Also worth noting that it doesn't copy anything into m->ltsymbol (see setlayout function), so could be that the layout is preserved but it just shows as []= in the bar.

For 3. I'm not entirely sure, but in cleanup when restarting we unmap and destroy the systray window. In most cases the systray application will try to map the systray icon window again. It may be that Gajim feels that life is not worth living after having been abandoned by the charming but abrasive systray window and decides to off itself.

[acid-bong]

So, I decided to continue all this stuff (I'm still porting only parts relevant to the patches I'm using) after adding pertag, and there are new issues now:

1. "incomplete typedef" in pertag-specific references
2. NUMTAGS is undefined (I'm using vanilla tag list)

In file included from dwm.c:340:
seamless_restart.c: In function 'setmonitorfields':
seamless_restart.c:68:26: error: 'NUMTAGS' undeclared (first use in this function)
   68 |         for (i = 0; i <= NUMTAGS; i++) {
      |                          ^~~~~~~
seamless_restart.c:68:26: note: each undeclared identifier is reported only once for each function it appears in
seamless_restart.c:70:35: error: invalid use of incomplete typedef 'Pertag'
   70 |                         (m->pertag->nmasters[i] & 0x7) |
      |                                   ^~
seamless_restart.c:71:50: error: invalid use of incomplete typedef 'Pertag'
   71 |                         (getlayoutindex(m->pertag->ltidxs[i][m->pertag->sellts[i]]) & 0xF) << 6 |
      |                                                  ^~
seamless_restart.c:71:71: error: invalid use of incomplete typedef 'Pertag'
   71 |                         (getlayoutindex(m->pertag->ltidxs[i][m->pertag->sellts[i]]) & 0xF) << 6 |
      |                                                                       ^~
seamless_restart.c:72:34: error: invalid use of incomplete typedef 'Pertag'
   72 |                         m->pertag->showbars[i] << 31
      |                                  ^~
seamless_restart.c: In function 'getmonitorfields':
seamless_restart.c:107:26: error: 'NUMTAGS' undeclared (first use in this function)
  107 |         for (i = 0; i <= NUMTAGS; i++) {
      |                          ^~~~~~~
seamless_restart.c:121:26: error: invalid use of incomplete typedef 'Pertag'
  121 |                 m->pertag->nmasters[i] = state & 0x7;
      |                          ^~
seamless_restart.c:124:34: error: invalid use of incomplete typedef 'Pertag'
  124 |                         m->pertag->ltidxs[i][m->pertag->sellts[i]] = &layouts[layout_index];
      |                                  ^~
seamless_restart.c:124:55: error: invalid use of incomplete typedef 'Pertag'
  124 |                         m->pertag->ltidxs[i][m->pertag->sellts[i]] = &layouts[layout_index];
      |                                                       ^~
seamless_restart.c:125:26: error: invalid use of incomplete typedef 'Pertag'
  125 |                 m->pertag->showbars[i] = (state >> 31) & 0x1; // maybe??
      |                          ^~
seamless_restart.c:128:47: error: invalid use of incomplete typedef 'Pertag'
  128 |                         m->nmaster = m->pertag->nmasters[i];
      |                                               ^~
seamless_restart.c:129:45: error: invalid use of incomplete typedef 'Pertag'
  129 |                         m->sellt = m->pertag->sellts[i];
      |                                             ^~
seamless_restart.c:130:52: error: invalid use of incomplete typedef 'Pertag'
  130 |                         m->lt[m->sellt] = m->pertag->ltidxs[i][m->sellt];
      |                                                    ^~
seamless_restart.c:131:47: error: invalid use of incomplete typedef 'Pertag'
  131 |                         m->showbar = m->pertag->showbars[i]; // maybex2 ??
      |                                               ^~

I decided to define NUMTAGS as LENGTH(tags), but it complained about type mismatch of tags.

The git tree URL is the same

[bakkeby]

That would suggest that you are sourcing seamless_restart.c before struct Pertag. If you move your include statement further below then most of those errors should go away.

[acid-bong]

Cheers, that helped. What about NUMTAGS? This is what happens with #define NUMTAGS LENGTH(tags) between *tags definition and seamless_restart.c's "meat":

In file included from dwm.c:349:
./seamless_restart.c:109:19: error: subscripted value is not an array, pointer, or vector
        for (i = 0; i <= NUMTAGS; i++) {
                         ^~~~~~~
./seamless_restart.c:1:17: note: expanded from macro 'NUMTAGS'
#define NUMTAGS LENGTH(tags)
                ^~~~~~~~~~~~
dwm.c:59:53: note: expanded from macro 'LENGTH'
#define LENGTH(X)               (sizeof X / sizeof X[0])
                                                   ~^~
In file included from dwm.c:349:
./seamless_restart.c:110:56: error: subscripted value is not an array, pointer, or vector
                if (!(XGetWindowProperty(dpy, root, dwm_monitor, i, (NUMTAGS + 1) * sizeof dl,
                                                                     ^~~~~~~
./seamless_restart.c:1:17: note: expanded from macro 'NUMTAGS'
#define NUMTAGS LENGTH(tags)
                ^~~~~~~~~~~~
dwm.c:59:53: note: expanded from macro 'LENGTH'
#define LENGTH(X)               (sizeof X / sizeof X[0])
                                                   ~^~

[bakkeby]

Where do you define the NUMTAGS macro? I am thinking that if you define it before the LENGTH macro is defined, then it may substitute NUMTAGS with "LENGTH(tags)" in plain text rather than "(sizeof(tags) / sizeof(tags[0]))".

[acid-bong]

I define it right after tags is defined or in the beginning of seamless_restart.c, both of which happen long after LENGTH is defined. Tried defining it before LENGTH, same result, looks like in C their order doesn't matter much

[bakkeby]

Ah, it's fairly straightforward actually.

It is just a variable name clash.

You define tags in your config.h right, but in the function getmonitorfields we also have this variable declared:

unsigned int tags = m->tagset[m->seltags] << 1;

and that variable is not an array, hence the error.

If you change that local variable name to something else, e.g. montags, and the one place where this is used then it should compile just fine.

[acid-bong]

So I mitigated another name clash (pushed), and it compiles successfully.

However, the same issues remain, except for these:

1. n.2 (pseudotiling) doesn't happen anymore. I think it's because I misconfigured attachtop in the first attempt and now it's unpatched from my branch.
2. New one, pertag-specific: I put one tag in monocle mode (I have only tiling and monocle enabled) and now it's stuck in it and can't switch back to tiling

[bakkeby]

As for 2; in your specific build you rely solely on setlayout(0) to toggle to the previous layout. So if the current and previous layout ends up being the same after a restart then doing Super+m, or using the mouse and clicking on the layout symbol, is not going to make any difference.