Merge topic 'define-property-optional-args'

edb5059216 define_property(): Make BRIEF_DOCS and FULL_DOCS optional
7d26baff46 cmDefinePropertyCommand: Refactor to use cmArgumentParser

Acked-by: Kitware Robot <kwrobot@kitware.com>
Tested-by: buildbot <buildbot@kitware.com>
Merge-request: !6875

8 files changed, 65 insertions, 46 deletions

diff --git a/Help/command/define_property.rst b/Help/command/define_property.rst
index 8f7439b..9474513 100644
--- a/Help/command/define_property.rst
+++ b/Help/command/define_property.rst
@@ -8,8 +8,8 @@ Define and document custom properties.
   define_property(<GLOBAL | DIRECTORY | TARGET | SOURCE |
                    TEST | VARIABLE | CACHED_VARIABLE>
                    PROPERTY <name> [INHERITED]
-                   BRIEF_DOCS <brief-doc> [docs...]
-                   FULL_DOCS <full-doc> [docs...])
+                   [BRIEF_DOCS <brief-doc> [docs...]]
+                   [FULL_DOCS <full-doc> [docs...]])
 
 Defines one property in a scope for use with the :command:`set_property` and
 :command:`get_property` commands.  This is primarily useful to associate
diff --git a/Help/release/dev/define-property-optional-args.rst b/Help/release/dev/define-property-optional-args.rst
new file mode 100644
index 0000000..b1cbf5e
--- /dev/null
+++ b/Help/release/dev/define-property-optional-args.rst
@@ -0,0 +1,5 @@
+define-property-optional-args
+-----------------------------
+
+* The :command:`define_property` ``BRIEF_DOCS`` and ``FULL_DOCS`` arguments are
+  now optional.
diff --git a/Source/cmDefinePropertyCommand.cxx b/Source/cmDefinePropertyCommand.cxx
index 4e2d9b0..10c36cd 100644
--- a/Source/cmDefinePropertyCommand.cxx
+++ b/Source/cmDefinePropertyCommand.cxx
@@ -2,9 +2,13 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmDefinePropertyCommand.h"
 
+#include <cmext/string_view>
+
+#include "cmArgumentParser.h"
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
 #include "cmProperty.h"
+#include "cmRange.h"
 #include "cmState.h"
 #include "cmStringAlgorithms.h"
 
@@ -44,37 +48,21 @@ bool cmDefinePropertyCommand(std::vector<std::string> const& args,
   // Parse remaining arguments.
   bool inherited = false;
   std::string PropertyName;
-  std::string BriefDocs;
-  std::string FullDocs;
-  enum Doing
-  {
-    DoingNone,
-    DoingProperty,
-    DoingBrief,
-    DoingFull
-  };
-  Doing doing = DoingNone;
-  for (unsigned int i = 1; i < args.size(); ++i) {
-    if (args[i] == "PROPERTY") {
-      doing = DoingProperty;
-    } else if (args[i] == "BRIEF_DOCS") {
-      doing = DoingBrief;
-    } else if (args[i] == "FULL_DOCS") {
-      doing = DoingFull;
-    } else if (args[i] == "INHERITED") {
-      doing = DoingNone;
-      inherited = true;
-    } else if (doing == DoingProperty) {
-      doing = DoingNone;
-      PropertyName = args[i];
-    } else if (doing == DoingBrief) {
-      BriefDocs += args[i];
-    } else if (doing == DoingFull) {
-      FullDocs += args[i];
-    } else {
-      status.SetError(cmStrCat("given invalid argument \"", args[i], "\"."));
-      return false;
-    }
+  std::vector<std::string> BriefDocs;
+  std::vector<std::string> FullDocs;
+
+  cmArgumentParser<void> parser;
+  parser.Bind("PROPERTY"_s, PropertyName);
+  parser.Bind("BRIEF_DOCS"_s, BriefDocs);
+  parser.Bind("FULL_DOCS"_s, FullDocs);
+  parser.Bind("INHERITED"_s, inherited);
+  std::vector<std::string> invalidArgs;
+
+  parser.Parse(cmMakeRange(args).advance(1), &invalidArgs);
+  if (!invalidArgs.empty()) {
+    status.SetError(
+      cmStrCat("given invalid argument \"", invalidArgs.front(), "\"."));
+    return false;
   }
 
   // Make sure a property name was found.
@@ -83,19 +71,10 @@ bool cmDefinePropertyCommand(std::vector<std::string> const& args,
     return false;
   }
 
-  // Make sure documentation was given.
-  if (BriefDocs.empty()) {
-    status.SetError("not given a BRIEF_DOCS <brief-doc> argument.");
-    return false;
-  }
-  if (FullDocs.empty()) {
-    status.SetError("not given a FULL_DOCS <full-doc> argument.");
-    return false;
-  }
-
   // Actually define the property.
   status.GetMakefile().GetState()->DefineProperty(
-    PropertyName, scope, BriefDocs, FullDocs, inherited);
+    PropertyName, scope, cmJoin(BriefDocs, ""), cmJoin(FullDocs, ""),
+    inherited);
 
   return true;
 }
diff --git a/Source/cmGetPropertyCommand.cxx b/Source/cmGetPropertyCommand.cxx
index b74ed48..4a25311 100644
--- a/Source/cmGetPropertyCommand.cxx
+++ b/Source/cmGetPropertyCommand.cxx
@@ -184,7 +184,8 @@ bool cmGetPropertyCommand(std::vector<std::string> const& args,
           status.GetMakefile().GetState()->GetPropertyDefinition(propertyName,
                                                                  scope)) {
       output = def->GetShortDescription();
-    } else {
+    }
+    if (output.empty()) {
       output = "NOTFOUND";
     }
     status.GetMakefile().AddDefinition(variable, output);
@@ -195,7 +196,8 @@ bool cmGetPropertyCommand(std::vector<std::string> const& args,
           status.GetMakefile().GetState()->GetPropertyDefinition(propertyName,
                                                                  scope)) {
       output = def->GetFullDescription();
-    } else {
+    }
+    if (output.empty()) {
       output = "NOTFOUND";
     }
     status.GetMakefile().AddDefinition(variable, output);
diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt
index f0457a8..dd786b8 100644
--- a/Tests/RunCMake/CMakeLists.txt
+++ b/Tests/RunCMake/CMakeLists.txt
@@ -418,6 +418,7 @@ add_RunCMake_test(ctest_update)
 add_RunCMake_test(ctest_upload)
 add_RunCMake_test(ctest_environment)
 add_RunCMake_test(ctest_fixtures)
+add_RunCMake_test(define_property)
 add_RunCMake_test(file -DCYGWIN=${CYGWIN} -DMSYS=${MSYS})
 add_RunCMake_test(file-CHMOD -DMSYS=${MSYS})
 add_RunCMake_test(file-RPATH -DCMAKE_SYSTEM_NAME=${CMAKE_SYSTEM_NAME})
diff --git a/Tests/RunCMake/define_property/CMakeLists.txt b/Tests/RunCMake/define_property/CMakeLists.txt
new file mode 100644
index 0000000..d8200fc
--- /dev/null
+++ b/Tests/RunCMake/define_property/CMakeLists.txt
@@ -0,0 +1,3 @@
+cmake_minimum_required(VERSION 3.22)
+project(${RunCMake_TEST} NONE)
+include(${RunCMake_TEST}.cmake)
diff --git a/Tests/RunCMake/define_property/RunCMakeTest.cmake b/Tests/RunCMake/define_property/RunCMakeTest.cmake
new file mode 100644
index 0000000..5cb581b
--- /dev/null
+++ b/Tests/RunCMake/define_property/RunCMakeTest.cmake
@@ -0,0 +1,3 @@
+include(RunCMake)
+
+run_cmake(define_property)
diff --git a/Tests/RunCMake/define_property/define_property.cmake b/Tests/RunCMake/define_property/define_property.cmake
new file mode 100644
index 0000000..d657538
--- /dev/null
+++ b/Tests/RunCMake/define_property/define_property.cmake
@@ -0,0 +1,26 @@
+function(assert_prop_scope_eq prop scope value)
+  get_property(actual_value TARGET NONE PROPERTY ${prop} ${scope})
+  if(NOT actual_value STREQUAL value)
+    message(SEND_ERROR "Expected value of ${prop}'s ${scope}:\n  ${value}\nActual value:\n  ${actual_value}")
+  endif()
+endfunction()
+
+define_property(TARGET PROPERTY PROP1)
+define_property(TARGET PROPERTY PROP2
+  BRIEF_DOCS "Brief")
+define_property(TARGET PROPERTY PROP3
+  FULL_DOCS "Full")
+define_property(TARGET PROPERTY PROP4
+  BRIEF_DOCS "Brief"
+  FULL_DOCS "Full")
+
+assert_prop_scope_eq(PROP0 BRIEF_DOCS "NOTFOUND")
+assert_prop_scope_eq(PROP0 FULL_DOCS "NOTFOUND")
+assert_prop_scope_eq(PROP1 BRIEF_DOCS "NOTFOUND")
+assert_prop_scope_eq(PROP1 FULL_DOCS "NOTFOUND")
+assert_prop_scope_eq(PROP2 BRIEF_DOCS "Brief")
+assert_prop_scope_eq(PROP2 FULL_DOCS "NOTFOUND")
+assert_prop_scope_eq(PROP3 BRIEF_DOCS "NOTFOUND")
+assert_prop_scope_eq(PROP3 FULL_DOCS "Full")
+assert_prop_scope_eq(PROP4 BRIEF_DOCS "Brief")
+assert_prop_scope_eq(PROP4 FULL_DOCS "Full")