-
Notifications
You must be signed in to change notification settings - Fork 37
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.
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
Add a detection sample #126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea on making TerminalInfo
printable. There's just a few changes we should make.
@@ -4,13 +4,13 @@ import com.github.ajalt.mordant.internal.* | |||
import com.github.ajalt.mordant.rendering.AnsiLevel | |||
import com.github.ajalt.mordant.rendering.AnsiLevel.* | |||
|
|||
internal object TerminalDetection { | |||
object TerminalDetection { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's any reason to make this public. Instead of TerminalDetection.detectTerminal()
, folks can can the same object from Terminal().info
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out Terminal().info
, that's much nicer, and I've dropped this commit from the PR.
/** The terminal width, in cells */ | ||
width: Int, | ||
var width: Int, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a change in API, but I don't see any harm in it, and I like using the data class for toString
.
|
||
fun main() { | ||
val terminal = Terminal() | ||
terminal.println(TerminalDetection.detectTerminal()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
terminal.println(TerminalDetection.detectTerminal()) | |
terminal.println(terminal.info) |
This allows to easily compare instances and benefit from an auto-generated `toString()` representation. The downside is that `width` and `height` could now be set from the outside after construction, but that is probably not too much of an issue as arbitrary values could also have been passed via the constructor initially anyway.
2ab2f46
to
254d2a4
Compare
254d2a4
to
046ad77
Compare
Later on printing of selected environment variables or similar useful output could be added.
046ad77
to
8b43936
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it, thanks!
Please have a look at the individual commit messages for the details.