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.

Sign up for GitHub

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

Working on fixing Issue #3293 #5137

Closed
wants to merge 1 commit into from
+55 −0
Closed

Working on fixing Issue #3293 #5137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .hintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"extends": [
"development"
],
"hints": {
"no-inline-styles": "off"
}
}
5 changes: 5 additions & 0 deletions src/browser/public/Terminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ export class Terminal extends Disposable implements ITerminalApi {
this._core = this._register(new TerminalCore(options));
this._addonManager = this._register(new AddonManager());

// Store user's cursor style
this._core.options.userCursorStyle = this._core.options.cursorStyle;



this._publicOptions = { ... this._core.options };
const getter = (propName: string): any => {
return this._core.options[propName];
Expand Down
4 changes: 4 additions & 0 deletions src/common/InputHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2734,6 +2734,10 @@ export class InputHandler extends Disposable implements IInputHandler {
public setCursorStyle(params: IParams): boolean {
const param = params.params[0] || 1;
switch (param) {
case 0:
console.log('Case 0 triggerd');
this._optionsService.options.cursorStyle = this._optionsService.options.userCursorStyle || 'block';
break;
Comment on lines +2737 to +2740
Copy link
Member

Choose a reason for hiding this comment

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

I think the approach you're taking it likely to cause problems (maybe what's causing the issue you mention now). Instead of trying to replace this._optionsService.options.cursorStyle or add a new this._optionsService.options.userCursorStyle to this._optionsService.options which is an object created by the user and not meant to be touched by us, we would want some way to setting an override which starts out at undefined. Makes sense to put this override in IDecPrivateModes as mentioned in #3293 (comment)

Then when we go to use the actual cursor style from the renderer, we would use devPrivateModes.cursorStyle ?? this._optionsService.options.cursorStyle ?? 'block'

Copy link
Author

Choose a reason for hiding this comment

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

I think the approach you're taking it likely to cause problems (maybe what's causing the issue you mention now). Instead of trying to replace this._optionsService.options.cursorStyle or add a new this._optionsService.options.userCursorStyle to this._optionsService.options which is an object created by the user and not meant to be touched by us, we would want some way to setting an override which starts out at undefined. Makes sense to put this override in IDecPrivateModes as mentioned in #3293 (comment)

Then when we go to use the actual cursor style from the renderer, we would use devPrivateModes.cursorStyle ?? this._optionsService.options.cursorStyle ?? 'block'

That makes sense, I will do this and get back to you, give me a while, thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Hi @Tyriar, wanted to update you that I got some academic commitments mid-way so paused work on this.
I am continuing working on this, will keep you updated.

Once again, thanks for your support!

case 1:
case 2:
this._optionsService.options.cursorStyle = 'block';
Expand Down
38 changes: 38 additions & 0 deletions test_file.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>xterm.js Test</title>
<script src="./node_modules/@xterm/xterm/lib/xterm.js"></script>
<link rel="stylesheet" href="./css/xterm.css">
<script>
console.log('Script is running');
const terminal = new Terminal({
cursorStyle: 'underline', // Custom user-defined cursor style
});

const userCursorStyle = terminal.options.cursorStyle;
console.log('User Cursor Style:', userCursorStyle); // Print the value of userCursorStyle

document.addEventListener('DOMContentLoaded', function() {
console.log('DOM fully loaded and parsed');
terminal.open(document.getElementById('terminal'));
console.log('Sending control sequence as given 1');
terminal.write('\x1b[5 q');
console.log('User Cursor Style:', userCursorStyle);
console.log('Current Style:', terminal.options.cursorStyle);

terminal.focus();
console.log('Sending control sequence as given 2');
terminal.write('\x1b[0 q');
console.log('User Cursor Style:', userCursorStyle);
console.log('Current cursor style', terminal.options.cursorStyle)
terminal.focus();
});
</script>
</head>
<body>
<div id="terminal" style="height: 100%; width: 100%;"></div>
</body>
</html>