Handle more signals #61

asdf · 2019-10-24T03:13:25Z

asdf commented

2019-10-24 03:13:25 +00:00

This PR expands on previous signal handling work.

It handles CTRL+Z, CTRL+C as well as returning to a stopped process, without breaking the terminal.

This PR expands on previous signal handling work. It handles CTRL+Z, CTRL+C as well as returning to a stopped process, without breaking the terminal.

asdf added the

enhancement

non-functional

non-urgent

labels 2019-10-24 03:13:25 +00:00

asdf self-assigned this 2019-10-24 03:13:37 +00:00

sloum was assigned by asdf

2019-10-24 03:13:38 +00:00

asdf reviewed 2019-10-24 03:17:40 +00:00

main.go

asdf commented

2019-10-24 03:17:40 +00:00

This was moved to cui.InitTerm() which is now executed directly in main(). I'm not sure on the full impact of this move.

sloum approved these changes 2019-10-24 03:33:27 +00:00

sloum left a comment

This looks great! I think these are very solid changes. :) I left a few comments, but nothing requiring action codewise. Approved :)

main.go

						
				@ -140,7 +140,6 @@ func loadConfig() error {

				func initClient() error {

					bombadillo = MakeClient("  ((( Bombadillo )))  ")

					cui.SetCharMode()

sloum commented

2019-10-24 03:28:17 +00:00

I see no reason that should be a problem.

asdf commented

2019-10-25 01:29:29 +00:00

Yeah upon further investigation it looks ok to me too, I was just a bit worried about error handling.

main.go

						
				@ -158,3 +156,1 @@

						cui.Tput("smcup") // use alternate screen

						cui.SetCharMode()

						bombadillo.Draw()

						switch <-c {

sloum commented

2019-10-24 03:32:33 +00:00

I have not used channels a lot (though did recently for a game I made...plummet, available on my tildegit). We are in a goroutine so it wont matter, but just for my own knowledge: this line will block until a signal is received, yeah?

asdf commented

2019-10-25 01:53:17 +00:00

That's correct. It's a buffered channel, and blocks until a value is received.

This is the first time I've used channels, but they are interesting and work well.

The general logic here is that signal.Notify listens for the specified signals, then sends them to the channel when they are received. In this goroutine, the channel is blocked until it gets a signal, and then it does the appropriate thing.

You can assign the value from a channel using x := <- c like you have done in plummet. You can also just do <- c if you just want to wait until it receives a value, but don't actually want to do anything with that value.

The way it is being used here, you could also write it as:
x := <-c
switch x {
case ...

This is also the first time I'm seeing the select statement, so thanks for the prompt on that.

btw plummet is pretty cool! A nice little game.

That's correct. It's a buffered channel, and blocks until a value is received. This is the first time I've used channels, but they are interesting and work well. The general logic here is that `signal.Notify` listens for the specified signals, then sends them to the channel when they are received. In this goroutine, the channel is blocked until it gets a signal, and then it does the appropriate thing. You can assign the value from a channel using `x := <- c` like you have done in plummet. You can also just do `<- c` if you just want to wait until it receives a value, but don't actually want to do anything with that value. The way it is being used here, you could also write it as: x := <-c switch x { case ... This is also the first time I'm seeing the [select](https://tour.golang.org/concurrency/5) statement, so thanks for the prompt on that. btw plummet is pretty cool! A nice little game.

main.go

						
				@ -161,0 +156,4 @@

						switch <-c {

						case syscall.SIGTSTP:

							cui.CleanupTerm()

							syscall.Kill(syscall.Getpid(), syscall.SIGSTOP)

sloum commented

2019-10-24 03:30:43 +00:00

I would have never found this :( The documentation for the syscall lib is the one really swampy place in golang.

asdf commented

2019-10-25 02:01:03 +00:00

Yeah, documentation is a bit light, and it seems like a lot of the code is automatically generated.

I actually found this by searching around. This is the same way that other languages perform this task. Also, in the shell, the command is very similar:

kill -s SIGNAL pid

Also - syscall is also apparently deprecated, but I'm not sure when it is going to be replaced. I guess eventually this will have to be rewritten slightly.

Yeah, documentation is a bit light, and it seems like a lot of the code is automatically generated. I actually found this by searching around. This is the same way that other languages perform this task. Also, in the shell, the command is very similar: kill -s SIGNAL pid Also - syscall is also apparently deprecated, but I'm not sure when it is going to be replaced. I guess eventually this will have to be rewritten slightly.