TW-1752: cleanup of diag output

- Thanks to David Patrick.

ChangeLog

@@ -38,6 +38,7 @@
           Misty De Meo).
 - TW-1750 REG_ENHANCED, used in RX.cpp, isn't defined in all versions of Darwin
           (thanks to Misty De Meo).
+- TW-1752 cleanup of diag output (thanks to David Patrick).
 - Fixed broken build for Cygwin and older GCC (thanks to Richard Boß).
 - The default configuration is now 256-color only.
 - The 'columns' report now shows whether a column is modifiable or read only.

src/commands/CmdDiagnostics.cpp

@@ -305,28 +305,76 @@ int CmdDiagnostics::execute (std::string& output)
       << "\n\n";
 
   // Disaply hook status.
+  Path hookLocation (context.config.get ("data.location"));
+  hookLocation += "hooks";
+
   out << bold.colorize (STRING_CMD_DIAG_HOOKS)
       << "\n"
-      << "    Scripts: "
+      << "     System: "
       << (context.config.getBoolean ("hooks") ? STRING_CMD_DIAG_HOOK_ENABLE : STRING_CMD_DIAG_HOOK_DISABLE)
+      << "\n"
+      << "   Location: "
+      << static_cast <std::string> (hookLocation)
       << "\n";
 
-  std::vector <std::string> hooks = context.hooks.list ();
+  auto hooks = context.hooks.list ();
   if (hooks.size ())
   {
+    unsigned int longest = 0;
+    for (auto& hook : hooks)
+      if (hook.length () > longest)
+        longest = hook.length ();
+    longest -= hookLocation._data.length () + 1;
+
+    out << "     Active: ";
+    int count = 0;
+    for (auto& hook : hooks)
+    {
+      Path p (hook);
+
+      std::string name = p.name ();
+
+      if (p.executable () &&
+          (name.substr (0, 6) == "on-add"    ||
+           name.substr (0, 9) == "on-modify" ||
+           name.substr (0, 9) == "on-launch" ||
+           name.substr (0, 7) == "on-exit"))
+      {
+        out << (count++ ? "             " : "");
+
+        out.width (longest);
+        out << std::left << name
+            << format (" ({1})", STRING_CMD_DIAG_HOOK_EXEC)
+            << (p.is_link () ? format (" ({1})", STRING_CMD_DIAG_HOOK_SYMLINK) : "")
+            << "\n";
+      }
+    }
+
+    out << "   Inactive: ";
+    count = 0;
     for (auto& hook : hooks)
     {
       Path p (hook);
       std::string name = p.name ();
-      out << "             "
-          << hook
-          << (p.executable () ? format (" ({1})", STRING_CMD_DIAG_HOOK_EXEC) : format (" ({1})", STRING_CMD_DIAG_HOOK_NO_EXEC))
-          << (p.is_link () ? format (" ({1})", STRING_CMD_DIAG_HOOK_SYMLINK) : "")
-          << ((name.substr (0, 6) == "on-add" ||
-               name.substr (0, 9) == "on-modify" ||
-               name.substr (0, 9) == "on-launch" ||
-               name.substr (0, 7) == "on-exit") ? "" : format (" ({1})", STRING_CMD_DIAG_HOOK_NAME))
-          << "\n";
+
+      if (! p.executable () ||
+          (name.substr (0, 6) != "on-add"    &&
+           name.substr (0, 9) != "on-modify" &&
+           name.substr (0, 9) != "on-launch" &&
+           name.substr (0, 7) != "on-exit"))
+      {
+        out << (count++ ? "             " : "");
+
+        out.width (longest);
+        out << std::left << name
+            << (p.executable () ? format (" ({1})", STRING_CMD_DIAG_HOOK_EXEC) : format (" ({1})", STRING_CMD_DIAG_HOOK_NO_EXEC))
+            << (p.is_link () ? format (" ({1})", STRING_CMD_DIAG_HOOK_SYMLINK) : "")
+            << ((name.substr (0, 6) == "on-add" ||
+                 name.substr (0, 9) == "on-modify" ||
+                 name.substr (0, 9) == "on-launch" ||
+                 name.substr (0, 7) == "on-exit") ? "" : format (" ({1})", STRING_CMD_DIAG_HOOK_NAME))
+            << "\n";
+      }
     }
   }
   else

test/hooks.env.t

@@ -68,7 +68,7 @@ class TestHooksOnLaunch(TestCase):
         self.t.hooks.add_default(hookname, log=True)
 
         code, out, err = self.t("diagnostics")
-        self.assertIn("on-launch-good-env (executable)", out)
+        self.assertRegexpMatches(out, r"on-launch-good-env\s+\(executable\)")
 
     def test_onlaunch_builtin_env_debug(self):
         """Verify that 'debug.hooks' shows hook details"""