[lldb/Commands] Alias script command to scripting run
#97263
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Med Ismail Bennani (medismailben) ChangesThis patch introduces a new top-level To avoid breaking the The reason behind this change is to have a top-level command that will cover scripting related subcommands. Full diff: https://github.com/llvm/llvm-project/pull/97263.diff 5 Files Affected:
diff --git a/lldb/source/Commands/CMakeLists.txt b/lldb/source/Commands/CMakeLists.txt
index 6a36c5376d5c5..76397227d535d 100644
--- a/lldb/source/Commands/CMakeLists.txt
+++ b/lldb/source/Commands/CMakeLists.txt
@@ -26,7 +26,7 @@ add_lldb_library(lldbCommands NO_PLUGIN_DEPENDENCIES
CommandObjectQuit.cpp
CommandObjectRegexCommand.cpp
CommandObjectRegister.cpp
- CommandObjectScript.cpp
+ CommandObjectScripting.cpp
CommandObjectSession.cpp
CommandObjectSettings.cpp
CommandObjectSource.cpp
diff --git a/lldb/source/Commands/CommandObjectScript.cpp b/lldb/source/Commands/CommandObjectScripting.cpp
similarity index 68%
rename from lldb/source/Commands/CommandObjectScript.cpp
rename to lldb/source/Commands/CommandObjectScripting.cpp
index 25f25b8e65947..72f653690e532 100644
--- a/lldb/source/Commands/CommandObjectScript.cpp
+++ b/lldb/source/Commands/CommandObjectScripting.cpp
@@ -1,4 +1,4 @@
-//===-- CommandObjectScript.cpp -------------------------------------------===//
+//===-- CommandObjectScripting.cpp ----------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,7 +6,7 @@
//
//===----------------------------------------------------------------------===//
-#include "CommandObjectScript.h"
+#include "CommandObjectScripting.h"
#include "lldb/Core/Debugger.h"
#include "lldb/DataFormatters/DataVisualization.h"
#include "lldb/Host/Config.h"
@@ -21,10 +21,10 @@
using namespace lldb;
using namespace lldb_private;
-#define LLDB_OPTIONS_script
+#define LLDB_OPTIONS_scripting_execute
#include "CommandOptions.inc"
-Status CommandObjectScript::CommandOptions::SetOptionValue(
+Status CommandObjectScriptingExecute::CommandOptions::SetOptionValue(
uint32_t option_idx, llvm::StringRef option_arg,
ExecutionContext *execution_context) {
Status error;
@@ -46,27 +46,29 @@ Status CommandObjectScript::CommandOptions::SetOptionValue(
return error;
}
-void CommandObjectScript::CommandOptions::OptionParsingStarting(
+void CommandObjectScriptingExecute::CommandOptions::OptionParsingStarting(
ExecutionContext *execution_context) {
language = lldb::eScriptLanguageNone;
}
llvm::ArrayRef<OptionDefinition>
-CommandObjectScript::CommandOptions::GetDefinitions() {
- return llvm::ArrayRef(g_script_options);
+CommandObjectScriptingExecute::CommandOptions::GetDefinitions() {
+ return llvm::ArrayRef(g_scripting_execute_options);
}
-CommandObjectScript::CommandObjectScript(CommandInterpreter &interpreter)
+CommandObjectScriptingExecute::CommandObjectScriptingExecute(
+ CommandInterpreter &interpreter)
: CommandObjectRaw(
- interpreter, "script",
+ interpreter, "scripting execute",
"Invoke the script interpreter with provided code and display any "
"results. Start the interactive interpreter if no code is supplied.",
- "script [--language <scripting-language> --] [<script-code>]") {}
+ "scripting execute [--language <scripting-language> --] "
+ "[<script-code>]") {}
-CommandObjectScript::~CommandObjectScript() = default;
+CommandObjectScriptingExecute::~CommandObjectScriptingExecute() = default;
-void CommandObjectScript::DoExecute(llvm::StringRef command,
- CommandReturnObject &result) {
+void CommandObjectScriptingExecute::DoExecute(llvm::StringRef command,
+ CommandReturnObject &result) {
// Try parsing the language option but when the command contains a raw part
// separated by the -- delimiter.
OptionsWithRaw raw_args(command);
@@ -111,3 +113,19 @@ void CommandObjectScript::DoExecute(llvm::StringRef command,
else
result.SetStatus(eReturnStatusFailed);
}
+
+#pragma mark CommandObjectMultiwordScripting
+
+// CommandObjectMultiwordScripting
+
+CommandObjectMultiwordScripting::CommandObjectMultiwordScripting(
+ CommandInterpreter &interpreter)
+ : CommandObjectMultiword(
+ interpreter, "scripting",
+ "Commands for operating on the scripting functionnalities.",
+ "scripting <subcommand> [<subcommand-options>]") {
+ LoadSubCommand("execute", CommandObjectSP(new CommandObjectScriptingExecute(
+ interpreter)));
+}
+
+CommandObjectMultiwordScripting::~CommandObjectMultiwordScripting() = default;
diff --git a/lldb/source/Commands/CommandObjectScript.h b/lldb/source/Commands/CommandObjectScripting.h
similarity index 71%
rename from lldb/source/Commands/CommandObjectScript.h
rename to lldb/source/Commands/CommandObjectScripting.h
index 3a8c4a890404a..b4a8a32cb644a 100644
--- a/lldb/source/Commands/CommandObjectScript.h
+++ b/lldb/source/Commands/CommandObjectScripting.h
@@ -1,4 +1,4 @@
-//===-- CommandObjectScript.h -----------------------------------*- C++ -*-===//
+//===-- CommandObjectScripting.h --------------------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -9,14 +9,21 @@
#ifndef LLDB_SOURCE_INTERPRETER_COMMANDOBJECTSCRIPT_H
#define LLDB_SOURCE_INTERPRETER_COMMANDOBJECTSCRIPT_H
-#include "lldb/Interpreter/CommandObject.h"
+#include "lldb/Interpreter/CommandObjectMultiword.h"
namespace lldb_private {
-class CommandObjectScript : public CommandObjectRaw {
+class CommandObjectMultiwordScripting : public CommandObjectMultiword {
public:
- CommandObjectScript(CommandInterpreter &interpreter);
- ~CommandObjectScript() override;
+ CommandObjectMultiwordScripting(CommandInterpreter &interpreter);
+
+ ~CommandObjectMultiwordScripting() override;
+};
+
+class CommandObjectScriptingExecute : public CommandObjectRaw {
+public:
+ CommandObjectScriptingExecute(CommandInterpreter &interpreter);
+ ~CommandObjectScriptingExecute() override;
Options *GetOptions() override { return &m_options; }
class CommandOptions : public Options {
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index fa8af7cb3d762..752121ee6edb2 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -835,7 +835,7 @@ let Command = "container add" in {
Desc<"Overwrite an existing command at this node.">;
}
-let Command = "script" in {
+let Command = "scripting execute" in {
def script_language : Option<"language", "l">,
EnumArg<"ScriptLang">, Desc<"Specify the scripting "
" language. If none is specific the default scripting language is used.">;
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 40c915b2c94fc..fe3e68ec4e2a0 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -33,7 +33,7 @@
#include "Commands/CommandObjectQuit.h"
#include "Commands/CommandObjectRegexCommand.h"
#include "Commands/CommandObjectRegister.h"
-#include "Commands/CommandObjectScript.h"
+#include "Commands/CommandObjectScripting.h"
#include "Commands/CommandObjectSession.h"
#include "Commands/CommandObjectSettings.h"
#include "Commands/CommandObjectSource.h"
@@ -518,6 +518,11 @@ void CommandInterpreter::Initialize() {
AddAlias("re", cmd_obj_sp);
}
+ cmd_obj_sp = GetCommandSPExact("scripting execute");
+ if (cmd_obj_sp) {
+ AddAlias("script", cmd_obj_sp);
+ }
+
cmd_obj_sp = GetCommandSPExact("session history");
if (cmd_obj_sp) {
AddAlias("history", cmd_obj_sp);
@@ -569,7 +574,7 @@ void CommandInterpreter::LoadCommandDictionary() {
REGISTER_COMMAND_OBJECT("process", CommandObjectMultiwordProcess);
REGISTER_COMMAND_OBJECT("quit", CommandObjectQuit);
REGISTER_COMMAND_OBJECT("register", CommandObjectRegister);
- REGISTER_COMMAND_OBJECT("script", CommandObjectScript);
+ REGISTER_COMMAND_OBJECT("scripting", CommandObjectMultiwordScripting);
REGISTER_COMMAND_OBJECT("settings", CommandObjectMultiwordSettings);
REGISTER_COMMAND_OBJECT("session", CommandObjectSession);
REGISTER_COMMAND_OBJECT("source", CommandObjectMultiwordSource);
|
medismailben
force-pushed
the
scripting-top-level-command
branch
from
2a003d7 to
1f3d049
Compare
|
✅ With the latest revision this PR passed the Python code formatter. |
medismailben
force-pushed
the
scripting-top-level-command
branch
from
1f3d049 to
8e016c7
Compare
jimingham
reviewed
Jul 1, 2024
jimingham
reviewed
Jul 1, 2024
jimingham
approved these changes
Jul 1, 2024
kastiglione
reviewed
Jul 1, 2024
Comment on lines
+523
to
+527
| AddAlias("sc", cmd_obj_sp); | ||
| AddAlias("scr", cmd_obj_sp); | ||
| AddAlias("scri", cmd_obj_sp); | ||
| AddAlias("scrip", cmd_obj_sp); | ||
| AddAlias("script", cmd_obj_sp); |
There was a problem hiding this comment.
this is unfortunate, do these pollute help output? if so, can we hide them?
is there no way to make a prefix match an alias, when it's shorter?
There was a problem hiding this comment.
The problem is that if we have the command scripting and the alias script and we see sc we need to resolve that command ambiguity. The only tool we have for ambiguous command resolution at present is "exact matches always win".
If we wanted to solve this w/o making all the exact matches we intend to win, we could add a ranking system to the commands and aliases, which bias ambiguous matches in favor of one or the other of the commands, or some similar system. I worry that would get messy pretty quickly, but anyway, you'd need something like that.
There was a problem hiding this comment.
I agree it's not the most elegant solution but I'd prefer solving the partial matching issue in a follow-up.
There was a problem hiding this comment.
Might be a good idea to write a comment explaining the constraint here? If I came across this code snippet I'd definitely want to know why we do this.
kastiglione
approved these changes
Jul 1, 2024
The problem with this command is that it either executes a script given to it on the command line, or it runs the embedded script interpreter... |
medismailben
force-pushed
the
scripting-top-level-command
branch
from
8e016c7 to
b9eba8c
Compare
This patch introduces a new top-level `scripting` command with an `run` sub-command, that basically replaces the `script` raw command. To avoid breaking the `script` command usages, this patch also adds an `script` alias to the `scripting run` sub-command. The reason behind this change is to have a top-level command that will cover scripting related subcommands. Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
force-pushed
the
scripting-top-level-command
branch
from
b9eba8c to
47344cb
Compare
medismailben
changed the title
script command to scripting executescript command to scripting run
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/141/builds/425 Here is the relevant piece of the build log for the reference: |
This is a flakey test that was already failing in previous builds: https://lab.llvm.org/buildbot/#/builders/141/builds/423 |
|
I've disabled the test on Windows and Linaro will look into it. |
I don't see the problem. The original |
I don't know if you saw but I actually changed the command to be |
|
@medismailben 👍 I saw, and also wanted to reply to Jim. |
lravenclaw
pushed a commit
to lravenclaw/llvm-project
that referenced
this pull request
Jul 3, 2024
This patch introduces a new top-level `scripting` command with an `run` sub-command, that basically replaces the `script` raw command. To avoid breaking the `script` command usages, this patch also adds an `script` alias to the `scripting run` sub-command. The reason behind this change is to have a top-level command that will cover scripting related subcommands. Signed-off-by: Med Ismail Bennani <[email protected]>
JDevlieghere
reviewed
Jul 3, 2024
Comment on lines
81
to
82
| command_interpreter.ResolveCommand(r"""sc print("\n\n\tHello!\n")""", result) | ||
| self.assertTrue(result.Succeeded()) |
There was a problem hiding this comment.
You could test all the prefixes with something like this:
str = 'script'
for i in range(2, len(str)):
command = str[0:i]
self.assertEqual(command ...)
Which other scripting subcommands do you envision, besides the template subcommand? |
From @jimingham reply: One that I want, powered by facilities we can add to the template, is that a class that implements a breakpoint callback should have a registry method (that's the part we can probably generate with the template instantiation) that adds callback class and help info to lldb. Then you would do: And similar for all the other affordances. We could do this by having BTW, I'm not proposing Maybe It might also be handy to have: (lldb) scripting modules list -l Python etc... Which would list all the Python modules that we've loaded with "command script import" and where they came from. |
kbluck
pushed a commit
to kbluck/llvm-project
that referenced
this pull request
Jul 6, 2024
This patch introduces a new top-level `scripting` command with an `run` sub-command, that basically replaces the `script` raw command. To avoid breaking the `script` command usages, this patch also adds an `script` alias to the `scripting run` sub-command. The reason behind this change is to have a top-level command that will cover scripting related subcommands. Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
added a commit
to medismailben/llvm-project
that referenced
this pull request
Aug 5, 2024
This patch is a follow-up to llvm#97263 that fix ambigous abbreviated command resolution. When multiple commands are resolved, instead of failing to pick a command to run, this patch changes to resolution logic to check if there is either a single user command match or a single alias match and if so, it will run that command instead of the others. This has as a side-effect that we don't need to make aliases for every substring of aliases to support abbrivated alias resolution. Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
added a commit
to medismailben/llvm-project
that referenced
this pull request
Aug 7, 2024
This patch is a follow-up to llvm#97263 that fix ambigous abbreviated command resolution. When multiple commands are resolved, instead of failing to pick a command to run, this patch changes to resolution logic to check if there is a single alias match and if so, it will run the alias instead of the other matches. This has as a side-effect that we don't need to make aliases for every substring of aliases to support abbrivated alias resolution. Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
added a commit
to medismailben/llvm-project
that referenced
this pull request
Aug 7, 2024
This patch is a follow-up to llvm#97263 that fix ambigous abbreviated command resolution. When multiple commands are resolved, instead of failing to pick a command to run, this patch changes to resolution logic to check if there is a single alias match and if so, it will run the alias instead of the other matches. This has as a side-effect that we don't need to make aliases for every substring of aliases to support abbrivated alias resolution. Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
added a commit
to medismailben/llvm-project
that referenced
this pull request
Aug 8, 2024
This patch is a follow-up to llvm#97263 that fix ambigous abbreviated command resolution. When multiple commands are resolved, instead of failing to pick a command to run, this patch changes to resolution logic to check if there is a single alias match and if so, it will run the alias instead of the other matches. This has as a side-effect that we don't need to make aliases for every substring of aliases to support abbrivated alias resolution. Signed-off-by: Med Ismail Bennani <[email protected]>
medismailben
added a commit
that referenced
this pull request
Aug 8, 2024
This patch is a follow-up to #97263 that fix ambigous abbreviated command resolution. When multiple commands are resolved, instead of failing to pick a command to run, this patch changes to resolution logic to check if there is a single alias match and if so, it will run the alias instead of the other matches. This has as a side-effect that we don't need to make aliases for every substring of aliases to support abbrivated alias resolution. Signed-off-by: Med Ismail Bennani <[email protected]>
adrian-prantl
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Oct 11, 2024
) This patch is a follow-up to llvm#97263 that fix ambigous abbreviated command resolution. When multiple commands are resolved, instead of failing to pick a command to run, this patch changes to resolution logic to check if there is a single alias match and if so, it will run the alias instead of the other matches. This has as a side-effect that we don't need to make aliases for every substring of aliases to support abbrivated alias resolution. Signed-off-by: Med Ismail Bennani <[email protected]> (cherry picked from commit 8334d2b)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch introduces a new top-level
scriptingcommand with anrunsub-command, that basically replaces thescriptraw command.To avoid breaking the
scriptcommand usages, this patch also adds anscriptalias to thescripting runsub-command.The reason behind this change is to have a top-level command that will cover scripting related subcommands.