blessed-xterm icon indicating copy to clipboard operation
blessed-xterm copied to clipboard

Published 20 hours ago • verified publisher icon rse

Blessed xterm fixes

Open johnpoth opened this issue 7 years ago • 4 comments

In this PR I propose to

  1. remove the cloning of the options as it may result in errors when using the parent:screen option.
  2. make blessed-xterm work in the browser as the global.window is read only.

Thanks!

johnpoth avatar Jan 16 '18 17:01 johnpoth

In my code comment for the clone() usage, I wrote: "clone options or all widget instances will show at least the same style, etc" How do you think this issue is now adresses by just removing the clone() call? I do not see what you are doing instead?

rse avatar Jan 16 '18 22:01 rse

Hi @rse , thanks for reviewing! I saw the comment. Could you please elaborate the problem of widgets having the same style? I tried reproducing using the sample.js file by applying different styles to the two terminals and it worked without the clone() call. Thanks!

johnpoth avatar Jan 17 '18 09:01 johnpoth

I've currently no time to reproduce, but AFAIK the issue was that the style of a window being rendered active, or scrolling (in visual mode) or in error state was broken without this. One of those three states was not rendered correctly AFAIK.

rse avatar Jan 17 '18 19:01 rse

@rse no worries I'll try to reproduce on my side too.

From a design standpoint I don't think it should be needed to clone the options. So if it is needed I think it would be more of a bug in blessed/blessed-xterm. My 2 cents ofc :)

johnpoth avatar Jan 18 '18 09:01 johnpoth