search

cpjreynolds / rustty

A terminal UI library
https://docs.rs/rustty
MIT License
152 stars 14 forks source link

Fix/simplification of cursor positioning #17

Closed jbschlosser-zz closed 8 years ago

jbschlosser-zz commented 9 years ago

I was experiencing weird behavior with cursor positioning, so I investigated and found that the system of tracking valid/invalid cursor coordinates was not functioning correctly when sending characters. I removed that system entirely and switched to the simplified form of just keeping a tuple for current cursor position. To fix my issue, I rewrote the character sending logic to:

  1. Move the cursor temporarily to the desired character coordinate.
  2. Write the character.
  3. Move the cursor back to its previous location.

With this in place, my issues are gone. I'm open to any alternatives that still address my issue; this just seemed like the simplest solution.

ghost commented 9 years ago

If you look at PR #15, you'll see that there is already such a simplification proposed. It's not as drastic as yours here, but I'm not sure that getting rid of the Position type altogether is such a good idea. It complexifies the signature of every function taking a position as an argument and will necessitate a lot of search&replace for all API users if we ever change our mind about a position being represented with usize.

jbschlosser-zz commented 9 years ago

Ah sorry, I didn't see that proposal at first. Even though I didn't modify any user-facing API calls, I do agree that using a naked tuple for cursor position isn't ideal. I'll change that to use a Position struct.

cpjreynolds commented 9 years ago

Looks good to me. Thanks for the PR!

cpjreynolds commented 9 years ago

Darn it, I didn't see the conflict there. I'm on my phone right now so I can't be of much help, but I'd love to get this merged today!

jbschlosser-zz commented 9 years ago

Got it merged and it should be good to go!

ghost commented 9 years ago

Just to be sure: we're losing two concepts here.

  1. The concept of remembering the last position before the last cursor pos set.
  2. The ability to have a None cursor pos.

It doesn't seem to me like it's a big deal to lose them (that is, it seems to me that it has no impact whatsoever on the way the library works), but maybe the @cpjreynolds would know better of the implications of those two losses.

cpjreynolds commented 9 years ago

@hsoft I may be wrong, it has been a few months since I wrote it, however I am fairly sure the last cursor position was stored to optimize some cursor movement, since if we know the current cursor position is sequential to the last, we won't need to emit an escape sequence to re-position. I'll take a closer look at this to be sure.

cpjreynolds commented 8 years ago

I'd like to apologize for essentially abandoning this project for so long, personal issues came up and it has been a rough year to say the least. That being said, the college application nightmare is over and I am going to have all summer to be able to work on this project! I'd like to start back off with a clean slate so I'm going to close this PR for now, however if you would like to revisit this, please do not hesitate to open another PR in the future!