JDevlieghere added inline comments.
================
Comment at: lldb/source/Commands/CommandObjectSession.cpp:10
+public:
+ // Constructors and Destructors
+ CommandObjectSessionSave(CommandInterpreter &interpreter)
----------------
Remove
================
Comment at: lldb/source/Commands/CommandObjectSession.cpp:43
+ bool DoExecute(Args &args, CommandReturnObject &result) override {
+ llvm::StringRef file_path = "";
+
----------------
StringRef is empty by default, remove `= ""`
================
Comment at: lldb/source/Commands/CommandObjectSession.cpp:48
+
+ bool success = m_interpreter.SaveTranscripts(file_path);
+
----------------
Inline the variable below.
================
Comment at: lldb/source/Commands/CommandObjectSession.cpp:64
+ CommandObjectSP(new CommandObjectSessionSave(interpreter)));
+ // TODO: Move 'history' subcommand from CommandObjectCommands
+}
----------------
Missing period.
================
Comment at: lldb/source/Commands/CommandObjectSession.h:20
+public:
+ // Constructors and Destructors
+ CommandObjectSession(CommandInterpreter &interpreter);
----------------
Remove
================
Comment at: lldb/source/Commands/CommandObjectSession.h:26
+private:
+ // For CommandObjectSession only
+ CommandObjectSession(const CommandObjectSession &) = delete;
----------------
Remove
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:124
+ m_command_source_depth(0), m_result(),
+ m_session_transcripts(new StreamString()) {
SetEventName(eBroadcastBitThreadShouldExit, "thread-should-exit");
----------------
use `make_shared`
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2815
+ if (line.find("session save") == line.npos) {
+ *m_session_transcripts << io_handler.GetPrompt() << ' ' << line.c_str()
----------------
this won't work for things like unambiguous abbreviations like `sess save`. The
command should do the saving.
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2913
+bool CommandInterpreter::SaveTranscripts(llvm::StringRef file_path) {
+ std::string output_file = file_path.str();
+
----------------
If you're going to convert the StringRef to a std::string you might as well
have that passed in by value so that the caller could move it when appropriate.
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2929
+ Status error;
+ user_id_t fd_dst = FileCache::GetInstance().OpenFile(
+ FileSpec(output_file), flags, lldb::eFilePermissionsFileDefault, error);
----------------
Why are you going through the `FileCache`?
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2941
+
+ if (fd_dst == UINT64_MAX || error.Fail())
+ return error_out("Unable to create file");
----------------
I think we have a constant LLDB_INVALID_FILE_DESCRIPTOR or something? If not
you should use std::numeric_limits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82155/new/
https://reviews.llvm.org/D82155
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
