[Lldb-commits] [PATCH] D112314: [lldb] [Utility/UriParser] Return results as 'struct URI'

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

nice



================
Comment at: lldb/include/lldb/Utility/UriParser.h:31-34
 class UriParser {
 public:
-  // Parses
-  // RETURN VALUE
-  //   if url is valid, function returns true and
-  //   scheme/hostname/port/path are set to the parsed values
-  //   port it set to llvm::None if it is not included in the URL
-  //
-  //   if the url is invalid, function returns false and
-  //   output parameters remain unchanged
-  static bool Parse(llvm::StringRef uri, llvm::StringRef &scheme,
-                    llvm::StringRef &hostname, llvm::Optional<uint16_t> &port,
-                    llvm::StringRef &path);
+  static llvm::Optional<URI> Parse(llvm::StringRef uri);
 };
----------------
As we're touching these anyway, let's also move the parse method into the URI class. Having a separate class for a single static method is (and was) useless.


================
Comment at: lldb/unittests/Utility/UriParserTest.cpp:6-16
 class UriTestCase {
 public:
   UriTestCase(const char *uri, const char *scheme, const char *hostname,
               llvm::Optional<uint16_t> port, const char *path)
-      : m_uri(uri), m_result(true), m_scheme(scheme), m_hostname(hostname),
-        m_port(port), m_path(path) {}
+      : m_uri(uri), m_result(URI{scheme, hostname, port, path}) {}
 
+  UriTestCase(const char *uri) : m_uri(uri), m_result(llvm::None) {}
----------------
I don't think this class makes sense anymore. You can just inline the values.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112314/new/

https://reviews.llvm.org/D112314