buildmimetree.py: handle counting of extended family in presence of other attachments

diff --git a/.config/neomutt/buildmimetree.py b/.config/neomutt/buildmimetree.py
index 491696f5412c4cd11374b29c4acbedc4d6eaab10..1adfe5d864f976520ba20d187c93e8d292c516fd 100755 (executable)
--- a/.config/neomutt/buildmimetree.py
+++ b/.config/neomutt/buildmimetree.py
@@ -79,7 +79,23 @@ def parse_cli_args(*args, **kwargs):
     parser.add_argument(
         "--related-to-html-only",
         action="store_true",
     parser.add_argument(
         "--related-to-html-only",
         action="store_true",

-        help="Make related content be sibling to HTML parts only"
+        help="Make related content be sibling to HTML parts only",
+    )
+
+    def positive_integer(value):
+        try:
+            if int(value) > 0:
+                return int(value)
+
+        except ValueError:
+            pass
+
+        raise ValueError(f"Must be a positive integer")
+
+    parser.add_argument(
+        "--max-number-other-attachments",
+        type=positive_integer,
+        help="Make related content be sibling to HTML parts only",

     )
 
     parser.add_argument(
     )
 
     parser.add_argument(


@@ -383,10 +399,11 @@ class MIMETreeDFWalker:
         self._walk(
             root,
             ancestry=[],
         self._walk(
             root,
             ancestry=[],

+            descendents=[],

             visitor_fn=visitor_fn or self._visitor_fn,
         )
 
             visitor_fn=visitor_fn or self._visitor_fn,
         )
 

-    def _walk(self, node, *, ancestry, visitor_fn):
+    def _walk(self, node, *, ancestry, descendents, visitor_fn):

         # Let's start by enumerating the parts at the current level. At the
         # root level, ancestry will be the empty list, and we expect a
         # multipart/* container at this level. Later, e.g. within a
         # Let's start by enumerating the parts at the current level. At the
         # root level, ancestry will be the empty list, and we expect a
         # multipart/* container at this level. Later, e.g. within a


@@ -409,18 +426,24 @@ class MIMETreeDFWalker:
                 self._walk(
                     child,
                     ancestry=ancestry,
                 self._walk(
                     child,
                     ancestry=ancestry,

+                    descendents=descendents,

                     visitor_fn=visitor_fn,
                 )
             assert ancestry.pop() == node
                     visitor_fn=visitor_fn,
                 )
             assert ancestry.pop() == node

+            sibling_descendents = descendents
+            descendents.extend(node.children)

 
         else:
             self.debugprint(f"{lead}├{node}")
 
         else:
             self.debugprint(f"{lead}├{node}")

+            sibling_descendents = descendents

 
         if False and ancestry:
             self.debugprint(lead[:-1] + " │")
 
         if visitor_fn:
 
         if False and ancestry:
             self.debugprint(lead[:-1] + " │")
 
         if visitor_fn:

-            visitor_fn(node, ancestry, debugprint=self.debugprint)
+            visitor_fn(
+                node, ancestry, sibling_descendents, debugprint=self.debugprint
+            )

 
     def debugprint(self, s, **kwargs):
         if self._debug:
 
     def debugprint(self, s, **kwargs):
         if self._debug:


@@ -507,6 +530,7 @@ def do_massage(
     converter=convert_markdown_to_html,
     related_to_html_only=True,
     only_build=False,
     converter=convert_markdown_to_html,
     related_to_html_only=True,
     only_build=False,

+    max_other_attachments=20,

     tempdir=None,
     debug_commands=False,
     debug_walk=False,
     tempdir=None,
     debug_commands=False,
     debug_walk=False,


@@ -543,7 +567,7 @@ def do_massage(
 
     state = dict(pos=1, tags={}, parts=1)
 
 
     state = dict(pos=1, tags={}, parts=1)
 

-    def visitor_fn(item, ancestry, *, debugprint=None):
+    def visitor_fn(item, ancestry, descendents, *, debugprint=None):

         """
         Visitor function called for every node (part) of the MIME tree,
         depth-first, and responsible for telling NeoMutt how to assemble
         """
         Visitor function called for every node (part) of the MIME tree,
         depth-first, and responsible for telling NeoMutt how to assemble


@@ -587,11 +611,36 @@ def do_massage(
             if item.cid:
                 cmds.push(f"<edit-content-id>{KILL_LINE}{item.cid}<enter>")
 
             if item.cid:
                 cmds.push(f"<edit-content-id>{KILL_LINE}{item.cid}<enter>")
 

+            # Now for the biggest hack in this script, which is to handle
+            # attachments, such as PDFs, that aren't related or alternatives.
+            # The problem is that when we add an inline image, it always gets
+            # appended to the list, i.e. inserted *after* other attachments.
+            # Since we don't know the number of attachments, we also cannot
+            # infer the postition of the new attachment. Therefore, we bubble
+            # it all the way to the top, only to then move it down again:
+            if state["pos"] > 1:  # skip for the first part
+                for i in range(max_other_attachments):
+                    # could use any number here, but has to be larger than the
+                    # number of possible attachments. The performance
+                    # difference of using a high number is negligible.
+                    # Bubble up the new part
+                    cmds.push(f"<move-up>")
+
+                # As we push the part to the right position in the list (i.e.
+                # the last of the subset of attachments this script added), we
+                # must handle the situation that subtrees are skipped by
+                # NeoMutt. Hence, the actual number of positions to move down
+                # is decremented by the number of descendents so far
+                # encountered.
+                for i in range(1, state["pos"] - len(descendents)):
+                    cmds.push(f"<move-down>")
+

         elif isinstance(item, Multipart):
             # This node has children, but we already visited them (see
             # above). The tags dictionary of State should contain a list of
             # their positions in the NeoMutt compose window, so iterate those
             # and tag the parts there:
         elif isinstance(item, Multipart):
             # This node has children, but we already visited them (see
             # above). The tags dictionary of State should contain a list of
             # their positions in the NeoMutt compose window, so iterate those
             # and tag the parts there:

+            n_tags = len(state["tags"][item])

             for tag in state["tags"][item]:
                 cmds.push(f"<jump>{tag}<enter><tag-entry>")
 
             for tag in state["tags"][item]:
                 cmds.push(f"<jump>{tag}<enter><tag-entry>")
 


@@ -606,9 +655,8 @@ def do_massage(
                     f"Handling of multipart/{item.subtype} is not implemented"
                 )
 
                     f"Handling of multipart/{item.subtype} is not implemented"
                 )
 

-            state["pos"] -= len(state["tags"][item]) - 1
+            state["pos"] -= n_tags - 1

             state["parts"] += 1
             state["parts"] += 1

-            del state["tags"][item]

 
         else:
             # We should never get here
 
         else:
             # We should never get here


@@ -627,6 +675,7 @@ def do_massage(
             lead = "│ " * (len(ancestry) + 1) + "* "
             debugprint(
                 f"{lead}ancestry={[a.subtype for a in ancestry]}\n"
             lead = "│ " * (len(ancestry) + 1) + "* "
             debugprint(
                 f"{lead}ancestry={[a.subtype for a in ancestry]}\n"

+                f"{lead}descendents={[d.subtype for d in descendents]}\n"

                 f"{lead}children_positions={state['tags'][ancestry[-1]]}\n"
                 f"{lead}pos={state['pos']}, parts={state['parts']}"
             )
                 f"{lead}children_positions={state['tags'][ancestry[-1]]}\n"
                 f"{lead}pos={state['pos']}, parts={state['parts']}"
             )


@@ -674,6 +723,7 @@ if __name__ == "__main__":
                 extensions=args.extensions,
                 cssfile=args.css_file,
                 related_to_html_only=args.related_to_html_only,
                 extensions=args.extensions,
                 cssfile=args.css_file,
                 related_to_html_only=args.related_to_html_only,

+                max_other_attachments=args.max_number_other_attachments,

                 only_build=args.only_build,
                 tempdir=args.tempdir,
                 debug_commands=args.debug_commands,
                 only_build=args.only_build,
                 tempdir=args.tempdir,
                 debug_commands=args.debug_commands,


@@ -805,8 +855,8 @@ try:
 
             items = []
 
 
             items = []
 

-            def visitor_fn(item, ancestry, debugprint):
-                items.append((item, len(ancestry)))
+            def visitor_fn(item, ancestry, descendents, debugprint):
+                items.append((item, len(ancestry), len(descendents)))

 
             mimetree.walk(
                 mime_tree_related_to_alternative, visitor_fn=visitor_fn
 
             mimetree.walk(
                 mime_tree_related_to_alternative, visitor_fn=visitor_fn


@@ -814,20 +864,25 @@ try:
             assert len(items) == 5
             assert items[0][0].subtype == "plain"
             assert items[0][1] == 2
             assert len(items) == 5
             assert items[0][0].subtype == "plain"
             assert items[0][1] == 2

+            assert items[0][2] == 0

             assert items[1][0].subtype == "html"
             assert items[1][1] == 2
             assert items[1][0].subtype == "html"
             assert items[1][1] == 2

+            assert items[1][2] == 0

             assert items[2][0].subtype == "alternative"
             assert items[2][1] == 1
             assert items[2][0].subtype == "alternative"
             assert items[2][1] == 1

+            assert items[2][2] == 2

             assert items[3][0].subtype == "png"
             assert items[3][1] == 1
             assert items[3][0].subtype == "png"
             assert items[3][1] == 1

+            assert items[3][2] == 2

             assert items[4][0].subtype == "relative"
             assert items[4][1] == 0
             assert items[4][0].subtype == "relative"
             assert items[4][1] == 0

+            assert items[4][2] == 4

 
         def test_MIMETreeDFWalker_list_to_mixed(self, const1):
             mimetree = MIMETreeDFWalker()
             items = []
 
 
         def test_MIMETreeDFWalker_list_to_mixed(self, const1):
             mimetree = MIMETreeDFWalker()
             items = []
 

-            def visitor_fn(item, ancestry, debugprint):
+            def visitor_fn(item, ancestry, descendents, debugprint):

                 items.append(item)
 
             p = Part("text", "plain", const1)
                 items.append(item)
 
             p = Part("text", "plain", const1)


@@ -841,7 +896,7 @@ try:
         ):
             items = []
 
         ):
             items = []
 

-            def visitor_fn(item, ancestry, debugprint):
+            def visitor_fn(item, ancestry, descendents, debugprint):

                 items.append(item)
 
             mimetree = MIMETreeDFWalker(visitor_fn=visitor_fn)
                 items.append(item)
 
             mimetree = MIMETreeDFWalker(visitor_fn=visitor_fn)


@@ -895,10 +950,12 @@ try:
             ):
                 return mime_tree_related_to_alternative
 
             ):
                 return mime_tree_related_to_alternative
 

+            max_attachments = 5

             do_massage(
                 draft_f=string_io,
                 draftpath=const1,
                 cmd_f=sys.stdout,
             do_massage(
                 draft_f=string_io,
                 draftpath=const1,
                 cmd_f=sys.stdout,

+                max_other_attachments=max_attachments,

                 converter=converter,
             )
 
                 converter=converter,
             )
 


@@ -909,6 +966,9 @@ try:
             assert "Plain" in lines.pop()
             assert "part.html" in lines.pop()
             assert "toggle-unlink" in lines.pop()
             assert "Plain" in lines.pop()
             assert "part.html" in lines.pop()
             assert "toggle-unlink" in lines.pop()

+            for i in range(max_attachments):
+                assert "move-up" in lines.pop()
+            assert "move-down" in lines.pop()

             assert "HTML" in lines.pop()
             assert "jump>1" in lines.pop()
             assert "jump>2" in lines.pop()
             assert "HTML" in lines.pop()
             assert "jump>1" in lines.pop()
             assert "jump>2" in lines.pop()


@@ -917,6 +977,9 @@ try:
             assert "logo.png" in lines.pop()
             assert "toggle-unlink" in lines.pop()
             assert "content-id" in lines.pop()
             assert "logo.png" in lines.pop()
             assert "toggle-unlink" in lines.pop()
             assert "content-id" in lines.pop()

+            for i in range(max_attachments):
+                assert "move-up" in lines.pop()
+            assert "move-down" in lines.pop()

             assert "Logo" in lines.pop()
             assert "jump>1" in lines.pop()
             assert "jump>4" in lines.pop()
             assert "Logo" in lines.pop()
             assert "jump>1" in lines.pop()
             assert "jump>4" in lines.pop()


@@ -1168,10 +1231,23 @@ try:
             assert "Plain" in lines.pop()
             assert "part.html" in lines.pop()
             assert "toggle-unlink" in lines.pop()
             assert "Plain" in lines.pop()
             assert "part.html" in lines.pop()
             assert "toggle-unlink" in lines.pop()

+            assert "move-up" in lines.pop()
+            while True:
+                top = lines.pop()
+                if "move-up" not in top:
+                    break
+            assert "move-down" in top

             assert "HTML" in lines.pop()
             assert "logo.png" in lines.pop()
             assert "toggle-unlink" in lines.pop()
             assert "content-id" in lines.pop()
             assert "HTML" in lines.pop()
             assert "logo.png" in lines.pop()
             assert "toggle-unlink" in lines.pop()
             assert "content-id" in lines.pop()

+            assert "move-up" in lines.pop()
+            while True:
+                top = lines.pop()
+                if "move-up" not in top:
+                    break
+            assert "move-down" in top
+            assert "move-down" in lines.pop()

             assert "Logo" in lines.pop()
             assert "jump>2" in lines.pop()
             assert "jump>3" in lines.pop()
             assert "Logo" in lines.pop()
             assert "jump>2" in lines.pop()
             assert "jump>3" in lines.pop()


@@ -1184,5 +1260,87 @@ try:
             assert "send-message" in lines.pop()
             assert len(lines) == 0
 
             assert "send-message" in lines.pop()
             assert len(lines) == 0
 

+        def test_mime_tree_nested_trees_does_not_break_positioning(
+            self, string_io, const1, capsys
+        ):
+            def converter(
+                drafttext,
+                draftpath,
+                cssfile,
+                related_to_html_only,
+                extensions,
+                tempdir,
+            ):
+                return Multipart(
+                    "relative",
+                    children=[
+                        Multipart(
+                            "alternative",
+                            children=[
+                                Part(
+                                    "text",
+                                    "plain",
+                                    "part.txt",
+                                    desc="Plain",
+                                    orig=True,
+                                ),
+                                Multipart(
+                                    "alternative",
+                                    children=[
+                                        Part(
+                                            "text",
+                                            "plain",
+                                            "part.txt",
+                                            desc="Nested plain",
+                                        ),
+                                        Part(
+                                            "text",
+                                            "html",
+                                            "part.html",
+                                            desc="Nested HTML",
+                                        ),
+                                    ],
+                                    desc="Nested alternative",
+                                ),
+                            ],
+                            desc="Alternative",
+                        ),
+                        Part(
+                            "text",
+                            "png",
+                            "logo.png",
+                            cid="logo.png",
+                            desc="Logo",
+                        ),
+                    ],
+                    desc="Related",
+                )
+
+            do_massage(
+                draft_f=string_io,
+                draftpath=const1,
+                cmd_f=sys.stdout,
+                converter=converter,
+            )
+
+            captured = capsys.readouterr()
+            lines = captured.out.splitlines()
+            while not "logo.png" in lines.pop():
+                pass
+            lines.pop()
+            assert "content-id" in lines.pop()
+            assert "move-up" in lines.pop()
+            while True:
+                top = lines.pop()
+                if "move-up" not in top:
+                    break
+            assert "move-down" in top
+            # Due to the nested trees, the number of descendents of the sibling
+            # actually needs to be considered, not just the nieces. So to move
+            # from position 1 to position 6, it only needs one <move-down>
+            # because that jumps over the entire sibling tree. Thus what
+            # follows next must not be another <move-down>
+            assert "Logo" in lines.pop()
+

 except ImportError:
     pass
 except ImportError:
     pass