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

feat: add Java API for setting insertMode #21

Merged
merged 2 commits into from
Oct 19, 2021
Merged

feat: add Java API for setting insertMode #21

merged 2 commits into from
Oct 19, 2021

Conversation

javier-godoy
Copy link
Member

The use of hardcoded DECSCUSR sequences (#19) is not correct, but that's a different issue.

@javier-godoy javier-godoy requested a review from mlopezFC October 13, 2021 19:57
Copy link
Member

@mlopezFC mlopezFC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of small extra spaces to remove, but the rest LGTM

@@ -54,7 +55,7 @@ class ConsoleAddon extends TerminalAddon<IConsoleMixin> {
this.cursorBackward({params:[1]});
return true;
} else if (buffer.lines.get(buffer.y+buffer.ybase).isWrapped) {
this.cursorPrecedingLine({params:[1]});
this.cursorPrecedingLine({params:[1]});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we discard this unneeded change?

@@ -185,13 +181,27 @@ type Constructor<T = {}> = new (...args: any[]) => T;
export function XTermConsoleMixin<TBase extends Constructor<TerminalMixin>>(Base: TBase) {
return class XTermConsoleMixin extends Base implements IConsoleMixin {
escapeEnabled: Boolean;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And also this one

@javier-godoy javier-godoy requested a review from mlopezFC October 18, 2021 19:37
@javier-godoy javier-godoy changed the title Feature/17 feat: add Java API for setting insertMode Oct 18, 2021
Copy link
Member

@mlopezFC mlopezFC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mlopezFC mlopezFC merged commit 915a717 into master Oct 19, 2021
@mlopezFC mlopezFC deleted the feature/17 branch October 19, 2021 21:26
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 this pull request may close these issues.

2 participants