Enabling terminal dimension feature #16
Conversation
| /** Functionality for getting the dimensions of the terminal */ | ||
| trait Dimensions { | ||
| object dimensions { | ||
| def getDimensions: effect.Dimensions ?=> Unit = effect ?=> |
There was a problem hiding this comment.
If this is wrapped in an object named dimensions, we don't need to name the method getDimensions. Just get is fine.
Alternatively it could just be getDimensions without the wrapping object. This is not a method that is going to have a name collision, unlike the methods for setting color and moving the cursor.
| @@ -47,6 +46,15 @@ class JLineTerminal(terminal: JTerminal) extends Terminal { | |||
| } | |||
| } | |||
|
|
|||
| // Do we need to consider getBufferSize? | |||
There was a problem hiding this comment.
I have no idea what most of these methods do. The JLine documentation is, unfortunately, not very good.
There was a problem hiding this comment.
It isn't, but I followed the docs and it leads you to this.
TLDR, the getSize method should be used in full screen mode, and getBufferSize otherwise.
JLine doesn't have the ability to get whether it's full screen mode, so we could create another method that tries to determine whether the users terminal is in full screen mode or not. Terminus Users could have use for this.
Or, we could keep the logic local to this function
There was a problem hiding this comment.
It might be worth running some experiments to what the value of the two methods is. I think for most cases getSize will do the job. I'm not sure it's possible to get things working perfectly across all terminals, but the terminals for which it won't work will be fairly obscure.
Alternatively, this has a section on getting the terminal size: https://viewsourcecode.org/snaptoken/kilo/03.rawInputAndOutput.html Reading the JLine source code might help understand which method they are using.
There was a problem hiding this comment.
From the terminals I have at my disposal, I couldn't see any difference in output of the two. So have just used the standard method here
Lgoldfinch
force-pushed
the
addTerminalDimensions
branch
from
0c1509a to
263ac4c
Compare
Lgoldfinch
force-pushed
the
addTerminalDimensions
branch
from
2e03812 to
d94dd39
Compare
|
|
||
| final case class TerminalDimensions(noOfColumns: Int, noOfRows: Int) | ||
|
|
||
| //object TerminalDimensions { |
There was a problem hiding this comment.
The project didn't compile when I included this. But I did want to include some way to indicate that TerminalDimensions and Size are linked. Alternatively, I could add an Iso instance to handle the mapping.
Conversely, you might not see any value in this anyway! So happy to omit
| def setDimensions(dimensions: TerminalDimensions): Unit | ||
| } | ||
|
|
||
| final case class TerminalDimensions(noOfColumns: Int, noOfRows: Int) |
There was a problem hiding this comment.
Do you want me to add opaque types for this?
Lgoldfinch
force-pushed
the
addTerminalDimensions
branch
6 times, most recently
from
5d7fcd0 to
6ed9c9a
Compare
* Get terminal dimensions (creativescala#13) * Set terminal dimensions (creativescala#12)
Lgoldfinch
force-pushed
the
addTerminalDimensions
branch
from
6ed9c9a to
aec3251
Compare
This will only work for terminals that support
JLine.I added the set terminal size feature to enable testing for this issue.