Extract runCommandsInFile method

[aaron.ballman]

I'm not opposed to the changes here, but I am wondering what the benefits are to splitting this off into its own function?

clang-query/tool/ClangQuery.cpp

61 ↗

(On Diff #162554)

I think this function should return a bool instead of an int. The caller can decide how to translate failure into a return code.

61–62 ↗

(On Diff #162554)

You should run your patch through clang-format to properly format only the parts that you've changed. Also, it should be ExeName and FileName per the coding standard.

[steveire]

Thanks, I fixed the issues before committing. This commit was a prerequisite to https://reviews.llvm.org/D51261

clang-tools-extra/trunk/clang-query/tool/ClangQuery.cpp

static cl::list<std::string> Commands("c", cl::desc("Specify command to run"),
                                       cl::value_desc("command"),
                                       cl::cat(ClangQueryCategory));
 
 static cl::list<std::string> CommandFiles("f",
                                           cl::desc("Read commands from file"),
                                           cl::value_desc("file"),
                                           cl::cat(ClangQueryCategory));
 
+bool runCommandsInFile(const char *ExeName, std::string const &FileName,
+                       QuerySession &QS) {
+  std::ifstream Input(FileName.c_str());
+  if (!Input.is_open()) {
+    llvm::errs() << ExeName << ": cannot open " << FileName << "\n";
+    return 1;
+  }
+  while (Input.good()) {
+    std::string Line;
+    std::getline(Input, Line);
+
+    QueryRef Q = QueryParser::parse(Line, QS);
+    if (!Q->run(llvm::outs(), QS))
+      return true;
+  }
+  return false;
+}
+
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
   CommonOptionsParser OptionsParser(argc, argv, ClangQueryCategory);
 
   if (!Commands.empty() && !CommandFiles.empty()) {
     llvm::errs() << argv[0] << ": cannot specify both -c and -f\n";
     return 1;
int main(int argc, const char **argv) {
   if (!Commands.empty()) {
     for (auto I = Commands.begin(), E = Commands.end(); I != E; ++I) {
       QueryRef Q = QueryParser::parse(*I, QS);
       if (!Q->run(llvm::outs(), QS))
         return 1;
     }
   } else if (!CommandFiles.empty()) {
     for (auto I = CommandFiles.begin(), E = CommandFiles.end(); I != E; ++I) {
-      std::ifstream Input(I->c_str());
-      if (!Input.is_open()) {
-        llvm::errs() << argv[0] << ": cannot open " << *I << "\n";
+      if (runCommandsInFile(argv[0], *I, QS))
         return 1;
-      }
-      while (Input.good()) {
-        std::string Line;
-        std::getline(Input, Line);
-
-        QueryRef Q = QueryParser::parse(Line, QS);
-        if (!Q->run(llvm::outs(), QS))
-          return 1;
-      }
-    }
   } else {
     LineEditor LE("clang-query");
     LE.setListCompleter([&QS](StringRef Line, size_t Pos) {
       return QueryParser::complete(Line, Pos, QS);
     });
     while (llvm::Optional<std::string> Line = LE.readLine()) {
       QueryRef Q = QueryParser::parse(*Line, QS);
       Q->run(llvm::outs(), QS);
       llvm::outs().flush();
       if (QS.Terminate)
         break;
     }
   }
 
   return 0;
 }