Re: [cmake-developers] PATCH: add subcommand string(APPEND)
On 07/06/2015 04:47 PM, Daniel Pfeifer wrote: > OK, now with tests and release notes. Applied, thanks: string: add APPEND subcommand http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=2b18cdca > There is a debatable edge case: > When a variable is not-set and zero elements are appended, do we > expect the result to be a) not-set or b) an empty string? > My current implementation considers appending zero elements a no-op, > i.e. it follows approach a). The list(APPEND) command is a no-op when nothing is given, so this would be consistent with that. -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] PATCH: add subcommand string(APPEND)
On Mon, Jul 6, 2015 at 10:55 PM, James Bigler wrote: > list(APPEND) requires at least one element argument, right? No, see https://github.com/Kitware/CMake/blob/master/Source/cmListCommand.cxx#L236 -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] PATCH: add subcommand string(APPEND)
list(APPEND) requires at least one element argument, right?
Can you require the same thing for string(APPEND)? That would make it
symmetric and remove your edge case.
On Mon, Jul 6, 2015 at 2:47 PM, Daniel Pfeifer
wrote:
> On Mon, Jul 6, 2015 at 8:41 PM, Brad King wrote:
> > On 07/04/2015 06:27 PM, Daniel Pfeifer wrote:
> >> Attached is a patch that adds a subcommand string(APPEND).
> >> This allows to write
> >>
> >>> string(APPEND string_variable "some string")
> >>
> >> instead of
> >>
> >>> set(string_variable "${string_variable}some string")
> >
> > Thanks. Please extend the first patch to also add explicit coverage
> > of the feature in the test suite, perhaps in Tests/RunCMake/string
> > similar to the Concat test case. I'd prefer to get the implementation,
> > documentation, and tests of the new command integrated and working
> > before considering use of the command everywhere else.
>
> OK, now with tests and release notes.
>
> There is a debatable edge case:
> When a variable is not-set and zero elements are appended, do we
> expect the result to be a) not-set or b) an empty string?
> My current implementation considers appending zero elements a no-op,
> i.e. it follows approach a).
>
> cheers, Daniel
>
> --
>
> Powered by www.kitware.com
>
> Please keep messages on-topic and check the CMake FAQ at:
> http://www.cmake.org/Wiki/CMake_FAQ
>
> Kitware offers various services to support the CMake community. For more
> information on each offering, please visit:
>
> CMake Support: http://cmake.org/cmake/help/support.html
> CMake Consulting: http://cmake.org/cmake/help/consulting.html
> CMake Training Courses: http://cmake.org/cmake/help/training.html
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/mailman/listinfo/cmake-developers
>
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] PATCH: add subcommand string(APPEND)
On Mon, Jul 6, 2015 at 8:41 PM, Brad King wrote:
> On 07/04/2015 06:27 PM, Daniel Pfeifer wrote:
>> Attached is a patch that adds a subcommand string(APPEND).
>> This allows to write
>>
>>> string(APPEND string_variable "some string")
>>
>> instead of
>>
>>> set(string_variable "${string_variable}some string")
>
> Thanks. Please extend the first patch to also add explicit coverage
> of the feature in the test suite, perhaps in Tests/RunCMake/string
> similar to the Concat test case. I'd prefer to get the implementation,
> documentation, and tests of the new command integrated and working
> before considering use of the command everywhere else.
OK, now with tests and release notes.
There is a debatable edge case:
When a variable is not-set and zero elements are appended, do we
expect the result to be a) not-set or b) an empty string?
My current implementation considers appending zero elements a no-op,
i.e. it follows approach a).
cheers, Daniel
From f257b6098443295d9874ddc02df298ecd70af34b Mon Sep 17 00:00:00 2001
From: Daniel Pfeifer
Date: Mon, 6 Jul 2015 22:28:04 +0200
Subject: [PATCH] add subcommand string(APPEND)
---
Help/command/string.rst | 3 ++
Help/release/dev/string-append.rst| 4 ++
Source/cmStringCommand.cxx| 32 +++
Source/cmStringCommand.h | 1 +
Tests/RunCMake/string/Append.cmake| 58 +++
Tests/RunCMake/string/AppendNoArgs-result.txt | 1 +
Tests/RunCMake/string/AppendNoArgs-stderr.txt | 4 ++
Tests/RunCMake/string/AppendNoArgs.cmake | 1 +
Tests/RunCMake/string/RunCMakeTest.cmake | 3 ++
9 files changed, 107 insertions(+)
create mode 100644 Help/release/dev/string-append.rst
create mode 100644 Tests/RunCMake/string/Append.cmake
create mode 100644 Tests/RunCMake/string/AppendNoArgs-result.txt
create mode 100644 Tests/RunCMake/string/AppendNoArgs-stderr.txt
create mode 100644 Tests/RunCMake/string/AppendNoArgs.cmake
diff --git a/Help/command/string.rst b/Help/command/string.rst
index 34c1b61..bc4399c 100644
--- a/Help/command/string.rst
+++ b/Help/command/string.rst
@@ -15,6 +15,7 @@ String operations.
string(REPLACE
[...])
+ string(APPEND [...])
string(CONCAT [...])
string(
)
@@ -55,6 +56,8 @@ through argument parsing.
``REPLACE`` will replace all occurrences of ``match_string`` in the input
with ``replace_string`` and store the result in the output.
+``APPEND`` will append all the input arguments to the string.
+
``CONCAT`` will concatenate all the input arguments together and store
the result in the named output variable.
diff --git a/Help/release/dev/string-append.rst b/Help/release/dev/string-append.rst
new file mode 100644
index 000..190b51e
--- /dev/null
+++ b/Help/release/dev/string-append.rst
@@ -0,0 +1,4 @@
+string-append
+-
+
+* The :command:`string` command learned a new ``APPEND`` subcommand.
diff --git a/Source/cmStringCommand.cxx b/Source/cmStringCommand.cxx
index edc6afc..efc1f16 100644
--- a/Source/cmStringCommand.cxx
+++ b/Source/cmStringCommand.cxx
@@ -74,6 +74,10 @@ bool cmStringCommand
{
return this->HandleLengthCommand(args);
}
+ else if(subCommand == "APPEND")
+{
+return this->HandleAppendCommand(args);
+}
else if(subCommand == "CONCAT")
{
return this->HandleConcatCommand(args);
@@ -730,6 +734,34 @@ bool cmStringCommand
}
//
+bool cmStringCommand::HandleAppendCommand(std::vector const& args)
+{
+ if(args.size() < 2)
+{
+this->SetError("sub-command APPEND requires at least one argument.");
+return false;
+}
+
+ // Skip if nothing to append.
+ if(args.size() < 3)
+{
+return true;
+}
+
+ const std::string& variable = args[1];
+
+ std::string value;
+ const char* oldValue = this->Makefile->GetDefinition(variable);
+ if(oldValue)
+{
+value = oldValue;
+}
+ value += cmJoin(cmRange(args).advance(2), std::string());
+ this->Makefile->AddDefinition(variable, value.c_str());
+ return true;
+}
+
+//
bool cmStringCommand
::HandleConcatCommand(std::vector const& args)
{
diff --git a/Source/cmStringCommand.h b/Source/cmStringCommand.h
index 9c75095..3ed17eb 100644
--- a/Source/cmStringCommand.h
+++ b/Source/cmStringCommand.h
@@ -67,6 +67,7 @@ protected:
bool HandleReplaceCommand(std::vector const& args);
bool HandleLengthCommand(std::vector const& args);
bool HandleSubstringCommand(std::vector const& args);
+ bool HandleAppendCommand(std::vector const& args);
bool HandleConcatCommand(std::vector const& args);
bool HandleStripCommand(std::vector const& args);
bool HandleRandomCommand(std::vector const& args);
diff --git a/Tests/RunCMake/string/Append.cmake b/Tests/RunCMake/string/Append.cmake
new fil
Re: [cmake-developers] PATCH: add subcommand string(APPEND)
On 07/04/2015 06:27 PM, Daniel Pfeifer wrote:
> Attached is a patch that adds a subcommand string(APPEND).
> This allows to write
>
>> string(APPEND string_variable "some string")
>
> instead of
>
>> set(string_variable "${string_variable}some string")
Thanks. Please extend the first patch to also add explicit coverage
of the feature in the test suite, perhaps in Tests/RunCMake/string
similar to the Concat test case. I'd prefer to get the implementation,
documentation, and tests of the new command integrated and working
before considering use of the command everywhere else.
> Two other patches make use of this subcommand. The changes have been
> created with
>
>> find Modules -type f -print0 | xargs -0 perl -i -0pe \
>> 's/set\(([a-zA-Z0-9_]+)(\s+)"\$\{\1\}([^"])/string(APPEND \1\2"\3/g'
Nice. I'm also glad to see those instructions in the commit message.
This will make it easy to create these changes later after the feature
itself is worked out as above.
Thanks,
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
