Skip to content
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

Consider reverting cursor::MoveToColumn(0) non-zero handling or changing it? #683

Closed
dsherret opened this issue Jun 29, 2022 · 8 comments · Fixed by #684
Closed

Consider reverting cursor::MoveToColumn(0) non-zero handling or changing it? #683

dsherret opened this issue Jun 29, 2022 · 8 comments · Fixed by #684

Comments

@dsherret
Copy link
Contributor

dsherret commented Jun 29, 2022

Is your feature request related to a problem? Please describe.

I was using MoveToColumn(0) because I assumed it was 0-indexed, but in #627 it was changed to be silently ignored so the output of my code started acting funny when I upgraded crossterm.

It seems better to just let 0 be the same as 1 for MoveToColumn? I think some other people might also get confused why it's not working as well and blame crossterm on being buggy.

Describe the solution you'd like

MoveToColumn(0) is equivalent to 1 or perhaps panic (maybe only panic in debug)? Alternatively, maybe this should only accept a NonZeroU16, though that api might not be so nice. 0 equivalent to 1 seems most ideal.

I can understand how this current functionality is desired for what's described in #626, but not so much for this.

I guess also MoveToRow(0) too actually.

@Johann150
Copy link

I think it would make more sense if they were zero indexed because MoveTo and cursor::position are zero indexed, and this is documented. The phrasing of the documentation lead me to believe that the statement was true for the whole API, which is confusing:

Top left cell is represented as 0,0.
-- https://docs.rs/crossterm/0.23.2/crossterm/cursor/struct.MoveTo.html#notes

@Johann150
Copy link

Duplicate of #632

@TimonPost
Copy link
Member

I see there is correct confusing how the API operates and in being consistent. The aim is to be 0-based but ANSI codes are often 1-based. MoveTo internally increments the passed parameters to conform that.

image

I think this is indeed something to be reverted. We should then just increment the parameter by one and accept 0 parametes everywere, in all cursor functions.

@TimonPost
Copy link
Member

TimonPost commented Jun 30, 2022

Previous time I thought that it was confusing having Move Left by 0 moves left by 1. But if you see it like: Move left to the zeroth cell then it makes more sense.

@Johann150
Copy link

If I'm not mistaken, MoveLeft is a relative move. As such I would expect it to do nothing when passed zero. But on the other hand I would expect absolute moves such as MoveTo, MoveToColumn etc. to be zero-indexed.

So in my mind MoveLeft(0) would do nothing, MoveToColumn(0) would have the same effect as a carriage return, i.e. move to the leftmost column.

@TimonPost
Copy link
Member

TimonPost commented Jun 30, 2022

So the question becomes do we want all cursor functions to be consistent aka 0-based. Or does it make sense for some functions like moving left/right to be 1-based? But that we properly document it.

@Johann150
Copy link

For me it does not make sense to adjust relative moves in this way. Something like "go two steps left" always has the same meaning, no matter if the coordinate system starts at 0,0 or 1,1. I would be very confused if I say "go two steps left" and the cursor goes three steps left.

Either way, documenting it properly would be good.

@TimonPost
Copy link
Member

Yes that was my previous thought as well. Lets do it like that, thus 1 is really moving 1. And document that this is the case rather than being consistent with moveto commands that people were confused by.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants