[PATCH] clang-query quit command

[aaron.ballman]

Summary

I would like to propose adding a clang-query command called 'quit' that terminates the clang-query interactive session. While Ctrl+C works well enough to terminate the application, it's not particularly discoverable, and it runs into an interesting conflict on Windows 10 where the terminal has received some love recently. Ctrl+C will perform a copy operation in the terminal now, depending on whether any text is selected or not. Muscle memory has made for some annoying mistakes recently for me.

I put together a patch that implements this functionality in terms of terminating the query session object.

Thanks!

[klimek]

+pcc for the opinion of the creator :)

[klimek]

For what it's worth, I think this looks good, mainly curious whether pcc has any remarks code-wise.

[pcc]

I normally quit clang-query, or pretty much any other interactive command line program, by typing end-of-file (Ctrl-D on Unix, Ctrl-Z <enter> on Windows). If this works on Windows 10 I'd rather users do it that way. We can make this more discoverable through documentation.

[aaron.ballman]

In D11761#218141, @pcc wrote:

I normally quit clang-query, or pretty much any other interactive command line program, by typing end-of-file (Ctrl-D on Unix, Ctrl-Z <enter> on Windows). If this works on Windows 10 I'd rather users do it that way. We can make this more discoverable through documentation.

While Ctrl+Z <enter> does terminate clang-query on Windows, it's an incredibly obtuse way to terminate an interactive application on Windows. (Having used Windows for 20 years, I had to go look up what Ctrl+Z does!) Saying "but we can document it" doesn't really change that fact; users can already look at the documentation on MSDN to learn that Ctrl+Break or Ctrl+C will send a SIGINT (Ctrl+Z sets the EOF indicator, which is even more obtuse).

I'm not certain that control codes to terminate interactive shell programs on Windows is as normal as it is on Unix-like systems.

~Aaron

[klimek]

Peter, any particular reason?

In D11761#218151, @aaron.ballman wrote:

In D11761#218141, @pcc wrote:

I normally quit clang-query, or pretty much any other interactive command line program, by typing end-of-file (Ctrl-D on Unix, Ctrl-Z <enter> on Windows). If this works on Windows 10 I'd rather users do it that way. We can make this more discoverable through documentation.

While Ctrl+Z <enter> does terminate clang-query on Windows, it's an incredibly obtuse way to terminate an interactive application on Windows. (Having used Windows for 20 years, I had to go look up what Ctrl+Z does!) Saying "but we can document it" doesn't really change that fact; users can already look at the documentation on MSDN to learn that Ctrl+Break or Ctrl+C will send a SIGINT (Ctrl+Z sets the EOF indicator, which is even more obtuse).

I'm not certain that control codes to terminate interactive shell programs on Windows is as normal as it is on Unix-like systems.

~Aaron

+1 to that; I do think we want this to work well for windows users, and the expected knowledge is different

[pcc]

You'd have had to look at the documentation (i.e. the "help" message) anyway to determine the right way to terminate the program. We can document Ctrl-{D,Z} there instead of introducing a new command.

But this isn't worth arguing about and I am clearly in the minority here, if you still think that adding a new command here is the way to go, LGTM.

[aaron.ballman]

In D11761#218178, @pcc wrote:

You'd have had to look at the documentation (i.e. the "help" message) anyway to determine the right way to terminate the program. We can document Ctrl-{D,Z} there instead of introducing a new command.

FWIW (and I should probably be ashamed to admit this in public), but when I first ran clang-query in interactive mode, I tried "quit", then I tried "exit", and then I just went and End Tasked the application. Control codes didn't enter my thought process, and I didn't try "help" for another few sessions. Discoverability isn't just about reading documentation, it's about ecosystem "norms" too.

But this isn't worth arguing about and I am clearly in the minority here, if you still think that adding a new command here is the way to go, LGTM.

Thank you, I've commit in r244206

~Aaron

clang-query/Query.h

   QK_Match,
   QK_SetBool,
   QK_SetOutputKind,
+  QK_Quit
 };
 
 class QuerySession;

   static bool classof(const Query *Q) { return Q->Kind == QK_Help; }
 };
 
+/// Query for "quit".
+struct QuitQuery : Query {
+  QuitQuery() : Query(QK_Quit) {}
+  bool run(llvm::raw_ostream &OS, QuerySession &QS) const override;
+
+  static bool classof(const Query *Q) { return Q->Kind == QK_Quit; }
+};
+
 /// Query for "match MATCHER".
 struct MatchQuery : Query {
   MatchQuery(const ast_matchers::dynamic::DynTypedMatcher &Matcher)

clang-query/Query.cpp

         "  set output (diag|print|dump)      "
         "Set whether to print bindings as diagnostics,\n"
         "                                    "
-        "AST pretty prints or AST dumps.\n\n";
+        "AST pretty prints or AST dumps.\n"
+        "  quit                              "
+        "Terminates the query session.\n\n";
   return true;
 }
 
+bool QuitQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
+  QS.Terminate = true;
+  return true;
+}
+
 namespace {
 
 struct CollectBoundNodes : MatchFinder::MatchCallback {

clang-query/QueryParser.cpp

   PQK_Match,
   PQK_Set,
   PQK_Unlet,
+  PQK_Quit
 };
 
 enum ParsedQueryVariable {

                               .Case("match", PQK_Match)
                               .Case("set", PQK_Set)
                               .Case("unlet", PQK_Unlet)
+                              .Case("quit", PQK_Quit)
                               .Default(PQK_Invalid);
 
   switch (QKind) {

   case PQK_Help:
     return endQuery(new HelpQuery);
 
+  case PQK_Quit:
+    return endQuery(new QuitQuery);
+
   case PQK_Let: {
     StringRef Name = lexWord();
 

clang-query/QuerySession.h

 class QuerySession {
 public:
   QuerySession(llvm::ArrayRef<std::unique_ptr<ASTUnit>> ASTs)
-      : ASTs(ASTs), OutKind(OK_Diag), BindRoot(true) {}
+      : ASTs(ASTs), OutKind(OK_Diag), BindRoot(true), Terminate(false) {}
 
   llvm::ArrayRef<std::unique_ptr<ASTUnit>> ASTs;
   OutputKind OutKind;
   bool BindRoot;
+  bool Terminate;
   llvm::StringMap<ast_matchers::dynamic::VariantValue> NamedValues;
 };
 

clang-query/tool/ClangQuery.cpp

       QueryRef Q = QueryParser::parse(*Line, QS);
       Q->run(llvm::outs(), QS);
       llvm::outs().flush();
+      if (QS.Terminate)
+        break;
     }
   }