Replace calls to stty with <termios.h> cgo #156

dancek · 2020-05-23T13:07:40Z

dancek commented

2020-05-23 13:07:40 +00:00

This is IMHO a better way to do it than #155, but I'm leaving both open now if you want to compare a pure-Go single platform solution vs. this multi-platform cgo version.

This removes the dependency on stty and is the standard POSIX way of
modifying terminal state. It requires cgo, so cross-compiling might not
be trivial. Otherwise there are no additional dependencies brought in,
and this already supports all POSIXish platforms.

Fixes #154.

This is IMHO a better way to do it than #155, but I'm leaving both open now if you want to compare a pure-Go single platform solution vs. this multi-platform cgo version. --- This removes the dependency on `stty` and is the standard POSIX way of modifying terminal state. It requires cgo, so cross-compiling might not be trivial. Otherwise there are no additional dependencies brought in, and this already supports all POSIXish platforms. Fixes #154.

sloum commented

2020-05-23 15:51:00 +00:00

This seems slightly simpler than the pure Go route. What is the build process for this? Particularly for cross-compilation.

I'm not, in theory, against C Go and I do get what is going on here... so that is nice. That said, there is a lot of appeal to a pure Go solution. I'll try to build both of these this afternoon and maybe hack on the pure go version a little bit and see if I can get it to build and work for Linux as well. If we can get Linux and OSX working I would feel comfortable moving forward with that. Adding BSD would be nice. We do not officially support Plan9 (I have no idea if Bombadillo will run on 9Front or the like... I've never been able to build the system on my thinkpad to be able to try). We do not support Windows at all, with no plan to do so any time soon (due to the different way they handle the terminal screen... in the long run it would be nice to support, but it is not a high priority at all).

This seems slightly simpler than the pure Go route. What is the build process for this? Particularly for cross-compilation. I'm not, in theory, against C Go and I do get what is going on here... so that is nice. That said, there is a lot of appeal to a pure Go solution. I'll try to build both of these this afternoon and maybe hack on the pure go version a little bit and see if I can get it to build and work for Linux as well. If we can get Linux and OSX working I would feel comfortable moving forward with that. Adding BSD would be nice. We do not officially support Plan9 (I have no idea if Bombadillo will run on 9Front or the like... I've never been able to build the system on my thinkpad to be able to try). We do not support Windows at all, with no plan to do so any time soon (due to the different way they handle the terminal screen... in the long run it would be nice to support, but it is not a high priority at all).

makeworld commented

2020-05-23 17:05:48 +00:00

First-time contributor

I think cgo is probably the wrong decision. So far Bombadillo has avoided adding any dependencies, but I think it makes sense to count cgo as one, and a heavy one at that. Cross-compiling C is more much annoying than Go, one of the best ways to do it right now is to use something like xgo, which relies on 2 GB Docker image (which expands to 6 GB!).

If you want to remove the stty dependency, which makes sense, I think using another Go module would be a much more maintable and easy solution.

I think cgo is probably the wrong decision. So far Bombadillo has avoided adding any dependencies, but I think it makes sense to count cgo as one, and a heavy one at that. Cross-compiling C is more much annoying than Go, one of the best ways to do it right now is to use something like [xgo](https://github.com/techknowlogick/xgo), which relies on 2 GB Docker image (which expands to 6 GB!). If you want to remove the `stty` dependency, which makes sense, I think using another Go module would be a much more maintable and easy solution.

👍 1

dancek commented

2020-05-23 17:18:05 +00:00

The build is just make build, ie. when building for the current platform the Go compiler seems to automatically run cgo when needed.

Replacing tput will probably require cgo or a different approach altogether (ie. 3.x and a completely rebuilt UI based on some library).

If cross-compiling is important and xgo works, I can't really see disk space usage as a huge issue. If xgo is a no-go (pun intended) I think it's possible to have the cgo implementation with // +build cgo and an stty+tput based fallback with // +build !cgo.

The build is just `make build`, ie. when building for the current platform the Go compiler seems to automatically run `cgo` when needed. Replacing `tput` will probably require `cgo` or a different approach altogether (ie. 3.x and a completely rebuilt UI based on some library). If cross-compiling is important and xgo works, I can't really see disk space usage as a huge issue. If xgo is a no-go (pun intended) I think it's possible to have the cgo implementation with `// +build cgo` and an `stty`+`tput` based fallback with `// +build !cgo`.

makeworld commented

2020-05-23 17:27:24 +00:00

First-time contributor

I would say that cross-compiling is important for any Go project, but that's just me.

I brought up the disk space, not because 6 GB is too big for developers or anything, but just because I wanted to make a point about how if you want to be able cross-compile, cgo is itself a large dependency, in comparison to just adding Go module.

I would say that cross-compiling is important for any Go project, but that's just me. I brought up the disk space, not because 6 GB is too big for developers or anything, but just because I wanted to make a point about how if you want to be able cross-compile, cgo is itself a large dependency, in comparison to just adding Go module.

sloum commented

2020-05-23 17:29:06 +00:00

Thank you for the input @makeworld , I am leaning toward thinking you are right. I think complicating the build process is a really big concern that should be taken seriously and CGO will definitely do that.

Dancek was a ble to get a version with syscalls working for OSX. If we can figure this out to make it work on Linux as well (something that is seeming less and less likely the more I research it), then this could be a solution.

I just tried out their branch for the Darwin build without stty. It does seem to work. Though I am experiencing a few glitches:

Cursor hiding is no longer working so I see the cursor in the bottom right (and sometimes it shoots around to other places)
It works great for single character input. However, when in line mode it works until I hit enter (like when entering an address). It then spits out what looks like the print version of a struct and then performs whatever action is desired. This seems like something that should be easy to fix.

Some initial reading seems to imply that the syscall library does not make TCGETA or TCSETAF available on Linux. However, a bit of digging uncovered this. The link has a querystring for the architacture in question. When I get a chance today (I am not on my mac rather than my main comp, so I'll do it when I get a spare moment in the other room) I'll try just hard coding the const value for TCGETA out of that package and see if it will work as a drop in replacement... I'm not sure how usable that will be in the long run, but it may work to at least get a POC going for linux.

Thank you for the input @makeworld , I am leaning toward thinking you are right. I think complicating the build process is a really big concern that should be taken seriously and CGO will definitely do that. Dancek was a ble to get a version with syscalls working for OSX. If we can figure this out to make it work on Linux as well (something that is seeming less and less likely the more I research it), then this could be a solution. I just tried out their branch for the Darwin build without stty. It _does_ seem to work. Though I am experiencing a few glitches: - Cursor hiding is no longer working so I see the cursor in the bottom right (and sometimes it shoots around to other places) - It works great for single character input. However, when in line mode it works until I hit enter (like when entering an address). It then spits out what looks like the print version of a struct and then performs whatever action is desired. This seems like something that should be easy to fix. Some initial reading seems to imply that the syscall library does not make `TCGETA` or `TCSETAF` available on Linux. However, a bit of digging uncovered [this](https://godoc.org/golang.org/x/sys/unix?GOOS=linux&GOARCH=amd64). The link has a querystring for the architacture in question. When I get a chance today (I am not on my mac rather than my main comp, so I'll do it when I get a spare moment in the other room) I'll try just hard coding the const value for `TCGETA` out of that package and see if it will work as a drop in replacement... I'm not sure how usable that will be in the long run, but it may work to at least get a POC going for linux.

👍 1

sloum commented

2020-05-23 23:27:21 +00:00

Good news! The syscall route has paid off. A pure Go solution has been built as a part of #155 and it works on both Darwin and Linux without having to resort to hard coding in values (everythig turned out to be available via syscall, it was just tricky to find and not particularly well documented).

As such, I am going to close this PR. I think this PR though will be a great reference point if we ever do end up needing to do anything with CGO.

Good news! The `syscall` route has paid off. A pure Go solution has been built as a part of #155 and it works on both Darwin and Linux without having to resort to hard coding in values (everythig turned out to be available via `syscall`, it was just tricky to find and not particularly well documented). As such, I am going to close this PR. I think this PR though will be a great reference point if we ever do end up needing to do anything with CGO.

sloum closed this pull request

2020-05-23 23:27:27 +00:00

dancek referenced this pull request

2020-05-24 06:29:07 +00:00

Replace calls to `stty` with syscalls #158