From 4e83fbb99ce74f5d0c6cd09f4951f87a0f1bc557 Mon Sep 17 00:00:00 2001
From: Benedikt Nerb <nerb.benedikt@gmail.com>
Date: Sun, 1 Feb 2026 22:05:57 +0100
Subject: [PATCH] Fix and improve command usage printing

---
 pumpkin/src/command/tree/format.rs | 95 ++++++++++++++++++------------
 1 file changed, 56 insertions(+), 39 deletions(-)

diff --git a/pumpkin/src/command/tree/format.rs b/pumpkin/src/command/tree/format.rs
index 8c2eb061e..89ff3a801 100644
--- a/pumpkin/src/command/tree/format.rs
+++ b/pumpkin/src/command/tree/format.rs
@@ -40,7 +40,7 @@ fn flatten_require_nodes(nodes: &[Node], children: &[usize]) -> Vec<usize> {
         let node = &nodes[i];
         match &node.node_type {
             NodeType::Require { .. } => {
-                new_children.extend(flatten_require_nodes(nodes, node.children.as_slice()));
+                new_children.extend(flatten_require_nodes(nodes, &node.children));
             }
             _ => new_children.push(i),
         }
@@ -49,57 +49,74 @@ fn flatten_require_nodes(nodes: &[Node], children: &[usize]) -> Vec<usize> {
     new_children
 }
 
-impl Display for CommandTree {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        write!(f, "/{}", self.names[0])?;
+fn write_tree(f: &mut Formatter<'_>, nodes: &[Node], children: &[usize]) -> std::fmt::Result {
+    // Map node indices to actual nodes
+    // NOTE: We assume that Require nodes have already been "flattened" out
+    let nodes_iter = children.iter().map(|&idx| &nodes[idx]);
 
-        let current_children = &self.children[..];
+    // Check if there is a sibling of type ExecuteLeaf
+    // If there is, arguments on the current level are optional and will be printed surrounded by square brackets
+    let argument_is_optional = nodes_iter
+        .clone()
+        .any(|node| matches!(node.node_type, NodeType::ExecuteLeaf { .. }));
 
-        while !current_children.is_empty() {
-            let flattened = flatten_require_nodes(&self.nodes, current_children);
+    let visible_nodes: Vec<&Node> = nodes_iter
+        .clone()
+        .filter(|node| node.is_visible())
+        .collect();
 
-            let mut visible_iter = flattened
-                .iter()
-                .copied()
-                .filter(|&i| self.nodes[i].is_visible());
+    if visible_nodes.is_empty() {
+        return Ok(());
+    }
 
-            let Some(first_idx) = visible_iter.next() else {
-                break;
-            };
+    let multiple_visible_nodes = visible_nodes.len() > 1;
 
-            let second_idx = visible_iter.next();
+    write!(f, " ")?;
 
-            f.write_char(' ')?;
+    if argument_is_optional {
+        write!(f, "[")?;
+    }
 
-            let is_optional = flattened
-                .iter()
-                .any(|&i| matches!(self.nodes[i].node_type, NodeType::ExecuteLeaf { .. }));
+    if multiple_visible_nodes {
+        write!(f, "(")?;
+    }
 
-            if is_optional {
-                f.write_char('[')?;
-            }
+    for (idx, &node) in visible_nodes.iter().enumerate() {
+        // Print usage of this node
+        write!(f, "{node}")?;
 
-            if let Some(second_idx) = second_idx {
-                f.write_char('(')?;
-                self.nodes[first_idx].fmt(f)?;
+        // Recursively print usage of it's children
+        write_tree(f, nodes, &node.children)?;
 
-                write!(f, " | ")?;
-                self.nodes[second_idx].fmt(f)?;
+        if multiple_visible_nodes && idx != visible_nodes.len() - 1 {
+            write!(f, " | ")?;
+        }
+    }
 
-                for idx in visible_iter {
-                    write!(f, " | ")?;
-                    self.nodes[idx].fmt(f)?;
-                }
-                f.write_char(')')?;
+    if multiple_visible_nodes {
+        write!(f, ")")?;
+    }
 
-                break;
-            }
+    if argument_is_optional {
+        write!(f, "]")?;
+    }
 
-            if is_optional {
-                f.write_char(']')?;
-            }
-        }
+    Ok(())
+}
 
-        Ok(())
+impl Display for CommandTree {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        write!(f, "/{}", self.names[0])?;
+        // TODO: Help of commands like /bossbar becomes really long
+        //       A possible solution would be to add a check if the first level consists of literals
+        //       only and if so to run the printing as if it were separate commands
+
+        // TODO: Consider adding a max depth to print command usage only up to a certain depth.
+        //       Vanilla seems to do this too (as can be seen with /help effect)
+
+        // Clean up graph by 'flattening' require nodes to their children
+        let flattened = flatten_require_nodes(&self.nodes, &self.children);
+        // Recursively iterate over tree to print help usage of a command
+        write_tree(f, &self.nodes, &flattened)
     }
 }