85% of URL #564
85% of URL #564
Conversation
lacci/lib/shoes/app.rb
Outdated
| private | ||
|
|
||
| def render_index_if_defined_on_first_boot | ||
| return if $first_boot_finished |
There was a problem hiding this comment.
Maybe a global here is too gnarly and an ivar will do
There was a problem hiding this comment.
Yeah, ivar would be plenty.
| return if @do_shutdown | ||
|
|
||
| with_slot(@document_root) do | ||
| @content_container = flow(width: 1.0, height: 1.0) | ||
| with_slot(@content_container, &@app_code_body) | ||
| end | ||
|
|
||
| render_index_if_defined_on_first_boot |
There was a problem hiding this comment.
I tried a few homes for this, which seemed to break and not make sense. Hopefully this is the best home
There was a problem hiding this comment.
This looks reasonable to me on first blush.
|
Will review this soon - got a friend visiting and we're off to get him a variety of whisky :-) |
lacci/lib/shoes/app.rb
Outdated
| begin | ||
| # I'm not finding a global_variable_get or similar... | ||
| global_value = eval s | ||
| drawables &= [global_value] | ||
| rescue | ||
| #raise Shoes::Errors::InvalidAttributeValueError, "Error getting global variable: #{spec.inspect}" | ||
| rescue StandardError |
There was a problem hiding this comment.
Rescuing StandardError is the default. Any reason to mention it explicitly?
There was a problem hiding this comment.
that was an autoformatter rule for rescuing without specifying what we're rescuing. but if you don't say anything it reverts explicitly to the default. i have no opinion on this so i've reverted. and my autoformatter is now off
| @@ -181,34 +185,33 @@ def current_draw_context | |||
| # want to (and/or can't) take control of the event loop. | |||
| def run | |||
| if @do_shutdown | |||
| $stderr.puts "Destroy has already been signaled, but we just called Shoes::App.run!" | |||
| warn 'Destroy has already been signaled, but we just called Shoes::App.run!' | |||
There was a problem hiding this comment.
Now I wonder if we ever shut down the logfile stuff. Because warn won't appear if logfile stuff isn't currently working. But this should only get called after the logs are initialised, so definitely no startup problem.
Description
I'm SO CLOSE to getting all the way there. But there is a fair enough chunk of this to ship the current utility.
You define URLs atop the file. You can then "visit" them (this is patched on top of existing
pagefunctionality). The REALLY neat thing is you can pass in parameters like integers and strings to make the page render DIFFERENT things. (Think like a controller!). This was part of original shoes urls. Very fun. Very exciting.Also I stumbled upon an implicit shoes convention and implemented it. If all of your app code is boxed into methods for your urls, it defaults to
indexif defined. So I added in support for rendering the index url if defined. This only applies to urlNote one
My autoformatting editor makes this harder to review than I'd like. Instead of going back and undoing all the autoformatters this time... I'll turn my autoformatter off for future PRs. I'm sorry.
Note two
@noahgibbs maybe I can nerd snipe you on this one. (No presh obvs, you might find it interesting)
If you look at
_why-stories.rb, on line 18, you can see what we're missing. Essentially we needpara link('foo', click: '/my/url').But my efforts were not able to yield a win yet. Because
clickis a javascript and web concern as well, so, trying to update it to 'visit' a valid url, we get failures saying that it "could not open url". And I struggled to get outputs even with debug on run. We currently support going to an "actual" external URL, but I'm okay for that feature to die. It was never shoes-spec and I don't really want this to navigate to external websites anyway.I think it could be very tricky! But we also haven't touched link in a year and a half I think so...
BONUS:
We also don't currently support
But I might file an issue.
Image(if needed, helps for a faster review)
Checklist