johnnovak / illwill

A curses inspired simple cross-platform console library for Nim
Do What The F*ck You Want To Public License
398 stars 27 forks source link

Added JS support #13

Closed JohnAD closed 3 years ago

JohnAD commented 4 years ago

This is a work in progress, please do not merge until the [WIP] prefix has been removed.

This is my second pass at adding Javascript (JS) support. This attempt will properly use the TerminalBuffer storage.

This first commit is simply a refactoring to prepare for the new code. In fact, it temporarily breaks windows support as I've only refactored for posix so far. I'll correct that fairly soon.

JohnAD commented 4 years ago

I brought a windows laptop home for the weekend. I'll get windows compilation working again soon; probably tomorrow. So far, everything is going very cleanly.

There is a odd problem with cursor positioning in posix. The cursor moves to the right spot from buffer point of view. But the visible cursor seems to be "off by one" sometimes. I'll try to fix that also.

JohnAD commented 4 years ago

Style is mostly working. All that is left for me to do is fix up styleReverse and update the documentation.

image

vs what is seen in a bash terminal:

image

JohnAD commented 4 years ago

Almost done. I need to document a few extra things. And, I will still want to fix that showCursor bug that sometimes appears.

JohnAD commented 4 years ago

Done! This PR is ready for review. Thanks!

JohnAD commented 4 years ago

I added a example program called "badtictactoe.nim" to demonstrate the timerLoop and user-input routines more fully.

As seen in a posix terminal:

image

As seen in a web broswer:

image

enthus1ast commented 4 years ago

This looks awesome @JohnAD ! I'm eager to try it! Is mouse supported?

JohnAD commented 4 years ago

Thanks!

Mouse support is not in yet. There are, in fact, two more things I'd like to add:

  1. Mouse support; tested against the main broswers (Chrome/Firefox/Edge/Safari/etc.)
  2. A mechanism for "Enter" to work. By default, a web browser treats a user pressing enter as a message to "submit the form". So, the Enter key is never seen. One work-around is using Ctrl-Enter instead. That already works. (Reminder to self: document this.) But another method is to build a javascript method of translating a client-side form submission has an Enter key. I've got some experimenting to do.

But, I'm refraining for now. I don't want to add further complication without this first PR being reviewed/accepted. I'd rather not build further on a structure that might need to be changed on review.

JohnAD commented 4 years ago

(BTW, I probably won't get to it this year, but I'd like to also add support for Android in-screen terminals via the nim Godot bindings. That usage would also be event-driven like JS. But, one thing at a time!)

JohnAD commented 4 years ago

Does anyone have a method of contacting @johnnovak ? I've not heard from him yet regarding this PR. I'm happy to be patient, but I just want to make sure it is on someones todo list.

0xACE commented 4 years ago

I've got no idea of how often he checks on here. But luckily git commits contains an e-mail address, you could e-mail him and check that way git > github ;-)

My guess would be that this is a rather large branch to test, requiring more effort than just merging it blindly :).

johnnovak commented 4 years ago

@JohnAD Sorry, I should check here more often. I'm a bit busy now but I will review this in a week or two. Seems like you've done some great work here, that's good! The screenshots of the tic-tac-toe game seem very nice!

So, I just want to reiterate, this library was originally meant for terminal output. I'm not against adding JS support, as long as it's not too invasive and does not overcomplicate the code and the API. Just be forewarned, I'll review the PR from this perspective 😄 Also, I don't want to cause any complications or additional complexities for users who simply don't care about supporting the browser. So, in my mind, the JS support should be a completely non-invasive "optional add-on".

But yeah, great work otherwise!

johnnovak commented 4 years ago

My guess would be that this is a rather large branch to test, requiring more effort than just merging it blindly :)

I'm a bit afraid of this too, hence I put the disclaimer into my previous comment. We'll see how it goes.

The general direction seems to be good and that's how I would have done it (separating the backends into 3 files), so that's good so far 👍

enthus1ast commented 4 years ago

@JohnAD , was able to try it finally, really nice 👍

johnnovak commented 3 years ago

Ok, started reviewing. First of all, the output doesn't look quite alright in the browser on OS X. Tried both Firefox and Chrome, same results.

On Windows there's no weird white lines in the output.

Another comment: the white foreground on light blue (cyan?) background makes things hard to see in the Tic Tac Toe game. Can you please make it more contrasty? (e.g. by changing the foreground colour to black)

image image
johnnovak commented 3 years ago

forminput.nim doesn't work in the browser:

image

Works in the terminal, but please use a better bg/fg colour combination for the input boxes, and get rid of the exit debug message.

image

johnnovak commented 3 years ago

@JohnAD First of all, thanks for this, there's lots of things that I like in your work. For example, separating the backends into different files was a good idea and it has been long overdue. I also applaud the effort to create some kind of a simple text input box.

I'm not totally conviced that adding a JS backend is useful, though. It complicates the API, e.g. you needed to introduce timerLoop to make it work. Realistically, who would want to write a console app and then want it to run it in the browser too? Seems very unlikely to me. Can you think of any use cases? I can't, because I personally would rather write a backend that contains just the processing, logic, algorithms and whatnot, then different frontends for it: one for the console using illwill, a completely new browser based UI, then a completely different graphical UI, etc. You know, probably no one would want to simulate the console in OpenGL or something either--they would just write a brand new graphical UI that's much better for their app.

I'm personally not interested in maintaining the JS backend at all, and unless there's a very strong argument for including it, I don't think I want to get it merged in.

Either way, I think the rest of the work minus the JS backend should probably go in.

Anyway, here's a list of further notes and comments, from which you can already see there's a number of issues introduced by timerLoop, which was necessitated by the JS backend. And we can't run the existing examples in the browser either without further modifications...

  1. timerLoop (which should be eventLoop) doesn't handle resizing the terminal window. Run some of the existing examples and resize the terminal window and see what happens, then repeat with the new examples that use timerLoop. Basically, you should just create a new terminal buffer in each loop iteration in timerLoop with the current dimensions of the terminal, and then redraw everything into it.

  2. badtictactoe: You're kind of assuming a 90x24 terminal window, but you can't make such assumptions. At the very least, you should make sure that if the terminal window is smaller, the output is clipped to the visible area. For bonus points you could detect that the terminal is smaller than 90x24 and display a message like "This example needs a 90x24 terminal window (or larger), please resize your terminal window."

  3. styleddisplay: Clean exit must be implemented, just like in the existing examples.

  4. I'm not a fan of dumping all the source files into the root folder. Then you have illwill.nim, and common.nim in addition to that. I suggest the following structure as the current best-practice:

illwill.nim     - contains just the exports, lots of stuff can be moved into common
illwill/
  common.nim
  js.nim        - just lose _terminal from the backend names
  posix.nim
  windows.nim
  1. There's lots of typos and sentences with strange wording in the doco. I gave up commenting on all of these after a while, it would be easier to just rewrite it myself... Please read through everything carefully a few times and try to improve it. Also please try to be succint.
JohnAD commented 3 years ago

Thanks for all the feedback! I'm hoping to work on it again next weekend.

codewski commented 3 years ago

@JohnAD @johnnovak is this gonna merge or is abandoned? I'd like to use illwill for a project. Thanks in advance.

JohnAD commented 3 years ago

@codewski Unfortunately, I've gotten far busier, so I'm not sure when I'll be able to work on it again. Ironically, it is not far from finished.

Myself, I'm using my local fork for a few projects.

Feel free to fork my repo: https://github.com/JohnAD/illwill

And, if you end up finishing it, you can either make a new PR to merge it with Illwill. Or you can rename it into a new library for nimble. johnnovak was not looking forward to supporting JS output, so I suspect he would be okay with a js-oriented version based on his workk.

johnnovak commented 3 years ago

johnnovak was not looking forward to supporting JS output, so I suspect he would be okay with a js-oriented version based on his work.

That's right, this is meant to be used in "proper" terminals, so JS output won't be added as it complicates everything a bit too much for my taste for such a niche use-case. But of course, feel free to fork it and do whatever 😄