[Lldb-commits] [PATCH] D130219: [lldb][NFCI] Refactor regex filtering logic in CommandObjectTypeFormatterList

[Jorge Gorbe Moya via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa3a2e611d84: [lldb][NFCI] Refactor regex filtering logic in 
CommandObjectTypeFormatterList (authored by jgorbe).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130219

Files:
  lldb/source/Commands/CommandObjectType.cpp

Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -1055,6 +1055,15 @@
 return false;
   }
 
+  static bool ShouldListItem(llvm::StringRef s, RegularExpression *regex) {
+// If we have a regex, it can match two kinds of results:
+//   - An item created with that same regex string (exact string match), so
+// the user can list it using the same string it used at creation time.
+//   - Items that match the regex.
+// No regex means list everything.
+return regex == nullptr || s == regex->GetText() || regex->Execute(s);
+  }
+
   bool DoExecute(Args , CommandReturnObject ) override {
 const size_t argc = command.GetArgumentCount();
 
@@ -1096,24 +1105,13 @@
 .SetExact([, _regex, _printed](
   const TypeMatcher _matcher,
   const FormatterSharedPointer _sp) -> bool {
-  if (formatter_regex) {
-bool escape = true;
-if (type_matcher.CreatedBySameMatchString(
-ConstString(formatter_regex->GetText( {
-  escape = false;
-} else if (formatter_regex->Execute(
-   type_matcher.GetMatchString().GetStringRef())) {
-  escape = false;
-}
-
-if (escape)
-  return true;
+  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
+ formatter_regex.get())) {
+any_printed = true;
+result.GetOutputStream().Printf(
+"%s: %s\n", type_matcher.GetMatchString().GetCString(),
+format_sp->GetDescription().c_str());
   }
-
-  any_printed = true;
-  result.GetOutputStream().Printf(
-  "%s: %s\n", type_matcher.GetMatchString().GetCString(),
-  format_sp->GetDescription().c_str());
   return true;
 });
 
@@ -1121,24 +1119,13 @@
 .SetWithRegex([, _regex, _printed](
   const TypeMatcher _matcher,
   const FormatterSharedPointer _sp) -> bool {
-  if (formatter_regex) {
-bool escape = true;
-if (type_matcher.CreatedBySameMatchString(
-ConstString(formatter_regex->GetText( {
-  escape = false;
-} else if (formatter_regex->Execute(
-   type_matcher.GetMatchString().GetStringRef())) {
-  escape = false;
-}
-
-if (escape)
-  return true;
+  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
+ formatter_regex.get())) {
+any_printed = true;
+result.GetOutputStream().Printf(
+"%s: %s\n", type_matcher.GetMatchString().GetCString(),
+format_sp->GetDescription().c_str());
   }
-
-  any_printed = true;
-  result.GetOutputStream().Printf(
-  "%s: %s\n", type_matcher.GetMatchString().GetCString(),
-  format_sp->GetDescription().c_str());
   return true;
 });
 
@@ -1155,20 +1142,9 @@
   DataVisualization::Categories::ForEach(
   [_regex, _closure](
   const lldb::TypeCategoryImplSP ) -> bool {
-if (category_regex) {
-  bool escape = true;
-  if (category->GetName() == category_regex->GetText()) {
-escape = false;
-  } else if (category_regex->Execute(category->GetName())) {
-escape = false;
-  }
-
-  if (escape)
-return true;
+if (ShouldListItem(category->GetName(), category_regex.get())) {
+  category_closure(category);
 }
-
-category_closure(category);
-
 return true;
   });
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130219: [lldb][NFCI] Refactor regex filtering logic in CommandObjectTypeFormatterList

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe added inline comments.



Comment at: lldb/source/Commands/CommandObjectType.cpp:1064
+// No regex means list everything.
+return (regex == nullptr || (s == regex->GetText() || regex->Execute(s)));
+  }

labath wrote:
> What's up with all the parentheses? This is just a regular `||` between three 
> choices...
Thanks for catching it. Fixed.


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

https://reviews.llvm.org/D130219

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130219: [lldb][NFCI] Refactor regex filtering logic in CommandObjectTypeFormatterList

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe updated this revision to Diff 446532.
jgorbe added a comment.

Removed unnecessary parentheses.


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

https://reviews.llvm.org/D130219

Files:
  lldb/source/Commands/CommandObjectType.cpp

Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -1055,6 +1055,15 @@
 return false;
   }
 
+  static bool ShouldListItem(llvm::StringRef s, RegularExpression *regex) {
+// If we have a regex, it can match two kinds of results:
+//   - An item created with that same regex string (exact string match), so
+// the user can list it using the same string it used at creation time.
+//   - Items that match the regex.
+// No regex means list everything.
+return regex == nullptr || s == regex->GetText() || regex->Execute(s);
+  }
+
   bool DoExecute(Args , CommandReturnObject ) override {
 const size_t argc = command.GetArgumentCount();
 
@@ -1096,24 +1105,13 @@
 .SetExact([, _regex, _printed](
   const TypeMatcher _matcher,
   const FormatterSharedPointer _sp) -> bool {
-  if (formatter_regex) {
-bool escape = true;
-if (type_matcher.CreatedBySameMatchString(
-ConstString(formatter_regex->GetText( {
-  escape = false;
-} else if (formatter_regex->Execute(
-   type_matcher.GetMatchString().GetStringRef())) {
-  escape = false;
-}
-
-if (escape)
-  return true;
+  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
+ formatter_regex.get())) {
+any_printed = true;
+result.GetOutputStream().Printf(
+"%s: %s\n", type_matcher.GetMatchString().GetCString(),
+format_sp->GetDescription().c_str());
   }
-
-  any_printed = true;
-  result.GetOutputStream().Printf(
-  "%s: %s\n", type_matcher.GetMatchString().GetCString(),
-  format_sp->GetDescription().c_str());
   return true;
 });
 
@@ -1121,24 +1119,13 @@
 .SetWithRegex([, _regex, _printed](
   const TypeMatcher _matcher,
   const FormatterSharedPointer _sp) -> bool {
-  if (formatter_regex) {
-bool escape = true;
-if (type_matcher.CreatedBySameMatchString(
-ConstString(formatter_regex->GetText( {
-  escape = false;
-} else if (formatter_regex->Execute(
-   type_matcher.GetMatchString().GetStringRef())) {
-  escape = false;
-}
-
-if (escape)
-  return true;
+  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
+ formatter_regex.get())) {
+any_printed = true;
+result.GetOutputStream().Printf(
+"%s: %s\n", type_matcher.GetMatchString().GetCString(),
+format_sp->GetDescription().c_str());
   }
-
-  any_printed = true;
-  result.GetOutputStream().Printf(
-  "%s: %s\n", type_matcher.GetMatchString().GetCString(),
-  format_sp->GetDescription().c_str());
   return true;
 });
 
@@ -1155,20 +1142,9 @@
   DataVisualization::Categories::ForEach(
   [_regex, _closure](
   const lldb::TypeCategoryImplSP ) -> bool {
-if (category_regex) {
-  bool escape = true;
-  if (category->GetName() == category_regex->GetText()) {
-escape = false;
-  } else if (category_regex->Execute(category->GetName())) {
-escape = false;
-  }
-
-  if (escape)
-return true;
+if (ShouldListItem(category->GetName(), category_regex.get())) {
+  category_closure(category);
 }
-
-category_closure(category);
-
 return true;
   });
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130219: [lldb][NFCI] Refactor regex filtering logic in CommandObjectTypeFormatterList

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Commands/CommandObjectType.cpp:1064
+// No regex means list everything.
+return (regex == nullptr || (s == regex->GetText() || regex->Execute(s)));
+  }

What's up with all the parentheses? This is just a regular `||` between three 
choices...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130219

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130219: [lldb][NFCI] Refactor regex filtering logic in CommandObjectTypeFormatterList

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe created this revision.
jgorbe added a reviewer: labath.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.

Extract a bit of copy/pasted regex filtering logic into a separate function and 
simplify it a little bit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130219

Files:
  lldb/source/Commands/CommandObjectType.cpp

Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -1055,6 +1055,15 @@
 return false;
   }
 
+  static bool ShouldListItem(llvm::StringRef s, RegularExpression *regex) {
+// If we have a regex, it can match two kinds of results:
+//   - An item created with that same regex string (exact string match), so
+// the user can list it using the same string it used at creation time.
+//   - Items that match the regex.
+// No regex means list everything.
+return (regex == nullptr || (s == regex->GetText() || regex->Execute(s)));
+  }
+
   bool DoExecute(Args , CommandReturnObject ) override {
 const size_t argc = command.GetArgumentCount();
 
@@ -1096,24 +1105,13 @@
 .SetExact([, _regex, _printed](
   const TypeMatcher _matcher,
   const FormatterSharedPointer _sp) -> bool {
-  if (formatter_regex) {
-bool escape = true;
-if (type_matcher.CreatedBySameMatchString(
-ConstString(formatter_regex->GetText( {
-  escape = false;
-} else if (formatter_regex->Execute(
-   type_matcher.GetMatchString().GetStringRef())) {
-  escape = false;
-}
-
-if (escape)
-  return true;
+  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
+ formatter_regex.get())) {
+any_printed = true;
+result.GetOutputStream().Printf(
+"%s: %s\n", type_matcher.GetMatchString().GetCString(),
+format_sp->GetDescription().c_str());
   }
-
-  any_printed = true;
-  result.GetOutputStream().Printf(
-  "%s: %s\n", type_matcher.GetMatchString().GetCString(),
-  format_sp->GetDescription().c_str());
   return true;
 });
 
@@ -1121,24 +1119,13 @@
 .SetWithRegex([, _regex, _printed](
   const TypeMatcher _matcher,
   const FormatterSharedPointer _sp) -> bool {
-  if (formatter_regex) {
-bool escape = true;
-if (type_matcher.CreatedBySameMatchString(
-ConstString(formatter_regex->GetText( {
-  escape = false;
-} else if (formatter_regex->Execute(
-   type_matcher.GetMatchString().GetStringRef())) {
-  escape = false;
-}
-
-if (escape)
-  return true;
+  if (ShouldListItem(type_matcher.GetMatchString().GetStringRef(),
+ formatter_regex.get())) {
+any_printed = true;
+result.GetOutputStream().Printf(
+"%s: %s\n", type_matcher.GetMatchString().GetCString(),
+format_sp->GetDescription().c_str());
   }
-
-  any_printed = true;
-  result.GetOutputStream().Printf(
-  "%s: %s\n", type_matcher.GetMatchString().GetCString(),
-  format_sp->GetDescription().c_str());
   return true;
 });
 
@@ -1155,20 +1142,9 @@
   DataVisualization::Categories::ForEach(
   [_regex, _closure](
   const lldb::TypeCategoryImplSP ) -> bool {
-if (category_regex) {
-  bool escape = true;
-  if (category->GetName() == category_regex->GetText()) {
-escape = false;
-  } else if (category_regex->Execute(category->GetName())) {
-escape = false;
-  }
-
-  if (escape)
-return true;
+if (ShouldListItem(category->GetName(), category_regex.get())) {
+  category_closure(category);
 }
-
-category_closure(category);
-
 return true;
   });
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits