Skip to content
This repository has been archived by the owner on Jan 19, 2024. It is now read-only.

Replace mustache and fmt::format with std::format. #43

Closed
ScottBailey opened this issue Apr 20, 2023 · 7 comments
Closed

Replace mustache and fmt::format with std::format. #43

ScottBailey opened this issue Apr 20, 2023 · 7 comments
Assignees

Comments

@ScottBailey
Copy link
Contributor

Mustache fails to test at compile time and could easily be replaced with std::format. Additionally, we are using fmt::format which can be replaced with std::format.

Moving to std::format can only be resolved when we switch to a C++20 minimum compiler spec.

If we'd like to remove Mustache before C++20, we can use fmt::format inseat od std::format and write and additional issue to update to std::format later.

@ScottBailey
Copy link
Contributor Author

One option is to make the format code look like this:

constexpr std::string_view add_contract_template = {
   "file(GLOB {}-source ${CMAKE_CURRENT_SOURCE_DIR}/../../../apps/{}/*{})\n" // target name, object name, source extension                                
   "add_contract({} {} ${{}-source})\n\n"};                                  // object name, target name, target name                                     

constexpr std::string_view add_library_template = {
   "file(GLOB {}-source ${CMAKE_CURRENT_SOURCE_DIR}/../../../libs/{}/*{})\n\n" // target name, object name, source extension                              
   "add_library({} ${{}-source})\n\n"};                                        // target name, target name                     

Another is to make it look like something like this:

constexpr std::string set_vals = {
   "set(APJ_TARGET_NAME \"{}\")\n"
   "set(APJ_OBJECT_NAME \"{}\")\n"
   "set(APJ_EXT \"{}\")\n"};

constexpr std::string_view add_contract_template = {
   "file(GLOB ${APJ_TARGET_NAME}-source ${CMAKE_CURRENT_SOURCE_DIR}/../../../apps/${APJ_OBJECT_NAME}/*${APJ_EXT})\n"
   "add_contract(${APJ_OBJECT_NAME} ${APJ_TARGET_NAME} ${${APJ_TARGET_NAME}-source})\n\n"};

constexpr std::string_view add_library_template = {
   "file(GLOB ${APJ_TARGET_NAME}-source ${CMAKE_CURRENT_SOURCE_DIR}/../../../libs/${APJ_OBJECT_NAME}/*${APJ_EXT})\n\n"
   "add_library(${APJ_TARGET_NAME} ${${APJ_TARGET_NAME}-source})\n\n"};	

@larryk85
Copy link
Contributor

While I agree that lack of it not verifying during compile time is a nice to have. It seems like much more boiler plate and repetition than just leaving mustache.

It is not clear to me as to what the big value add is with this.

@ScottBailey
Copy link
Contributor Author

@larryk85 There are two additional issues beyond compile time checking making std::format preferable to Mustache.

The first is that Mustache is a non-standard library that essentially duplicates the functionality of a standard one.

The second issue is it's lack of escape mechanism for braces in the source requiring some workaround for use with cmake files.

Given that we are committed to standard libraries and are already using format, it seems like a no brainer to switch away from Mustache.

I wouldn't bet money on it, but I think the number of lines of code (number of characters even) should be about the same between the two solutions; however, the format code should be more familiar to most of us. Of course, removing the Mustache repo reduces the number of lines of code.

I think this is a positive maintenance improvement.

@ScottBailey
Copy link
Contributor Author

In fact, here's some sample code, untested but you get the idea.

            // as is with workaround                                                                                                             
            s << temp.render(datum{"obj_name", obj.name()}
                                  ("target_name", target_name(obj))
                                  ("source_ext", system::extension(obj.language()))
                                  ("dollar_brace", "${")); // dollar_brace must be "${" to work around Mustache issue                            

            // first example above                                                                                                               
            std::format_to(std::ostream_iterator<char>(s), temp,
                           target_name(obj), obj.name(), system::extension(obj.language()),
                           obj.name(), target_name(obj), target_name(obj));

            // second example above                                                                                                              
            std::format_to(std::ostream_iterator<char>(s), set_vals, target_name(obj), obj.name(), system::extension(obj.language()));
            s << temp;

@ScottBailey
Copy link
Contributor Author

ScottBailey commented Apr 24, 2023

Here's the proposed code (#41):

            km::mustache cmake::add_contract_template = {
               "file(GLOB {{target_name}}-source ${CMAKE_CURRENT_SOURCE_DIR}/../../../apps/{{obj_name}}/*{{source_ext}})\n"
               "add_contract({{obj_name}} {{target_name}} {{dollar_brace}}{{target_name}}-source})\n\n"};

            km::mustache cmake::add_library_template = {
               "file(GLOB {{target_name}}-source ${CMAKE_CURRENT_SOURCE_DIR}/../../../libs/{{obj_name}}/*{{source_ext}})\n\n"
               "add_library({{target_name}} {{dollar_brace}}{{target_name}}-source})\n\n"};


            s << temp.render(datum{"obj_name", obj.name()}
                                  ("target_name", target_name(obj))
                                  ("source_ext", system::extension(obj.language()))
                                  ("dollar_brace", "${")); // dollar_brace must be "${" to work around Mustache issue                            

Here's the same code with fmt::format or std::format:

           constexpr std::string_view add_contract_template = {      // 0: object name, 1: target name, 2: source extension                     
               "file(GLOB {1}-source ${CMAKE_CURRENT_SOURCE_DIR}/../../../apps/{0}/*{2})\n"
               "add_contract({0} {1} ${{1}-source})\n\n"};

            constexpr std::string_view add_library_template = {       // 0: object name, 1: target name, 2: source extension                     
               "file(GLOB {1}-source ${CMAKE_CURRENT_SOURCE_DIR}/../../../libs/{0}/*{2})\n\n"
               "add_library({0} ${{1}-source})\n\n"};


            std::format_to(std::ostream_iterator<char>(s), temp,
                  obj.name(),
                  target_name(obj),
                  system::extension(obj.language()));

Really not that much different.

@larryk85
Copy link
Contributor

@larryk85 There are two additional issues beyond compile time checking making std::format preferable to Mustache.

The first is that Mustache is a non-standard library that essentially duplicates the functionality of a standard one.

The second issue is it's lack of escape mechanism for braces in the source requiring some workaround for use with cmake files.

Given that we are committed to standard libraries and are already using format, it seems like a no brainer to switch away from Mustache.

I wouldn't bet money on it, but I think the number of lines of code (number of characters even) should be about the same between the two solutions; however, the format code should be more familiar to most of us. Of course, removing the Mustache repo reduces the number of lines of code.

I think this is a positive maintenance improvement.

I'm not sure what you mean by "mustache is not standard", mustache is a standard for templating. https://github.com/mustache/spec/tree/master/specs.

I'm fine with the removal, but I still don't see the big advantage. Mustache also has far more functionality than fmt. Albeit, we aren't currently using that functionality, it's not a 1-1 replacement per se.

@ScottBailey
Copy link
Contributor Author

Re standard: https://isocpp.org/

I agree, this is not a 1:1 replacement. And the advantage is minor: one less dependency and one less hackey workaround. (Unless there was an escape mechanism I didn't see. But since no one raised their hand to point to it...)

This was closed in #41.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants