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

Create Cursor class #2290

Closed
jerch opened this issue Jul 6, 2019 · 4 comments
Closed

Create Cursor class #2290

jerch opened this issue Jul 6, 2019 · 4 comments
Assignees
Labels
type/debt Technical debt that could slow us down in the long run

Comments

@jerch
Copy link
Member

jerch commented Jul 6, 2019

Currently our cursor handling is scattered over several classes:

  • x/y values live in Buffer
  • positioning / movements happen in InputHandler
  • position is requested at various places with repeated sanity checks like if (buffer.x >= terminal.cols)

Proposal:

Lets unify the cursor handling with an explicit class holding the position and doing proper range checks during re-positioning.

Stub:

interface ICursor {
  readonly x: number;
  readonly y: number;
  readonly cols: number;  // maybe private or directly inherited from Buffer
  readonly rows: number;  // maybe private or directly inherited from Buffer
  move(x: number, y: number): void;  // relative movements
  set(x: number, y: number): void;   // absolute positions
}

For set and move the class can automatically enforce to stay within cols/rows limits. These limits could auto-update by terminal resize and such.

@jerch jerch added the type/proposal A proposal that needs some discussion before proceeding label Jul 6, 2019
@mofux
Copy link
Contributor

mofux commented Jul 8, 2019

Will a cursor instance then become part of a buffer instance? E.g. term.buffer.cursor.move(1,1)?

@Tyriar
Copy link
Member

Tyriar commented Jul 8, 2019

@mofux I don't think the API surface will change, we want people to adjust the cursor position using term.write instead of directly moving the cursor.

@jerch jerch self-assigned this Jul 13, 2019
@jerch
Copy link
Member Author

jerch commented Aug 4, 2019

note to myself:
Seems relative cursor positioning always has to respect the scroll borders and should not pass the area even if DECOM is disabled. (See test file t0075-DECSTBM_CUU_CUD.in and workaround in
0029853)
Might also apply to left/right borders once supported.

@Tyriar Tyriar added type/debt Technical debt that could slow us down in the long run and removed type/proposal A proposal that needs some discussion before proceeding labels Oct 7, 2019
@Tyriar
Copy link
Member

Tyriar commented Oct 12, 2021

Closing as it probably won't end up happening anyway, cursor is currently managed in buffer.ts and doesn't seem too bad atm.

@Tyriar Tyriar closed this as completed Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/debt Technical debt that could slow us down in the long run
Projects
None yet
Development

No branches or pull requests

3 participants