Refactor tool selection for edit_cache (#14544)

Refactor edit_cache tool selection to ask each global generator for its
preference.  Teach the Ninja generator to always use cmake-gui because
Ninja by design cannot run interactive terminal dialogs like ccmake.
Teach the Makefile generator to use cmake-gui when also using an "extra"
generator whose IDE has no terminal to run ccmake, and otherwise fall
back to CMAKE_EDIT_COMMAND selection for normal Makefile build systems.

Help/variable/CMAKE_EDIT_COMMAND.rst

@@ -1,7 +1,8 @@
 CMAKE_EDIT_COMMAND
 ------------------
 
-Full path to cmake-gui or ccmake.
+Full path to cmake-gui or ccmake.  Defined only for Makefile generators
+when not using an "extra" generator for an IDE.
 
 This is the full path to the CMake executable that can graphically
 edit the cache.  For example, cmake-gui or ccmake.

Source/cmExtraCodeBlocksGenerator.cxx

@@ -329,31 +329,11 @@ void cmExtraCodeBlocksGenerator
         {
         case cmTarget::GLOBAL_TARGET:
           {
-          bool insertTarget = false;
           // Only add the global targets from CMAKE_BINARY_DIR,
           // not from the subdirs
           if (strcmp(makefile->GetStartOutputDirectory(),
                      makefile->GetHomeOutputDirectory())==0)
             {
-            insertTarget = true;
-            // only add the "edit_cache" target if it's not ccmake, because
-            // this will not work within the IDE
-            if (ti->first == "edit_cache")
-              {
-              const char* editCommand = makefile->GetDefinition
-                                                        ("CMAKE_EDIT_COMMAND");
-              if (editCommand == 0)
-                {
-                insertTarget = false;
-                }
-              else if (strstr(editCommand, "ccmake")!=NULL)
-                {
-                insertTarget = false;
-                }
-              }
-            }
-          if (insertTarget)
-            {
             this->AppendTarget(fout, ti->first.c_str(), 0,
                                make.c_str(), makefile, compiler.c_str());
             }

Source/cmExtraEclipseCDT4Generator.cxx

@@ -995,29 +995,9 @@ void cmExtraEclipseCDT4Generator::CreateCProjectFile() const
         {
         case cmTarget::GLOBAL_TARGET:
           {
-          bool insertTarget = false;
           // Only add the global targets from CMAKE_BINARY_DIR,
           // not from the subdirs
           if (subdir.empty())
-           {
-           insertTarget = true;
-           // only add the "edit_cache" target if it's not ccmake, because
-           // this will not work within the IDE
-           if (ti->first == "edit_cache")
-             {
-             const char* editCommand = makefile->GetDefinition
-                                                        ("CMAKE_EDIT_COMMAND");
-             if (editCommand == 0)
-               {
-               insertTarget = false;
-               }
-             else if (strstr(editCommand, "ccmake")!=NULL)
-               {
-               insertTarget = false;
-               }
-             }
-           }
-         if (insertTarget)
            {
            this->AppendTarget(fout, ti->first, make, makeArgs, subdir, ": ");
            }

Source/cmExtraSublimeTextGenerator.cxx

@@ -172,31 +172,11 @@ void cmExtraSublimeTextGenerator::
         {
         case cmTarget::GLOBAL_TARGET:
           {
-          bool insertTarget = false;
           // Only add the global targets from CMAKE_BINARY_DIR,
           // not from the subdirs
           if (strcmp(makefile->GetStartOutputDirectory(),
                      makefile->GetHomeOutputDirectory())==0)
             {
-            insertTarget = true;
-            // only add the "edit_cache" target if it's not ccmake, because
-            // this will not work within the IDE
-            if (ti->first == "edit_cache")
-              {
-              const char* editCommand = makefile->GetDefinition
-                                                        ("CMAKE_EDIT_COMMAND");
-              if (editCommand == 0)
-                {
-                insertTarget = false;
-                }
-              else if (strstr(editCommand, "ccmake")!=NULL)
-                {
-                insertTarget = false;
-                }
-              }
-            }
-          if (insertTarget)
-            {
             this->AppendTarget(fout, ti->first.c_str(), *lg, 0,
                                make.c_str(), makefile, compiler.c_str(),
                                sourceFileFlags, false);

Source/cmGlobalGenerator.cxx

@@ -2158,11 +2158,11 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
     singleLine.erase(singleLine.begin(), singleLine.end());
     depends.erase(depends.begin(), depends.end());
 
-    // Use CMAKE_EDIT_COMMAND for the edit_cache rule if it is defined.
+    // Use generator preference for the edit_cache rule if it is defined.
-    // Otherwise default to the interactive command-line interface.
+    std::string edit_cmd = this->GetEditCacheCommand();
-    if(mf->GetDefinition("CMAKE_EDIT_COMMAND"))
+    if (!edit_cmd.empty())
       {
-      singleLine.push_back(mf->GetDefinition("CMAKE_EDIT_COMMAND"));
+      singleLine.push_back(edit_cmd);
       singleLine.push_back("-H$(CMAKE_SOURCE_DIR)");
       singleLine.push_back("-B$(CMAKE_BINARY_DIR)");
       cpackCommandLines.push_back(singleLine);
@@ -2174,13 +2174,14 @@ void cmGlobalGenerator::CreateDefaultGlobalTargets(cmTargets* targets)
     else
       {
       singleLine.push_back(cmakeCommand);
-      singleLine.push_back("-i");
+      singleLine.push_back("-E");
-      singleLine.push_back(".");
+      singleLine.push_back("echo");
+      singleLine.push_back("No interactive CMake dialog available.");
       cpackCommandLines.push_back(singleLine);
       (*targets)[editCacheTargetName] =
         this->CreateGlobalTarget(
           editCacheTargetName,
-          "Running interactive CMake command-line interface...",
+          "No interactive CMake dialog available...",
           &cpackCommandLines, depends, 0);
       }
     }

Source/cmGlobalGenerator.h

@@ -251,6 +251,9 @@ public:
   virtual const char* GetRebuildCacheTargetName()  const { return 0; }
   virtual const char* GetCleanTargetName()         const { return 0; }
 
+  // Lookup edit_cache target command preferred by this generator.
+  virtual std::string GetEditCacheCommand() const { return ""; }
+
   // Class to track a set of dependencies.
   typedef cmTargetDependSet TargetDependSet;
 

Source/cmGlobalNinjaGenerator.cxx

@@ -637,6 +637,13 @@ bool cmGlobalNinjaGenerator::HasRule(const std::string &name)
 //----------------------------------------------------------------------------
 // Private virtual overrides
 
+std::string cmGlobalNinjaGenerator::GetEditCacheCommand() const
+{
+  // Ninja by design does not run interactive tools in the terminal,
+  // so our only choice is cmake-gui.
+  return cmSystemTools::GetCMakeGUICommand();
+}
+
 // TODO: Refactor to combine with cmGlobalUnixMakefileGenerator3 impl.
 void cmGlobalNinjaGenerator::ComputeTargetObjects(cmGeneratorTarget* gt) const
 {

Source/cmGlobalNinjaGenerator.h

@@ -306,6 +306,7 @@ protected:
 
 
 private:
+  virtual std::string GetEditCacheCommand() const;
 
   /// @see cmGlobalGenerator::ComputeTargetObjects
   virtual void ComputeTargetObjects(cmGeneratorTarget* gt) const;

Source/cmGlobalUnixMakefileGenerator3.cxx

@@ -67,6 +67,42 @@ void cmGlobalUnixMakefileGenerator3
   entry.Brief = "Generates standard UNIX makefiles.";
 }
 
+//----------------------------------------------------------------------------
+std::string cmGlobalUnixMakefileGenerator3::GetEditCacheCommand() const
+{
+  // If generating for an extra IDE, the edit_cache target cannot
+  // launch a terminal-interactive tool, so always use cmake-gui.
+  if(this->GetExtraGeneratorName())
+    {
+    return cmSystemTools::GetCMakeGUICommand();
+    }
+
+  // Use an internal cache entry to track the latest dialog used
+  // to edit the cache, and use that for the edit_cache target.
+  cmake* cm = this->GetCMakeInstance();
+  std::string editCacheCommand = cm->GetCMakeEditCommand();
+  if(!cm->GetCacheDefinition("CMAKE_EDIT_COMMAND") ||
+     !editCacheCommand.empty())
+    {
+    if(editCacheCommand.empty())
+      {
+      editCacheCommand = cmSystemTools::GetCMakeCursesCommand();
+      }
+    if(editCacheCommand.empty())
+      {
+      editCacheCommand = cmSystemTools::GetCMakeGUICommand();
+      }
+    if(!editCacheCommand.empty())
+      {
+      cm->AddCacheEntry
+        ("CMAKE_EDIT_COMMAND", editCacheCommand.c_str(),
+         "Path to cache edit program executable.", cmCacheManager::INTERNAL);
+      }
+    }
+  const char* edit_cmd = cm->GetCacheDefinition("CMAKE_EDIT_COMMAND");
+  return edit_cmd? edit_cmd : "";
+}
+
 //----------------------------------------------------------------------------
 void
 cmGlobalUnixMakefileGenerator3
@@ -99,6 +135,14 @@ cmGlobalUnixMakefileGenerator3
     }
 }
 
+void cmGlobalUnixMakefileGenerator3::Configure()
+{
+  // Initialize CMAKE_EDIT_COMMAND cache entry.
+  this->GetEditCacheCommand();
+
+  this->cmGlobalGenerator::Configure();
+}
+
 void cmGlobalUnixMakefileGenerator3::Generate()
 {
   // first do superclass method

Source/cmGlobalUnixMakefileGenerator3.h

@@ -77,6 +77,8 @@ public:
   virtual void EnableLanguage(std::vector<std::string>const& languages,
                               cmMakefile *, bool optional);
 
+  virtual void Configure();
+
   /**
    * Generate the all required files for building this project/tree. This
    * basically creates a series of LocalGenerators for each directory and
@@ -186,6 +188,7 @@ protected:
 
   cmGeneratedFileStream *CommandDatabase;
 private:
+  virtual std::string GetEditCacheCommand() const;
   virtual void ComputeTargetObjects(cmGeneratorTarget* gt) const;
 };
 

Source/cmLocalUnixMakefileGenerator3.cxx

@@ -699,17 +699,6 @@ cmLocalUnixMakefileGenerator3
     << "# Escaping for special characters.\n"
     << "EQUALS = =\n"
     << "\n";
-
-  if(const char* edit_cmd =
-     this->Makefile->GetDefinition("CMAKE_EDIT_COMMAND"))
-    {
-    makefileStream
-      << "# The program to use to edit the cache.\n"
-      << "CMAKE_EDIT_COMMAND = "
-      << this->ConvertShellCommand(edit_cmd, FULL) << "\n"
-      << "\n";
-    }
-
   makefileStream
     << "# The top-level source directory on which CMake was run.\n"
     << "CMAKE_SOURCE_DIR = "

Source/cmake.cxx

@@ -944,38 +944,6 @@ int cmake::AddCMakePaths()
     ("CMAKE_CPACK_COMMAND", cmSystemTools::GetCPackCommand().c_str(),
      "Path to cpack program executable.", cmCacheManager::INTERNAL);
 #endif
-  // if the edit command is not yet in the cache,
-  // or if CMakeEditCommand has been set on this object,
-  // then set the CMAKE_EDIT_COMMAND in the cache
-  // This will mean that the last gui to edit the cache
-  // will be the one that make edit_cache uses.
-  if(!this->GetCacheDefinition("CMAKE_EDIT_COMMAND")
-    || !this->CMakeEditCommand.empty())
-    {
-    // Find and save the command to edit the cache
-    std::string editCacheCommand;
-    if(!this->CMakeEditCommand.empty())
-      {
-      editCacheCommand = this->CMakeEditCommand;
-      }
-    if(!cmSystemTools::FileExists(editCacheCommand.c_str()) &&
-       !cmSystemTools::GetCMakeCursesCommand().empty())
-      {
-      editCacheCommand = cmSystemTools::GetCMakeCursesCommand();
-      }
-    if(!cmSystemTools::FileExists(editCacheCommand.c_str()) &&
-       !cmSystemTools::GetCMakeGUICommand().empty())
-      {
-      editCacheCommand = cmSystemTools::GetCMakeGUICommand();
-      }
-    if(cmSystemTools::FileExists(editCacheCommand.c_str()))
-      {
-      this->CacheManager->AddCacheEntry
-        ("CMAKE_EDIT_COMMAND", editCacheCommand.c_str(),
-         "Path to cache edit program executable.", cmCacheManager::INTERNAL);
-      }
-    }
-
   if(!cmSystemTools::FileExists(
        (cmSystemTools::GetCMakeRoot()+"/Modules/CMake.cmake").c_str()))
     {

Source/cmake.h

@@ -335,9 +335,10 @@ class cmake
   std::vector<std::string> const& GetDebugConfigs();
 
   void SetCMakeEditCommand(std::string const& s)
-    {
+    { this->CMakeEditCommand = s; }
-      this->CMakeEditCommand = s;
+  std::string const& GetCMakeEditCommand() const
-    }
+    { return this->CMakeEditCommand; }
+
   void SetSuppressDevWarnings(bool v)
     {
       this->SuppressDevWarnings = v;