Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Garak (hot reloading / REPL) #17

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft

Conversation

awfulcooking
Copy link

@awfulcooking awfulcooking commented Apr 28, 2023

This project is named Taylor, which made me think of Garak, the Tailor, from Deep Space Nine.

Anyhow.. I liked your work and wanted to quickly iterate on it as a base for my own stuff.

I started writing this module which implements a CLI REPL, and hot reloading.

It's intended to be used as an entry point (e.g. $ taylor garak.rb) and renders exceptions on screen until you save the file again, which reloads it.

Attaching commits from #16 here. This was developed on top of them, but I think only one or two are necessary.

This PR is to share the idea / offer the implementation, but I don't have the bandwidth to rebase it just now.

I'll try to separate it later.

It implements MawJump, with REPL code adapted from a previous implementation on DragonRuby.

Presenting as an RFC / sample. Cheers.

This was referenced Apr 28, 2023
@Salvakiya
Copy link

This is great to see!

- Doesn't need ".rb" suffix on filenames, like normal Ruby
- Tracks loaded files in $" array, useful for code reloading
Getting conflicting symbol error on Linux, Windows and Darwin too.
Worked around the conflicting symbol issue from mruby-uri-parser
`configure` no longer autodetects when we want to cross-compile, instead, --host / --build must be passed.
Next, it was figuring an incorrect path for the `ar` binary. So we set conf.archiver.command.
- Using `rake docker:build:mruby`
- This has updated mruby to version 3.2
Changes have been applied to scripts/mruby/*.rb now
@HellRok HellRok force-pushed the main branch 6 times, most recently from 6bfeb18 to 4590da2 Compare June 11, 2023 05:05
@Salvakiya
Copy link

how is this one coming along?

@awfulcooking
Copy link
Author

@HellRok would you like to work on this synchronously together?

Over Discord, or VS Code, for instance.

That's probably the easiest way to merge this 🙂

Here are some commits that might be easy to cherry pick:

That would help get a smaller overview of the "Garak" part and help to proceed

Then we could go from here or there 🙂

Don't worry if timing isn't convenient. Just showing signs of life, lol

@HellRok
Copy link
Owner

HellRok commented Oct 15, 2023

@awfulcooking I finally have some time to reply to this!

default_font method

I'd like this change but it will require tests and documentation

Switch to iij/mruby-require

I won't be accepting this change as iij/mruby-require is fairly out of date and unmaintained. I'm not sure what you mean by "Doesn't require .rb suffix on file names", do you mean the actual file names on the disk? If so I don't think that's something I'm interested in supporting as it would lead to a pretty confusing local dev setup.

Make alpha parameter to Colour.new() optional

This is a great change I'd love in Taylor but it will require tests and documentation.

Add constant Color = Colour (alias)

I will not be accepting this change as it's very trivial for anyone who wants it to add to their own project.

Add Colour[] and Rectangle[] methods

This is an excellent idea I'd love in Taylor but it will require tests and documentation

Set $0 global variable with script name

This is an excellent idea I'd love in Taylor but it will require tests and documentation


Other changes I'd be interested in seeing pulled out would be

get_collision_rec and check_collision_recs

These look like useful methods, but will require tests and documentation too.


Something I'd like to let you know is that I'm not planning on merging anything relating to 3D until after Taylor 1.0 releases (which should be April 2024). This is because I'm going to be doing some pretty aggressive refactoring soon to clear up the top level namespace and I want the smallest possible implementation for that. My advice to you for this would be to have two branches, one with the hot-reloading and one off of that with the 3D additions. This way we could merge the hot-reloading without affecting your 3D changes.

I did also peek at the hot-reloading implementation and noticed that it uses a lot of ANSI escape codes but without platform detection, I'm wondering how that all looks on Windows?

Plan of Action

Alright, I've talked your ear off enough for now. Let's get down to what actually matters. How do we move forward?

If you'd like to create separate merge requests for each of the features above that would be fantastic, but as stated they will need tests and documentation. If you're not happy to do that I can cherry pick your commits and add them for you, but I think it would be good experience for you to do, it'd also set a really good example for others going forward.

If you'd like to do some pair programming we can try to organise that, but keep in mind I operate currently in GMT+10:30.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants