From c80ba6e8dbcb59e696249a1cd4d78bc2c519fc85 Mon Sep 17 00:00:00 2001
From: silverweed <giacomo.parolini@cern.ch>
Date: Tue, 25 Nov 2025 11:37:09 +0100
Subject: [PATCH] [main] rootls: improve recursive printing

The current algorithm didn't properly handle certain cases when the -r
flag is used (namely, directories were not printed in a newline)

(cherry picked from commit 2d8b74950e337e8cae06c2f2f1709e7ec9025507)
---
 main/src/rootls.cxx                | 39 +++++++++++++++++++++---------
 roottest/main/CMakeLists.txt       |  5 ++++
 roottest/main/RecursiveRootls3.ref |  6 +++++
 3 files changed, 39 insertions(+), 11 deletions(-)
 create mode 100644 roottest/main/RecursiveRootls3.ref

diff --git a/main/src/rootls.cxx b/main/src/rootls.cxx
index 573d34f36391c..cc0a3a7ab22d7 100644
--- a/main/src/rootls.cxx
+++ b/main/src/rootls.cxx
@@ -281,7 +281,7 @@ static void PrintRNTuple(std::ostream &stream, const ROOT::RNTupleDescriptor &de
    std::size_t maxNameLen = 0, maxTypeLen = 0;
    std::vector<const ROOT::RFieldDescriptor *> fields;
    fields.reserve(rootField.GetLinkIds().size());
-   for (const auto &field: desc.GetFieldIterable(rootField.GetId())) {
+   for (const auto &field : desc.GetFieldIterable(rootField.GetId())) {
       fields.push_back(&field);
       maxNameLen = std::max(maxNameLen, field.GetFieldName().length());
       maxTypeLen = std::max(maxTypeLen, field.GetTypeName().length());
@@ -481,11 +481,11 @@ static void PrintNodesInColumns(std::ostream &stream, const RootLsTree &tree,
 
    const bool isTerminal = terminalSize.x + terminalSize.y > 0;
 
-   bool mustIndent = false;
+   auto curCol = 0u;
    for (auto i = 0u; i < nNodes; ++i) {
       NodeIdx childIdx = nodesBegin[i];
       const auto &child = tree.fNodes[childIdx];
-      if ((i % nCols) == 0 || mustIndent) {
+      if (curCol == 0) {
          PrintIndent(stream, indent);
       }
 
@@ -498,22 +498,39 @@ static void PrintNodesInColumns(std::ostream &stream, const RootLsTree &tree,
             stream << Color(kAnsiGreen);
       }
 
-      const bool isExtremal = !(((i + 1) % nCols) != 0 && i != nNodes - 1);
+      // Handle line breaks. Lines are broken in the following situations:
+      // - when the current column number reaches the max number of columns
+      // - when we are in recursive mode and the item is a directory with children
+      // - when we are in recursive mode and the NEXT item is a directory with children
+      
+      const bool isDirWithRecursiveDisplay = isDir && (flags & RootLsArgs::kRecursiveListing) && child.fNChildren > 0;
+
+      bool nextIsDirWithRecursiveDisplay = false;
+      if ((flags & RootLsArgs::kRecursiveListing) && i < nNodes - 1) {
+         NodeIdx nextChildIdx = nodesBegin[i + 1];
+         const auto &nextChild = tree.fNodes[nextChildIdx];
+         nextIsDirWithRecursiveDisplay =
+            nextChild.fNChildren > 0 && ClassInheritsFrom(nextChild.fClassName.c_str(), "TDirectory");
+      }
+
+      const bool isExtremal = (((curCol + 1) % nCols) == 0) || (i == nNodes - 1) || isDirWithRecursiveDisplay ||
+                              nextIsDirWithRecursiveDisplay;
       if (!isExtremal) {
-         stream << std::left << std::setw(colWidths[i % nCols]) << child.fName;
+         stream << std::left << std::setw(colWidths[curCol % nCols]) << child.fName;
       } else {
          stream << std::setw(1) << child.fName;
       }
       stream << Color(kAnsiNone);
 
-      if (isExtremal)
-         stream << "\n";
+      if (isExtremal) {
+         stream << '\n';
+         curCol = 0;
+      } else {
+         ++curCol;
+      }
 
-      if (isDir && (flags & RootLsArgs::kRecursiveListing)) {
-         if (!isExtremal)
-            stream << "\n";
+      if (isDirWithRecursiveDisplay) {
          PrintChildrenInColumns(stream, tree, childIdx, flags, indent + 2);
-         mustIndent = true;
       }
    }
 }
diff --git a/roottest/main/CMakeLists.txt b/roottest/main/CMakeLists.txt
index 8b81e9a5da861..5a9bea3c8d9fc 100644
--- a/roottest/main/CMakeLists.txt
+++ b/roottest/main/CMakeLists.txt
@@ -91,6 +91,11 @@ ROOTTEST_ADD_TEST(RecursiveRootls2
                   OUTREF RecursiveRootls2.ref
                   ENVIRONMENT ${test_env})
 
+ROOTTEST_ADD_TEST(RecursiveRootls3
+                  COMMAND ${TOOLS_PREFIX}/rootls${exeext} -r subdirs.root
+                  OUTREF RecursiveRootls3.ref
+                  ENVIRONMENT ${test_env})
+
 ROOTTEST_ADD_TEST(RootlsRNTuple
                   COMMAND ${TOOLS_PREFIX}/rootls${exeext} -R RNTuple.root
                   OUTREF RootlsRNTuple.ref
diff --git a/roottest/main/RecursiveRootls3.ref b/roottest/main/RecursiveRootls3.ref
new file mode 100644
index 0000000000000..5e152d052b2ac
--- /dev/null
+++ b/roottest/main/RecursiveRootls3.ref
@@ -0,0 +1,6 @@
+sub
+  s1  s2
+  s3
+    t1
+      c
+sub2 sub3