[rootbrowse] better option parsing, exit if in batch mode

Author: silverweed

main/CMakeLists.txt

@@ -111,9 +111,7 @@ if(WIN32)
   set_property(TARGET rootcling APPEND_STRING PROPERTY LINK_FLAGS " -STACK:4000000")
 endif()
 
-if(gui)
-  ROOT_EXECUTABLE(rootbrowse src/rootbrowse.cxx LIBRARIES RIO Core Rint Gui)
-endif()
+ROOT_EXECUTABLE(rootbrowse src/rootbrowse.cxx LIBRARIES RIO Core Rint Gui)
 ROOT_EXECUTABLE(rootls src/rootls.cxx LIBRARIES RIO Tree Core Rint ROOTNTuple)
 
 # Create aliases: rootcint, genreflex.

main/src/rootbrowse.cxx

@@ -10,6 +10,7 @@
 #include <TBrowser.h>
 #include <TError.h>
 #include <TFile.h>
+#include <TGFrame.h>
 #include <TROOT.h>
 #include <TSystem.h>
 
@@ -18,6 +19,7 @@
 #include <iostream>
 #include <memory>
 #include <thread>
+#include <string_view>
 
 static const char *const kShortHelp = "usage: rootbrowse [-w WEB|-wf] <file.root>\n";
 static const char *const kLongHelp = R"(
@@ -53,8 +55,8 @@ struct RootBrowseArgs {
       kLong
    };
    EPrintUsage fPrintHelp = EPrintUsage::kNo;
-   const char *fWeb = "on";
-   const char *fFileName = nullptr;
+   std::string_view fWeb;
+   std::string_view fFileName;
 };
 
 static RootBrowseArgs ParseArgs(const char **args, int nArgs)
@@ -64,25 +66,50 @@ static RootBrowseArgs ParseArgs(const char **args, int nArgs)
 
    for (int i = 0; i < nArgs; ++i) {
       const char *arg = args[i];
+
+      if (strcmp(arg, "--") == 0) {
+         forcePositional = true;
+         continue;
+      }
+
       bool isFlag = !forcePositional && arg[0] == '-';
       if (isFlag) {
-         if (strcmp(arg, "-w") == 0 || strcmp(arg, "--web") == 0) {
+         ++arg;
+         // Parse long or short flag and its argument into `argStr` / `nxtArgStr`.
+         std::string_view argStr, nxtArgStr;
+         if (arg[0] == '-') {
+            ++arg;
+            // long flag: may be either of the form `--web off` or `--web=off`
+            const char *eq = strchr(arg, '=');
+            if (eq) {
+               argStr = std::string_view(arg, eq - arg);
+               nxtArgStr = std::string_view(eq + 1);
+            } else {
+               argStr = std::string_view(arg);
+               if (i < nArgs - 1 && args[i + 1][0] != '-') {
+                  nxtArgStr = args[i + 1];
+                  ++i;
+               }
+            }
+         } else {
+            // short flag (note that it might be more than 1 character long, like `-wf`)
+            argStr = std::string_view(arg);
             if (i < nArgs - 1 && args[i + 1][0] != '-') {
+               nxtArgStr = args[i + 1];
                ++i;
-               outArgs.fWeb = args[i];
             }
-         } else if (strcmp(arg, "-wf") == 0 || strcmp(arg, "--webOff") == 0) {
-            outArgs.fWeb = "off";
-         } else if (strcmp(arg, "-h") == 0 || strcmp(arg, "--help") == 0) {
+         }
+
+         if (argStr == "w" || argStr == "web") {
+            outArgs.fWeb = nxtArgStr.empty() ? "on" : nxtArgStr;
+         } else if (argStr == "h" || argStr == "help") {
             outArgs.fPrintHelp = RootBrowseArgs::EPrintUsage::kLong;
             break;
-         } else if (strcmp(arg, "--") == 0) {
-            forcePositional = true;
-         } else {
-            outArgs.fPrintHelp = RootBrowseArgs::EPrintUsage::kShort;
-            break;
+         } else if (argStr == "wf" || argStr == "--webOff") {
+            outArgs.fWeb = "off";
          }
-      } else if (outArgs.fFileName) {
+
+      } else if (!outArgs.fFileName.empty()) {
          outArgs.fPrintHelp = RootBrowseArgs::EPrintUsage::kShort;
          break;
       } else {
@@ -105,23 +132,35 @@ int main(int argc, char **argv)
       return 1;
    }
 
-   gROOT->SetWebDisplay(args.fWeb);
+   // NOTE: we need to instantiate TApplication ourselves, otherwise TBrowser
+   // will create a batch application that cannot show graphics.
+   TApplication app("rootbrowse", nullptr, nullptr);
+
+   if (!args.fWeb.empty())
+      gROOT->SetWebDisplay(std::string(args.fWeb).c_str());
 
    std::unique_ptr<TFile> file;
-   if (args.fFileName) {
+   if (!args.fFileName.empty()) {
       gErrorIgnoreLevel = kError;
-      file = std::unique_ptr<TFile>(TFile::Open(args.fFileName, "READ"));
+      file = std::unique_ptr<TFile>(TFile::Open(std::string(args.fFileName).c_str(), "READ"));
       if (!file || file->IsZombie()) {
          R__LOG_WARNING(RootBrowseLog()) << "File " << args.fFileName << " does not exist or is unreadable.";
       }
       gErrorIgnoreLevel = kUnset;
    }
 
-   // NOTE: we need to instantiate TApplication ourselves, otherwise TBrowser
-   // will create a batch application that cannot show graphics.
-   TApplication app("rootbrowse", nullptr, nullptr);
-
    auto browser = std::make_unique<TBrowser>();
+
+   if (gROOT->IsBatch())
+      return 1;
+
+   // For classic graphics: ensure rootbrowse quits when the window is closed
+   if (auto imp = browser->GetBrowserImp()) {
+      if (auto mainframe = imp->GetMainFrame()) {        
+         mainframe->Connect("CloseWindow()", "TApplication", &app, "Terminate()");
+      }
+   }
+
    std::cout << "Press ctrl+c to exit.\n";
    while (!gROOT->IsInterrupted() && !gSystem->ProcessEvents()) {
       std::this_thread::sleep_for(std::chrono::milliseconds(10));