[Lldb-commits] [lldb] Add warning message to `session save` when transcript isn't saved. (PR #109020)
Tom Yang via lldb-commits
lldb-commits at lists.llvm.org
Mon Sep 30 14:43:36 PDT 2024
https://github.com/zhyty updated https://github.com/llvm/llvm-project/pull/109020
>From 60045b710e1102d6f220dfd4367f997b73bb64df Mon Sep 17 00:00:00 2001
From: Tom Yang <toyang at fb.com>
Date: Tue, 17 Sep 2024 09:56:09 -0700
Subject: [PATCH 1/2] Add warning message to `session save` when transcript
isn't saved.
Somewhat recently, we made the change to hide the behavior to save LLDB
session history to the transcript buffer behind the flag
`interpreter.save-transcript`.
---
lldb/source/Commands/CommandObjectSession.cpp | 3 +-
.../source/Interpreter/CommandInterpreter.cpp | 2 ++
.../Interpreter/InterpreterProperties.td | 2 +-
.../commands/session/save/TestSessionSave.py | 29 +++++++++++++++++++
4 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectSession.cpp b/lldb/source/Commands/CommandObjectSession.cpp
index c381ba4f74f120..3f714cec414695 100644
--- a/lldb/source/Commands/CommandObjectSession.cpp
+++ b/lldb/source/Commands/CommandObjectSession.cpp
@@ -19,7 +19,8 @@ class CommandObjectSessionSave : public CommandObjectParsed {
: CommandObjectParsed(interpreter, "session save",
"Save the current session transcripts to a file.\n"
"If no file if specified, transcripts will be "
- "saved to a temporary file.",
+ "saved to a temporary file.\n"
+ "Note: transcripts will only be saved if interpreter.save-transcript is true.\n",
"session save [file]") {
AddSimpleArgumentList(eArgTypePath, eArgRepeatOptional);
}
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index b93f47a8a8d5ec..05426771ba0335 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3306,6 +3306,8 @@ bool CommandInterpreter::SaveTranscript(
result.SetStatus(eReturnStatusSuccessFinishNoResult);
result.AppendMessageWithFormat("Session's transcripts saved to %s\n",
output_file->c_str());
+ if (!GetSaveTranscript())
+ result.AppendError("Note: the setting interpreter.save-transcript is set to false, so the transcript might not have been recorded.");
if (GetOpenTranscriptInEditor() && Host::IsInteractiveGraphicSession()) {
const FileSpec file_spec;
diff --git a/lldb/source/Interpreter/InterpreterProperties.td b/lldb/source/Interpreter/InterpreterProperties.td
index a5fccbbca091cf..834f7be17480c6 100644
--- a/lldb/source/Interpreter/InterpreterProperties.td
+++ b/lldb/source/Interpreter/InterpreterProperties.td
@@ -16,7 +16,7 @@ let Definition = "interpreter" in {
def SaveSessionOnQuit: Property<"save-session-on-quit", "Boolean">,
Global,
DefaultFalse,
- Desc<"If true, LLDB will save the session's transcripts before quitting.">;
+ Desc<"If true, LLDB will save the session's transcripts before quitting. Note: transcripts will only be saved if interpreter.save-transcript is true.">;
def OpenTranscriptInEditor: Property<"open-transcript-in-editor", "Boolean">,
Global,
DefaultTrue,
diff --git a/lldb/test/API/commands/session/save/TestSessionSave.py b/lldb/test/API/commands/session/save/TestSessionSave.py
index aa99bcd56aed46..c81ff645d9d3b8 100644
--- a/lldb/test/API/commands/session/save/TestSessionSave.py
+++ b/lldb/test/API/commands/session/save/TestSessionSave.py
@@ -85,6 +85,8 @@ def test_session_save(self):
interpreter.HandleCommand("session save", res)
self.assertTrue(res.Succeeded())
raw += self.raw_transcript_builder(cmd, res)
+ # Also check that we don't print an error message about an empty transcript.
+ self.assertNotIn("interpreter.save-transcript is set to false", res.GetError())
with open(os.path.join(td.name, os.listdir(td.name)[0]), "r") as file:
content = file.read()
@@ -93,6 +95,33 @@ def test_session_save(self):
for line in lines:
self.assertIn(line, content)
+ @no_debug_info_test
+ def test_session_save_no_transcript_warning(self):
+ interpreter = self.dbg.GetCommandInterpreter()
+
+ self.runCmd("settings set interpreter.save-transcript false")
+
+ # These commands won't be saved, so are arbitrary.
+ commands = [
+ "p 1",
+ "settings set interpreter.save-session-on-quit true",
+ "fr v",
+ "settings set interpreter.echo-comment-commands true",
+ ]
+
+ for command in commands:
+ res = lldb.SBCommandReturnObject()
+ interpreter.HandleCommand(command, res)
+
+ output_file = self.getBuildArtifact('my-session')
+
+ res = lldb.SBCommandReturnObject()
+ interpreter.HandleCommand("session save " + output_file, res)
+ self.assertTrue(res.Succeeded())
+ # We should warn about the setting being false.
+ self.assertIn("interpreter.save-transcript is set to false", res.GetError())
+ self.assertTrue(os.path.getsize(output_file) == 0, "Output file should be empty since we didn't save the transcript.")
+
@no_debug_info_test
def test_session_save_on_quit(self):
raw = ""
>From 80d99fcad0c5d9e66a1d5608e9035e212dd7b93a Mon Sep 17 00:00:00 2001
From: Tom Yang <toyang at fb.com>
Date: Fri, 20 Sep 2024 17:49:28 -0700
Subject: [PATCH 2/2] fix clang, darker formatting
---
lldb/source/Commands/CommandObjectSession.cpp | 3 ++-
lldb/source/Interpreter/CommandInterpreter.cpp | 4 +++-
lldb/test/API/commands/session/save/TestSessionSave.py | 7 +++++--
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Commands/CommandObjectSession.cpp b/lldb/source/Commands/CommandObjectSession.cpp
index 3f714cec414695..ac7eec5e04f0a2 100644
--- a/lldb/source/Commands/CommandObjectSession.cpp
+++ b/lldb/source/Commands/CommandObjectSession.cpp
@@ -20,7 +20,8 @@ class CommandObjectSessionSave : public CommandObjectParsed {
"Save the current session transcripts to a file.\n"
"If no file if specified, transcripts will be "
"saved to a temporary file.\n"
- "Note: transcripts will only be saved if interpreter.save-transcript is true.\n",
+ "Note: transcripts will only be saved if "
+ "interpreter.save-transcript is true.\n",
"session save [file]") {
AddSimpleArgumentList(eArgTypePath, eArgRepeatOptional);
}
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 05426771ba0335..f6537a0135ce76 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3307,7 +3307,9 @@ bool CommandInterpreter::SaveTranscript(
result.AppendMessageWithFormat("Session's transcripts saved to %s\n",
output_file->c_str());
if (!GetSaveTranscript())
- result.AppendError("Note: the setting interpreter.save-transcript is set to false, so the transcript might not have been recorded.");
+ result.AppendError(
+ "Note: the setting interpreter.save-transcript is set to false, so the "
+ "transcript might not have been recorded.");
if (GetOpenTranscriptInEditor() && Host::IsInteractiveGraphicSession()) {
const FileSpec file_spec;
diff --git a/lldb/test/API/commands/session/save/TestSessionSave.py b/lldb/test/API/commands/session/save/TestSessionSave.py
index c81ff645d9d3b8..0f064e60844fe2 100644
--- a/lldb/test/API/commands/session/save/TestSessionSave.py
+++ b/lldb/test/API/commands/session/save/TestSessionSave.py
@@ -113,14 +113,17 @@ def test_session_save_no_transcript_warning(self):
res = lldb.SBCommandReturnObject()
interpreter.HandleCommand(command, res)
- output_file = self.getBuildArtifact('my-session')
+ output_file = self.getBuildArtifact("my-session")
res = lldb.SBCommandReturnObject()
interpreter.HandleCommand("session save " + output_file, res)
self.assertTrue(res.Succeeded())
# We should warn about the setting being false.
self.assertIn("interpreter.save-transcript is set to false", res.GetError())
- self.assertTrue(os.path.getsize(output_file) == 0, "Output file should be empty since we didn't save the transcript.")
+ self.assertTrue(
+ os.path.getsize(output_file) == 0,
+ "Output file should be empty since we didn't save the transcript.",
+ )
@no_debug_info_test
def test_session_save_on_quit(self):
More information about the lldb-commits
mailing list