Skip to content

minor REPL enhancements #233

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

minor REPL enhancements #233

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
21 changes: 11 additions & 10 deletions lib/bud/rebl.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ class ReblShell
@@abbrevs = @@commands.keys.abbrev
@@exit_message = "Rebellion quashed."

# Starts a rebl shell.
#--
# Starts a rebl shell.
#--
# This function is not covered by testcases, but setup
# and rebl_loop are.
#++
Expand All @@ -71,9 +71,6 @@ def self.run
# Performs setup as part of starting a rebl shell, and returns the instance of
# LibRebl that is created; testcases call this directly.
def self.setup
Signal.trap("INT") {do_exit}
Signal.trap("TERM") {do_exit}

ipport = ARGV[0] ? ARGV[0].split(":") : []
lib = LibRebl.new(*[(ipport[0] or "localhost"), (ipport[1] or 0)])
setup_history
Expand All @@ -92,13 +89,17 @@ def self.setup

# One step of the rebl shell loop: processes one rebl shell line from stdin
# and returns. May raise an Exception.
def self.rebl_loop(lib,noreadline=false)
def self.rebl_loop(lib, noreadline=false)
begin
line = Readline::readline('rebl> ') unless noreadline
line = gets if noreadline
if noreadline
line = gets
else
line = Readline::readline('rebl> ')
end
do_exit if line.nil?
puts line unless $stdin.tty?
Copy link
Member

Choose a reason for hiding this comment

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

what's the intention here?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see from your comment you want output when it's used in a pipe. I personally think this should be off by default, but it's a nice command-line option. For Postgres' psql it's the "-a" option, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's a nice REPL feature. I don't see why a flag would be useful, though. Is there a reason someone wouldn't want the output?

Copy link
Member

Choose a reason for hiding this comment

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

Well, if you're writing a pipe it's not necessarily nice to have things in the middle dumping stuff to stdout. As another example, there's tar -v. I'm old (from when cp was braindead) so I often run things like "tar cf - . | (cd /foo/bar; tar xf -)". I definitely don't want both of the tar calls to dump the file list, and in many cases I don't want either one to do so. But I agree that it's a useful flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But who's using REPL output as the input to a pipe? Again, I don't think there's a use case that would prefer the input not to be echoed, so no need for a flag. At least not one that's off by default.

Copy link
Member

Choose a reason for hiding this comment

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

It's not clear to me that you always (or usually) want to see rebl input lines on stdout any more than you'd want to see inbound network messages on stdout. If your goal is to control bud from a command line script and see what it does, it's not clear whether you want that control logic in the output.

Anyway, I'm very open to a command-line flag on this; we can ponder what the right default is but the important thing is to enable what you're asking for here.

line.strip!
return if line.empty?
return if line.empty? or line[0..0] == "#"
Readline::HISTORY.push(line) unless noreadline
split_line = line.split(" ")
if line[0..0] == @@escape_char then
Expand Down Expand Up @@ -183,7 +184,7 @@ def self.do_exit
end
@rebl_class_inst.stop_bg if @rebl_class_inst
puts "\n" + @@exit_message
exit!
exit!(0)
end
end

Expand Down