Skip to content

[lldb] Change some Swift REPL intro text #3125

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[lldb] Change some Swift REPL intro text #3125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ lldb::REPLSP SwiftREPL::CreateInstanceFromTarget(Status &err, Target &target,
lldb::REPLSP SwiftREPL::CreateInstanceFromDebugger(Status &err,
Debugger &debugger,
const char *repl_options) {
printf("Starting the Swift REPL...\n");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok since there's a bit of a delay before the REPL is available.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, a cold module cache can create some pretty long wait times on slower machines.

const char *bp_name = "repl_main";

FileSpec repl_executable = HostInfo::GetSupportExeDir();
Expand Down Expand Up @@ -262,9 +263,8 @@ lldb::REPLSP SwiftREPL::CreateInstanceFromDebugger(Status &err,
// Disable the cleanup, since we have a valid repl session now.
cleanup.release();

std::string swift_full_version(swift::version::getSwiftFullVersion());
printf("Welcome to %s.\nType :help for assistance.\n",
swift_full_version.c_str());
printf("You are now in an interactive environment powered by the LLDB debugger where you can type, run, and debug Swift code.\n\n");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this message, seems a bit superfluous in my opinion.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went over this a little bit in the forum post. The reason for this message is that a lot of folks "end up" in the REPL, either intentionally or unintentionally through some default behavior. For folks learning Swift for the first time or someone who doesn't have a lot of command line experience with Swift, it can be rather mysterious as to what they are looking at. I thought it was also nice to call out that this is actually LLDB and not some other thing, so people start to understand the context they are in.

Again, if the REPL becomes an explicit choice, then of course the user should know where they are.

printf("Type \x1B[1m:help\x1B[0m for debugger assistance. Type \x1B[1m:exit\x1B[0m to quit.\n");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this message. Maybe while we're at it, could we tell users that they can run any LLDB commands with :command? I feel that many users don't know that's possible,

I think we should definitely keep the swift version though, as that may be helpful to users.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the reason why I removed the version print is that I pushed it up to the driver so that it appears consistently at the top when running other sort of "automatic" commands. If the Swift REPL is no longer run by default (something I eventually want to change), then running the REPL will be an explicit command without the larger context. It wouldn't hurt to print the version twice in the meantime but that was the reason, anyway.


return repl_sp;
}
Expand Down
2 changes: 1 addition & 1 deletion lldb/test/Shell/SwiftREPL/CFString.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// REQUIRES: swift

// RUN: %lldb --repl < %s | FileCheck %s
// CHECK: Welcome to {{.*}}Swift
// CHECK: You are now in an interactive environment

import Foundation

Expand Down
2 changes: 1 addition & 1 deletion lldb/test/Shell/SwiftREPL/ComputedProperties.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ var x : Int {
}
}

// CHECK: Welcome to {{.*}}Swift
// CHECK: You are now in an interactive environment
// CHECK: Type :help
// CHECK: {{x}}: Int

Expand Down
2 changes: 1 addition & 1 deletion lldb/test/Shell/SwiftREPL/UninitVariables.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// RUN: %lldb --repl < %s | FileCheck %s

var x : Int
// CHECK: Welcome to {{.*}}Swift
// CHECK: You are now in an interactive environment
// CHECK: Type :help
// CHECK: {{x}}: Int = 0
x = 42
Expand Down