Revise pane_reparent()

Treating a disconnected pane (p->parent == p) as a special cas in
pane_reparent() is unpleasant as disconnected panes are not something I
want.

So change pane_reparent() to have a different rule for determining
whether the new parent needs to be attached below p->parent first.

Also ensure Child-Notify notifications are consistant.

If the reparent fails, LOG an error and teach python interface to
trigger an exception.

Finally, only notmuch depended on the special meaning of disconnected
panes, so change it to fit the new model and add better explanations.

Signed-off-by: NeilBrown <neil@brown.name>

diff --git a/DOC/TODO.md b/DOC/TODO.md
index a9a7c3831734625ea3a94103925e9c434a042277..bd6486ad447108aa6692477889162829063bdc77 100644 (file)
--- a/DOC/TODO.md
+++ b/DOC/TODO.md
@@ -1080,6 +1080,8 @@ Interaction with gdb would be nice too - things like
 
 - [ ] C config module that reads an ini-style file to set attributes
       based on path
+- [ ] configure "initial_panes"
+- [ ] discard old auto-load??
 
 What needs to be configured?  How is that done?
 

diff --git a/core-pane.c b/core-pane.c
index 0fc3b74cb08f5c0be49f43778388bc29e4f564a4..aab46946027d957daae71cdb6bbb54e092e39820 100644 (file)
--- a/core-pane.c
+++ b/core-pane.c
@@ -644,26 +644,53 @@ void pane_resize(struct pane *p safe, int x, int y, int w, int h)
 
 void pane_reparent(struct pane *p safe, struct pane *newparent safe)
 {
-       /* detach p from its parent and attach beneath its sibling newparent */
+       /* Change the parent of 'p' to be 'newparent.
+        * An important consideration is that a pane must never move up the
+        * hierarchy (towards the root), but only sideways or down.  This ensures
+        * that any mark the pane (or a descendant thereof) might hold still gets
+        * delivered to the correct document.  There is one exception that a pane
+        * that was newly created may be re-attached above some ancestors.
+        * There is not currently any test for "newness" though that may be added
+        * later (FIXME)
+        *
+        * 'newparent' must be a descendant of p->parent.
+        * If it is a sibling of p or descentant thereof, p is simply detached from
+        * its parent and reattached below newparent.
+        * If it is a descendant of p (it cannot be p itself), then as well as p
+        * being detached from it parent and attached to newparent, newparent is detached
+        * and attached between p and p->parent, thus ensuring no loop is created.
+        */
        int replaced = 0;
-       // FIXME this should be a failure, possibly with warning, not an
-       // assert.  I'm not sure just now how best to do warnings.
-       ASSERT(newparent->parent == p->parent || newparent->parent == newparent);
-       list_del(&p->siblings);
+       struct pane *pc = pane_my_child(p->parent, newparent);
+       if (pc == NULL || newparent == p) {
+               LOG("Cannot reparent %s to %s, new parent must be a sibling or their descendant",
+                   p->name, newparent->name);
+               LOG_BT();
+               return;
+       }
+       /* Detatch p */
+       list_del_init(&p->siblings);
        if (p->parent->focus == p)
-               p->parent->focus = newparent;
-       if (newparent->parent == newparent) {
+               p->parent->focus = pc;
+       if (pc == p) {
+               p->parent->focus = NULL;
+               /* newparent is below p, need to detach and reattach it */
+               if (newparent->parent->focus == newparent)
+                       newparent->parent->focus = NULL;
+               pane_call(newparent->parent, "Child-Notify", newparent, -2);
                newparent->parent = p->parent;
-               list_add(&newparent->siblings, &p->parent->children);
+               list_move(&newparent->siblings, &p->parent->children);
                pane_resize(newparent, 0, 0, p->parent->w, p->parent->h);
                replaced = 1;
        }
+       pane_call(p->parent, "Child-Notify", p, -2);
+       /* Reattach p under newparent */
        p->parent = newparent;
        newparent->damaged |= p->damaged;
        if (newparent->focus == NULL)
                newparent->focus = p;
        list_add(&p->siblings, &newparent->children);
-       pane_call(newparent->parent, "Child-Notify", p, -2);
+       pane_call(p->parent, "Child-Notify", p, 2);
        if (replaced)
                pane_call(newparent->parent, "Child-Notify", newparent, 2);
 }

diff --git a/lang-python.c b/lang-python.c
index 743bd1c5f83d91c6e8dcf393a601c97c1839dff4..19350e04fa9ad044c2b35c27a8b64ef305f3a33d 100644 (file)
--- a/lang-python.c
+++ b/lang-python.c
@@ -1379,8 +1379,13 @@ static PyObject *Pane_reparent(Pane *self safe, PyObject *args)
        Pane *newparent = NULL;
        int ret = PyArg_ParseTuple(args, "O!", &PaneType, &newparent);
 
-       if (ret > 0 && newparent && self->pane && newparent->pane)
+       if (ret > 0 && newparent && self->pane && newparent->pane) {
                pane_reparent(self->pane, newparent->pane);
+               if (self->pane->parent != newparent->pane) {
+                       PyErr_SetString(PyExc_TypeError, "reparent failed");
+                       return NULL;
+               }
+       }
        Py_INCREF(Py_None);
        return Py_None;
 }

diff --git a/python/module-notmuch.py b/python/module-notmuch.py
index 43650e8fac94e6744fec924a6c95b32ba30a3310..098e800f5c726b41ae60d2d3976c1bbaa4c71fb1 100644 (file)
--- a/python/module-notmuch.py
+++ b/python/module-notmuch.py
@@ -3617,12 +3617,21 @@ def render_master_view_attach(key, focus, comm2, **a):
     # The tile which displays the search list does not have a document, as it
     # refers down the main document.  So it doesn't automatically get borders
     # from a 'view', so we must add one explicitly.
+    # We assume 'focus' is a 'view' pane on a "doc" pane for the notmuch primary doc,
+    # which is (probably) on a "tile" pane.
+    # The master view needs to be below the "doc" pane, above the tile.
+    # We attach it on focus and use Pane.reparent to rotate
+    # from:  master_view -> view -> doc -> tile
+    #   to:  view -> doc -> master_view -> tile
 
     doc = focus.parent
-    main = notmuch_master_view()
+    main = notmuch_master_view(focus)
     doc.reparent(main)
     p = main.call("attach-tile", "notmuch", "main", ret='pane')
+    # Now we have tile(main) -> view -> doc -> master_view -> tile
+    # and want tile(main) above doc
     doc.reparent(p)
+    # Now 'view' doesn't have a child -we give it 'list_view'
     p = notmuch_list_view(focus)
     p = p.call("attach-render-format", ret='pane')
     main.list_pane = p