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 <toy...@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 <toy...@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): _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits