[Lldb-commits] [lldb] 835b5e2 - Add warning message to `session save` when transcript isn't saved. (#109020)

[via lldb-commits]

Author: Tom Yang
Date: 2024-10-05T00:29:44-07:00
New Revision: 835b5e278e525dc628d4d0c085eb272996aed466

URL: https://github.com/llvm/llvm-project/commit/835b5e278e525dc628d4d0c085eb272996aed466
DIFF: https://github.com/llvm/llvm-project/commit/835b5e278e525dc628d4d0c085eb272996aed466.diff

LOG: Add warning message to `session save` when transcript isn't saved. (#109020)

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`. By default, `interpreter.save-transcript`
is false. See #90703 for context.

I'm making a small update here to our `session save` messaging and some
help docs to clarify for users that aren't aware of this change. Maybe
`interpreter.save-transcript` could be true by default as well. Any
feedback welcome.

# Tests
```
bin/lldb-dotest -p TestSessionSave
```

---------

Co-authored-by: Tom Yang <toyang at fb.com>

Added: 
    

Modified: 
    lldb/source/Commands/CommandObjectSession.cpp
    lldb/source/Interpreter/CommandInterpreter.cpp
    lldb/source/Interpreter/InterpreterProperties.td
    lldb/test/API/commands/session/save/TestSessionSave.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Commands/CommandObjectSession.cpp b/lldb/source/Commands/CommandObjectSession.cpp
index c381ba4f74f120..ac7eec5e04f0a2 100644
--- a/lldb/source/Commands/CommandObjectSession.cpp
+++ b/lldb/source/Commands/CommandObjectSession.cpp
@@ -19,7 +19,9 @@ 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 d17aa6fec1f00e..8d3a82ef6c990a 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -3308,6 +3308,10 @@ 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..0f064e60844fe2 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,36 @@ 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 = ""