-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Fix a bug in handling ^C at the "y/n/a" completion prompt. #67621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
We just forget to check for interrupt while waiting for the answer to the prompt. But if we are in the interrupt state then the lower layers of the EditLine code just eat all characters so we never get out of the query prompt. You're pretty much stuck and have to kill lldb. The solution is to check for the interrupt. The patch is a little bigger because where I needed to check the Interrupt state I only had the ::EditLine object, but the editor state is held in lldb's EditLine wrapper, so I had to do a little work to get my hands on it.
@llvm/pr-subscribers-lldb ChangesWe just forget to check for interrupt while waiting for the answer to the prompt. But if we are in the interrupt state then the lower layers of the EditLine code just eat all characters so we never get out of the query prompt. You're pretty much stuck and have to kill lldb. The solution is to check for the interrupt. The patch is a little bigger because where I needed to check the Interrupt state I only had the ::EditLine object, but the editor state is held in lldb's EditLine wrapper, so I had to do a little work to get my hands on it. Full diff: https://github.com/llvm/llvm-project/pull/67621.diff 3 Files Affected:
diff --git a/lldb/include/lldb/Host/Editline.h b/lldb/include/lldb/Host/Editline.h
index e1807aa5492680e..c598244150788da 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -161,6 +161,10 @@ class Editline {
/// of Editline.
static Editline *InstanceFor(::EditLine *editline);
+ static void
+ DisplayCompletions(Editline &editline,
+ llvm::ArrayRef<CompletionResult::Completion> results);
+
/// Sets a string to be used as a prompt, or combined with a line number to
/// form a prompt.
void SetPrompt(const char *prompt);
diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp
index edefbf4008129e4..82e17ec753ab235 100644
--- a/lldb/source/Host/common/Editline.cpp
+++ b/lldb/source/Host/common/Editline.cpp
@@ -943,12 +943,12 @@ PrintCompletion(FILE *output_file,
}
}
-static void
-DisplayCompletions(::EditLine *editline, FILE *output_file,
- llvm::ArrayRef<CompletionResult::Completion> results) {
+void Editline::DisplayCompletions(
+ Editline &editline, llvm::ArrayRef<CompletionResult::Completion> results) {
assert(!results.empty());
- fprintf(output_file, "\n" ANSI_CLEAR_BELOW "Available completions:\n");
+ fprintf(editline.m_output_file,
+ "\n" ANSI_CLEAR_BELOW "Available completions:\n");
const size_t page_size = 40;
bool all = false;
@@ -960,7 +960,7 @@ DisplayCompletions(::EditLine *editline, FILE *output_file,
const size_t max_len = longest->GetCompletion().size();
if (results.size() < page_size) {
- PrintCompletion(output_file, results, max_len);
+ PrintCompletion(editline.m_output_file, results, max_len);
return;
}
@@ -969,17 +969,25 @@ DisplayCompletions(::EditLine *editline, FILE *output_file,
size_t remaining = results.size() - cur_pos;
size_t next_size = all ? remaining : std::min(page_size, remaining);
- PrintCompletion(output_file, results.slice(cur_pos, next_size), max_len);
+ PrintCompletion(editline.m_output_file, results.slice(cur_pos, next_size),
+ max_len);
cur_pos += next_size;
if (cur_pos >= results.size())
break;
- fprintf(output_file, "More (Y/n/a): ");
+ fprintf(editline.m_output_file, "More (Y/n/a): ");
char reply = 'n';
- int got_char = el_getc(editline, &reply);
- fprintf(output_file, "\n");
+ int got_char = el_getc(editline.m_editline, &reply);
+ // Check for a ^C or other interruption.
+ if (editline.m_editor_status == EditorStatus::Interrupted) {
+ editline.m_editor_status = EditorStatus::Editing;
+ fprintf(editline.m_output_file, "^C\n");
+ break;
+ }
+
+ fprintf(editline.m_output_file, "\n");
if (got_char == -1 || reply == 'n')
break;
if (reply == 'a')
@@ -1050,7 +1058,7 @@ unsigned char Editline::TabCommand(int ch) {
return CC_REDISPLAY;
}
- DisplayCompletions(m_editline, m_output_file, results);
+ DisplayCompletions(*this, results);
DisplayInput();
MoveCursor(CursorLocation::BlockEnd, CursorLocation::EditingCursor);
diff --git a/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py b/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
index 0e28a94adf1e4a3..a74a6572f71f872 100644
--- a/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
+++ b/lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py
@@ -75,6 +75,12 @@ def test_completion(self):
self.child.send("n")
self.expect_prompt()
+ # Start tab completion and abort showing more commands with '^C'.
+ self.child.send("\t")
+ self.child.expect_exact("More (Y/n/a)")
+ self.child.sendcontrol('c')
+ self.expect_prompt()
+
# Shouldn't crash or anything like that.
self.child.send("regoinvalid\t")
self.expect_prompt()
|
You can test this locally with the following command:darker --check --diff -r 22433cc541ff706d9e845774ef1c8c959dc67799..fab5e1f67433a29a5d54198d83960d7e608dc480 lldb/test/API/iohandler/completion/TestIOHandlerCompletion.py View the diff from darker here.--- TestIOHandlerCompletion.py 2023-09-28 00:07:33.000000 +0000
+++ TestIOHandlerCompletion.py 2023-09-28 00:12:29.727805 +0000
@@ -76,11 +76,11 @@
self.expect_prompt()
# Start tab completion and abort showing more commands with '^C'.
self.child.send("\t")
self.child.expect_exact("More (Y/n/a)")
- self.child.sendcontrol('c')
+ self.child.sendcontrol("c")
self.expect_prompt()
# Shouldn't crash or anything like that.
self.child.send("regoinvalid\t")
self.expect_prompt()
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Please fix the python formatting before landing.
Seriously, we're enforcing "" vrs. ''? This is silly. |
I pushed this from the repro, going through the PR hoops was too much work just for uglification... |
We just forget to check for interrupt while waiting for the answer to the prompt. But if we are in the interrupt state then the lower layers of the EditLine code just eat all characters so we never get out of the query prompt. You're pretty much stuck and have to kill lldb.
The solution is to check for the interrupt. The patch is a little bigger because where I needed to check the Interrupt state I only had the ::EditLine object, but the editor state is held in lldb's EditLine wrapper, so I had to do a little work to get my hands on it.