Merge pull request #82 from glynos/uri_build_with_query

Tried to address awkward API issue when append query parameters.

Author: deanberris

Diff for: .travis.yml

@@ -31,7 +31,7 @@ script:
     - mkdir _builds
     - cd _builds
     # Note: clang not support libc++
-    - cmake -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=${BUILD_SHARED_LIBS} -DCPP-NETLIB_DISABLE_LIBCXX=YES ..
+    - cmake -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} -DBUILD_SHARED_LIBS=${BUILD_SHARED_LIBS} -DUri_DISABLE_LIBCXX=YES -DUri_BUILD_TESTS=ON -DUri_BUILD_DOCS=OFF ..
     - make -j 8
     - make test
 

Diff for: CMakeLists.txt

@@ -7,6 +7,11 @@
 cmake_minimum_required(VERSION 2.8)
 project(Uri)
 
+option(Uri_BUILD_TESTS "Build the URI tests." ON)
+option(Uri_BUILD_DOCS "Build the URI documentation." ON)
+option(Uri_FULL_WARNINGS "Build the library with all warnings turned on." ON)
+option(Uri_WARNINGS_AS_ERRORS "Treat warnings as errors." ON)
+
 find_package(Threads REQUIRED)
 
 set(CMAKE_VERBOSE_MAKEFILE true)
@@ -20,51 +25,68 @@ if (${CMAKE_CXX_COMPILER_ID} MATCHES GNU)
   else()
     message(FATAL_ERROR "No C++ 11 support (Compiler does not define -std=c++11).")
   endif()
+
+  if (Uri_FULL_WARNINGS)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall")
+  endif()
+
+  if (Uri_WARNINGS_AS_ERRORS)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
+  endif()
+
   message("C++ Flags: ${CMAKE_CXX_FLAGS} link flags: ${CMAKE_CXX_LINK_FLAGS}")
 elseif(${CMAKE_CXX_COMPILER_ID} MATCHES Clang)
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -ftemplate-depth=1024")
-  if (NOT CPP-NETLIB_DISABLE_LIBCXX)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
+
+  if (NOT Uri_DISABLE_LIBCXX)
     set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++")
   endif()
+
+  if (Uri_FULL_WARNINGS)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall")
+  endif()
+
+  if (Uri_WARNINGS_AS_ERRORS)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
+  endif()
+
   message("C++ Flags: ${CMAKE_CXX_FLAGS} link flags: ${CMAKE_CXX_LINK_FLAGS}")
 endif()
 
 
 if (MSVC)
-  add_definitions(-D_SCL_SECURE_NO_WARNINGS -D_VARIADIC_MAX=10)
-endif(MSVC)
-if (WIN32)
-  add_definitions(-D_WIN32_WINNT=0x0501)
-endif(WIN32)
-
-foreach(flag_var
-        CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE
-        CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO)
-   if(${flag_var} MATCHES "/MD")
+  message(FATAL_ERROR "The current implementation uses features that aren't fully implemented on MSVC.")
+
+  foreach(flag_var
+      CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE
+      CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO)
+    if(${flag_var} MATCHES "/MD")
       string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}")
-   endif(${flag_var} MATCHES "/MD")
-endforeach(flag_var)
+    endif(${flag_var} MATCHES "/MD")
+  endforeach(flag_var)
+endif(MSVC)
 
 include_directories(${Uri_SOURCE_DIR}/src ${Uri_SOURCE_DIR}/include)
 
 add_subdirectory(src)
 
 # Testing
-if (CMAKE_PROJECT_NAME STREQUAL "Uri" OR CPP-NETLIB_BUILD_TESTS)
+if (Uri_BUILD_TESTS)
+  message(STATUS "Configuring tests")
   enable_testing()
   add_subdirectory(deps/googletest)
   add_subdirectory(test)
 endif()
 
 # Documentation
-find_package(Doxygen)
-if (DOXYGEN_FOUND)
-  if (NOT DEFINED CPP-NETLIB_GENERATE_DOCS)
+if (Uri_BUILD_DOCS)
+  message("Configuring documentation")
+  find_package(Doxygen)
+  if (DOXYGEN_FOUND)
     configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Doxyfile.in ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile @ONLY)
     add_custom_target(doc
       ${DOXYGEN_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/Doxyfile
       WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
       COMMENT "Generating API documentation with Doxygen" VERBATIM)
-    set(CPP-NETLIB_GENERATE_DOCS ON)
   endif()
-endif(DOXYGEN_FOUND)
+endif()

Diff for: include/network/string_view.hpp

@@ -152,6 +152,7 @@ class basic_string_view {
   size_type copy(charT* s, size_type n, size_type pos = 0) const {
     size_type rlen = std::min(n, size() - pos);
     std::copy_n(begin() + pos, rlen, s);
+    return rlen;
   }
 
   constexpr basic_string_view substr(size_type pos = 0,

Diff for: include/network/uri/uri_builder.hpp

@@ -187,8 +187,8 @@ class uri_builder {
    * \returns \c *this
    */
   template <typename Source>
-  uri_builder &query(const Source &query) {
-    set_query(detail::translate(query));
+  uri_builder &append_query(const Source &query) {
+    append_query(detail::translate(query));
     return *this;
   }
 
@@ -205,7 +205,7 @@ class uri_builder {
    * \returns \c *this
    */
   template <typename Key, typename Value>
-    uri_builder &query(const Key &key, const Value &value) {
+    uri_builder &append_query_key_value_pair(const Key &key, const Value &value) {
     if (!query_) {
       query_ = string_type();
     }
@@ -252,7 +252,7 @@ class uri_builder {
   void set_port(string_type port);
   void set_authority(string_type authority);
   void set_path(string_type path);
-  void set_query(string_type query);
+  void append_query(string_type query);
   void set_fragment(string_type fragment);
 
   optional<string_type> scheme_, user_info_, host_, port_, path_, query_,

Diff for: src/CMakeLists.txt

@@ -17,7 +17,7 @@ set(Uri_SRCS
 add_library(network-uri ${Uri_SRCS})
 target_link_libraries(network-uri)
 if(${CMAKE_CXX_COMPILER_ID} MATCHES Clang)
-  if (NOT CPP-NETLIB_DISABLE_LIBCXX)
+  if (NOT Uri_DISABLE_LIBCXX)
     target_link_libraries(network-uri "c++")
   endif()
 endif()

Diff for: src/detail/uri_advance_parts.cpp

@@ -26,7 +26,7 @@ uri_part copy_part(const std::string &uri, string_view::const_iterator &it) {
 
 void advance_parts(string_view &uri_view, uri_parts &parts,
                    const uri_parts &existing_parts) {
-  auto first = std::begin(uri_view), last = std::end(uri_view);
+  auto first = std::begin(uri_view);
 
   auto it = first;
   if (auto scheme = existing_parts.scheme) {

Diff for: src/detail/uri_percent_encode.hpp

@@ -59,11 +59,6 @@ Iter decode_encoded_unreserved_chars(Iter first, Iter last) {
   while (it != last) {
     if (*it == '%') {
       const auto sfirst = it;
-      const auto slast = [&]() {
-        auto slast = it;
-        std::advance(slast, 3);
-        return slast;
-      }();
       const auto opt_char = percent_encode(sfirst);
       if (opt_char && is_unreserved(*opt_char)) {
         *it2 = *opt_char;

Diff for: src/uri.cpp

@@ -57,6 +57,9 @@ inline optional<std::string> make_arg(optional<string_view> view) {
   }
   return nullopt;
 }
+
+template <class T>
+inline void ignore(T) {}
 }  // namespace
 
 void uri::initialize(optional<string_type> scheme,
@@ -374,6 +377,7 @@ uri uri::normalize(uri_comparison_level level) const {
     // need to parse the parts again as the underlying string has changed
     const_iterator it = std::begin(normalized_view), last = std::end(normalized_view);
     bool is_valid = detail::parse(it, last, parts);
+    ignore(is_valid);
     assert(is_valid);
 
     if (parts.hier_part.path) {
@@ -448,7 +452,7 @@ uri uri::make_relative(const uri &other) const {
   result.initialize(optional<string_type>(), optional<string_type>(),
                     optional<string_type>(), optional<string_type>(),
                     other_path, query, fragment);
-  return std::move(result);
+  return result;
 }
 
 uri uri::resolve(const uri &base) const {

Diff for: src/uri_builder.cpp

@@ -32,7 +32,7 @@ uri_builder::uri_builder(const network::uri &base_uri) {
   }
 
   if (base_uri.has_query()) {
-    set_query(base_uri.query().to_string());
+    append_query(base_uri.query().to_string());
   }
 
   if (base_uri.has_fragment()) {
@@ -64,8 +64,8 @@ uri_builder &uri_builder::clear_user_info() {
 
 void uri_builder::set_host(string_type host) {
   host_ = string_type();
-  auto end = network::uri::encode_host(std::begin(host), std::end(host),
-                                       std::back_inserter(*host_));
+  network::uri::encode_host(std::begin(host), std::end(host),
+                            std::back_inserter(*host_));
   detail::transform(*host_, std::begin(*host_),
                     [](char ch) { return std::tolower(ch, std::locale()); });
 }
@@ -111,8 +111,13 @@ uri_builder &uri_builder::clear_path() {
   return *this;
 }
 
-void uri_builder::set_query(string_type query) {
-  query_ = string_type();
+void uri_builder::append_query(string_type query) {
+  if (!query_) {
+    query_ = string_type();
+  }
+  else {
+    query_->append("&");
+  }
   network::uri::encode_query(std::begin(query), std::end(query),
                              std::back_inserter(*query_));
 }

Diff for: test/optional_test.cpp

@@ -42,7 +42,7 @@ TEST(optional_test, value_constructor_string) {
 
 TEST(optional_test, rvalue_ref_constructor) {
   int value = 42;
-  network::optional<int> opt{std::move(42)};
+  network::optional<int> opt{std::move(value)};
   ASSERT_TRUE(opt);
   ASSERT_EQ(*opt, 42);
 }