[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread walter erquinigo via Phabricator via lldb-commits 
wallace abandoned this revision.
wallace added a comment.

Will revisit it whenever I need it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Jim Ingham via Phabricator via lldb-commits 
jingham added a comment.

In D77582#1967031 , @labath wrote:

> In D77582#1966962 , @teemperor wrote:
>
> > I think the part of the patch that adds documentation to the existing 
> > method should go in (In general all patches that just add documentation 
> > have my blanket approval, so please just commit those).
>
>
> About that... the existing practice has been to attach documentation to the 
> python versions of these interfaces (`bindings/interface/*.i`). Now we 
> started to add it to the c++ headers too. More documentation is always 
> better, but OTOH repetition creates maintenance problems and increases the 
> risk of it going stale. Should choose one of the two interfaces and put all 
> documentation there (and maybe have a way to auto-generate the other one) ?


We need to keep the documentation in the interface files, that's what produces 
the python and online doc help.  That's where the vast majority of users of the 
API will go to find help for it.  But the C++ docs do go into the C++ API docs 
on the website so they also have some (if not as much) value.  Having a way to 
salt one from the other would be ideal, though sometimes the Python API needs a 
little extra color so this isn't entirely trivial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

In D77582#1966962 , @teemperor wrote:

> I think the part of the patch that adds documentation to the existing method 
> should go in (In general all patches that just add documentation have my 
> blanket approval, so please just commit those).


About that... the existing practice has been to attach documentation to the 
python versions of these interfaces (`bindings/interface/*.i`). Now we started 
to add it to the c++ headers too. More documentation is always better, but OTOH 
repetition creates maintenance problems and increases the risk of it going 
stale. Should choose one of the two interfaces and put all documentation there 
(and maybe have a way to auto-generate the other one) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Raphael Isemann via Phabricator via lldb-commits 
teemperor added a comment.

I think the part of the patch that adds documentation to the existing method 
should go in (In general all patches that just add documentation have my 
blanket approval, so please just commit those).

For the additional function: If you just change that CommandExists also checks 
user commands, I think that seems reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Shafik Yaghmour via Phabricator via lldb-commits 
shafik added a comment.

I agree, if we don't have an immediate use then I don't think we should add it 
now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-07 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a reviewer: jingham.
labath added a comment.

I don't have a hard objection to this, but if we're don't have a use for it, I 
don't see a need for adding it right now.
Particularly as it sounds weird to me that "user commands" are not a subset of 
"commands". I know that is how the underlying api works, but the underlying api 
looks weird to me too. However, that one (unlike the SB version) can be changed 
whenever we want.

As for the test, the same remarks I made on the other review apply here too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582



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

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
wallace added reviewers: labath, clayborg.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.
wallace updated this revision to Diff 255454.
wallace added a comment.
wallace updated this revision to Diff 255455.

.


wallace added a comment.

Improve some names


I don't have an immediate use for this, but I had already implemented
it, so I better make a diff for this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77582

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestUserCommand.cpp

Index: lldb/unittests/Interpreter/TestUserCommand.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestUserCommand.cpp
@@ -0,0 +1,45 @@
+//===-- TestUserCommand.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#include "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject ) {
+result.PutCString("It works");
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+};
+
+TEST(TestUserCommand, TestSimpleCommand) {
+  SBDebugger::Initialize();
+  SBDebugger dbg = SBDebugger::Create(false /*source_init_files*/);
+  SBCommandInterpreter interp = dbg.GetCommandInterpreter();
+
+  DummyCommand command;
+  interp.AddCommand("dummy", , nullptr /*help*/);
+
+  EXPECT_TRUE(interp.UserCommandExists("dummy"));
+
+  SBCommandReturnObject result;
+  interp.HandleCommand("dummy", result, true /*add_to_history*/);
+  EXPECT_TRUE(result.Succeeded());
+  EXPECT_STREQ(result.GetOutput(), "It works\n");
+}
Index: lldb/unittests/Interpreter/CMakeLists.txt
===
--- lldb/unittests/Interpreter/CMakeLists.txt
+++ lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,8 +1,10 @@
 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestUserCommand.cpp
 
   LINK_LIBS
+liblldb
 lldbInterpreter
 lldbUtilityHelpers
   )
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
+

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 255455.
wallace added a comment.

Improve some names


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestUserCommand.cpp

Index: lldb/unittests/Interpreter/TestUserCommand.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestUserCommand.cpp
@@ -0,0 +1,45 @@
+//===-- TestUserCommand.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#include "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject ) {
+result.PutCString("It works");
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+};
+
+TEST(TestUserCommand, TestSimpleCommand) {
+  SBDebugger::Initialize();
+  SBDebugger dbg = SBDebugger::Create(false /*source_init_files*/);
+  SBCommandInterpreter interp = dbg.GetCommandInterpreter();
+
+  DummyCommand command;
+  interp.AddCommand("dummy", , nullptr /*help*/);
+
+  EXPECT_TRUE(interp.UserCommandExists("dummy"));
+
+  SBCommandReturnObject result;
+  interp.HandleCommand("dummy", result, true /*add_to_history*/);
+  EXPECT_TRUE(result.Succeeded());
+  EXPECT_STREQ(result.GetOutput(), "It works\n");
+}
Index: lldb/unittests/Interpreter/CMakeLists.txt
===
--- lldb/unittests/Interpreter/CMakeLists.txt
+++ lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,8 +1,10 @@
 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestUserCommand.cpp
 
   LINK_LIBS
+liblldb
 lldbInterpreter
 lldbUtilityHelpers
   )
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
+  bool UserCommandExists(const char *cmd);
+
   bool AliasExists(const char *cmd);
 
   lldb::SBBroadcaster GetBroadcaster();
Index: lldb/bindings/interface/SBCommandInterpreter.i
===
---

[Lldb-commits] [PATCH] D77582: [CommandInterpreter] Implement UserCommandExists

2020-04-06 Thread walter erquinigo via Phabricator via lldb-commits 
wallace updated this revision to Diff 255454.
wallace added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77582

Files:
  lldb/bindings/interface/SBCommandInterpreter.i
  lldb/include/lldb/API/SBCommandInterpreter.h
  lldb/source/API/SBCommandInterpreter.cpp
  lldb/unittests/Interpreter/CMakeLists.txt
  lldb/unittests/Interpreter/TestUserCommand.cpp

Index: lldb/unittests/Interpreter/TestUserCommand.cpp
===
--- /dev/null
+++ lldb/unittests/Interpreter/TestUserCommand.cpp
@@ -0,0 +1,45 @@
+//===-- TestUserCommand.cpp --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===/
+
+#include "gtest/gtest.h"
+
+#include "lldb/API/SBCommandInterpreter.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBDebugger.h"
+
+#include 
+#include 
+#include 
+
+using namespace lldb;
+
+class DummyCommand : public SBCommandPluginInterface {
+public:
+  bool DoExecute(SBDebugger dbg, char **command,
+ SBCommandReturnObject ) {
+result.PutCString("It works");
+result.SetStatus(eReturnStatusSuccessFinishResult);
+return result.Succeeded();
+  }
+};
+
+TEST(TestUserCommand, TestCreation) {
+  SBDebugger::Initialize();
+  SBDebugger dbg = SBDebugger::Create(false /*source_init_files*/);
+  SBCommandInterpreter interp = dbg.GetCommandInterpreter();
+
+  DummyCommand command;
+  interp.AddCommand("dummy", , nullptr /*help*/);
+
+  EXPECT_TRUE(interp.UserCommandExists("dummy"));
+
+  SBCommandReturnObject result;
+  interp.HandleCommand("dummy", result, true /*add_to_history*/);
+  EXPECT_TRUE(result.Succeeded());
+  EXPECT_STREQ(result.GetOutput(), "It works\n");
+}
Index: lldb/unittests/Interpreter/CMakeLists.txt
===
--- lldb/unittests/Interpreter/CMakeLists.txt
+++ lldb/unittests/Interpreter/CMakeLists.txt
@@ -1,8 +1,10 @@
 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestUserCommand.cpp
 
   LINK_LIBS
+liblldb
 lldbInterpreter
 lldbUtilityHelpers
   )
Index: lldb/source/API/SBCommandInterpreter.cpp
===
--- lldb/source/API/SBCommandInterpreter.cpp
+++ lldb/source/API/SBCommandInterpreter.cpp
@@ -216,6 +216,14 @@
   : false);
 }
 
+bool SBCommandInterpreter::UserCommandExists(const char *cmd) {
+  LLDB_RECORD_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+ (const char *), cmd);
+
+  return (((cmd != nullptr) && IsValid()) ? m_opaque_ptr->UserCommandExists(cmd)
+  : false);
+}
+
 bool SBCommandInterpreter::AliasExists(const char *cmd) {
   LLDB_RECORD_METHOD(bool, SBCommandInterpreter, AliasExists, (const char *),
  cmd);
@@ -874,6 +882,8 @@
   LLDB_REGISTER_METHOD_CONST(bool, SBCommandInterpreter, operator bool, ());
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, CommandExists,
(const char *));
+  LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, UserCommandExists,
+   (const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, AliasExists,
(const char *));
   LLDB_REGISTER_METHOD(bool, SBCommandInterpreter, IsActive, ());
Index: lldb/include/lldb/API/SBCommandInterpreter.h
===
--- lldb/include/lldb/API/SBCommandInterpreter.h
+++ lldb/include/lldb/API/SBCommandInterpreter.h
@@ -91,8 +91,28 @@
 
   bool IsValid() const;
 
+  /// Determine whether a command exists in this CommandInterpreter.
+  ///
+  /// For commands registered using the LLDB API, see UserCommandExists
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
   bool CommandExists(const char *cmd);
 
+  /// Determine whether a user command exists in this CommandInterpreter.
+  /// Users commands are the ones registered using the LLDB API and are not
+  /// provided by default in LLDB.
+  ///
+  /// \param[in] cmd
+  /// The command to look up.
+  ///
+  /// \return
+  /// \b true if the provided command exists, \b false otherwise.
+  bool UserCommandExists(const char *cmd);
+
   bool AliasExists(const char *cmd);
 
   lldb::SBBroadcaster GetBroadcaster();
Index: lldb/bindings/interface/SBCommandInterpreter.i
===
--- lldb/bindings/interface/SBCommandInterpreter.i