From 329f52c90196a4a5e8f9c21b10a9d15e10743b11 Mon Sep 17 00:00:00 2001 From: Noah Gibbs Date: Thu, 29 Jun 2023 11:07:41 +0100 Subject: [PATCH] Fix most self bugs - use Scarpe::App as self nearly always. Make display properties properly inheritable Move DocumentRoot special behavior into app, make it 'just a Flow' Remove unused app-option override code in ControlInterface --- examples/selfitude.rb | 18 +++++++++ lib/scarpe/app.rb | 45 +++++++++++++++++++-- lib/scarpe/document_root.rb | 8 +--- lib/scarpe/download.rb | 47 ++++++++++++++++++++++ lib/scarpe/flow.rb | 7 +--- lib/scarpe/shape.rb | 1 + lib/scarpe/slot.rb | 6 +++ lib/scarpe/stack.rb | 54 ++------------------------ lib/scarpe/widget.rb | 30 +++++++------- lib/scarpe/widgets.rb | 2 + lib/scarpe/wv.rb | 2 +- lib/scarpe/wv/app.rb | 28 ++++++++++--- lib/scarpe/wv/control_interface.rb | 20 +++------- lib/scarpe/wv/document_root.rb | 48 ++--------------------- lib/scarpe/wv/webview_local_display.rb | 1 + lib/scarpe/wv/widget.rb | 10 +++-- test/test_link.rb | 8 ++-- test/test_slots.rb | 23 +++++++++++ 18 files changed, 206 insertions(+), 152 deletions(-) create mode 100644 examples/selfitude.rb create mode 100644 lib/scarpe/download.rb create mode 100644 lib/scarpe/slot.rb create mode 100644 test/test_slots.rb diff --git a/examples/selfitude.rb b/examples/selfitude.rb new file mode 100644 index 000000000..08dd42179 --- /dev/null +++ b/examples/selfitude.rb @@ -0,0 +1,18 @@ +def printish(msg) + #$stderr.puts msg + File.open("/tmp/shoesy_stuff.txt", "a") do |f| + f.write(msg + "\n") + end +end + +Shoes.app do + printish "Self top: #{self.inspect}" # It's an instance of Shoes::App, yes + stack do + printish "Self in stack: #{self.inspect}" # Yup, here too + end + button("Clickity") do + alert("You clicked me!") + printish "Self in button handler: #{self.inspect}" # And here too + end +end + diff --git a/lib/scarpe/app.rb b/lib/scarpe/app.rb index 61fd4af24..86a7e3eb1 100644 --- a/lib/scarpe/app.rb +++ b/lib/scarpe/app.rb @@ -20,8 +20,6 @@ def initialize( debug: ENV["SCARPE_DEBUG"] ? true : false, &app_code_body ) - log_init("Scarpe::App") - if Scarpe::App.instance @log.error("Trying to create a second Scarpe::App in the same process! Fail!") raise "Cannot create multiple Scarpe::App objects!" @@ -29,6 +27,8 @@ def initialize( Scarpe::App.instance = self end + log_init("Scarpe::App") + @do_shutdown = false super @@ -36,6 +36,8 @@ def initialize( # This creates the DocumentRoot, including its corresponding display widget @document_root = Scarpe::DocumentRoot.new + @slots = [] + # Now create the App display widget create_display_widget @@ -68,7 +70,31 @@ def init send_shoes_event(event_name: "init") return if @do_shutdown - @document_root.instance_eval(&@app_code_body) + ::Scarpe::App.instance.with_slot(@document_root, &@app_code_body) + end + + # "Container" widgets like flows, stacks, masks and the document root + # are considered "slots" in Scarpe parlance. When a new slot is created, + # we push it here in order to track what widgets are found in that slot. + def push_slot(slot) + @slots.push(slot) + end + + def pop_slot + @slots.pop + end + + def current_slot + @slots[-1] + end + + def with_slot(slot_item, &block) + return unless block_given? + + push_slot(slot_item) + Scarpe::App.instance.instance_eval(&block) + ensure + pop_slot end # This isn't supposed to return. The display service should take control @@ -87,3 +113,16 @@ def destroy(send_event: true) end end end + +# DSL methods +class Scarpe::App + def background(...) + current_slot.background(...) + end + + def border(...) + current_slot.border(...) + end + + alias_method :info, :puts +end diff --git a/lib/scarpe/document_root.rb b/lib/scarpe/document_root.rb index f0d4f3e7a..14a39c974 100644 --- a/lib/scarpe/document_root.rb +++ b/lib/scarpe/document_root.rb @@ -1,22 +1,18 @@ # frozen_string_literal: true class Scarpe - class DocumentRoot < Scarpe::Widget - include Scarpe::Background - + class DocumentRoot < Scarpe::Slot def initialize super create_display_widget end - # This can be absolutely huge in console output, and it's frequently printed. + # The default inspect string can be absolutely huge in console output, and it's frequently printed. def inspect "" end - alias_method :info, :puts - def all_widgets out = [] diff --git a/lib/scarpe/download.rb b/lib/scarpe/download.rb new file mode 100644 index 000000000..7ef76f769 --- /dev/null +++ b/lib/scarpe/download.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +require "net/http" +require "openssl" +require "nokogiri" + +class Scarpe::Widget + def download(url) + Thread.new do + begin + uri = URI(url) + http = Net::HTTP.new(uri.host, uri.port) + http.use_ssl = true + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + + request = Net::HTTP::Get.new(uri.request_uri) + + http.request(request) do |response| + case response + when Net::HTTPSuccess + # content = response.body + + # headers = response.header + + html_get_title(content) + else + Scarpe.error("Failed to download content. Response code: #{response.code}") + end + end + rescue StandardError => e + Scarpe.error("Error occurred while downloading: #{e.message}") + end + end + end + + private + + def html_get_title(content) + doc = Nokogiri::HTML(content) + + title = doc.at_css("title")&.text&.strip || "" + + # headings = doc.css("h1").map(&:text) + + title + end +end diff --git a/lib/scarpe/flow.rb b/lib/scarpe/flow.rb index dec3394db..8f2e3df8b 100644 --- a/lib/scarpe/flow.rb +++ b/lib/scarpe/flow.rb @@ -1,10 +1,7 @@ # frozen_string_literal: true class Scarpe - class Flow < Scarpe::Widget - include Scarpe::Background - include Scarpe::Border - + class Flow < Scarpe::Slot display_properties :width, :height, :margin, :padding def initialize(width: nil, height: "100%", margin: nil, padding: nil, &block) @@ -13,7 +10,7 @@ def initialize(width: nil, height: "100%", margin: nil, padding: nil, &block) # Create the display-side widget *before* instance_eval, which will add child widgets with their display widgets create_display_widget - instance_eval(&block) + Scarpe::App.instance.with_slot(self, &block) if block_given? end end end diff --git a/lib/scarpe/shape.rb b/lib/scarpe/shape.rb index 70d23bfa2..c3270d20d 100644 --- a/lib/scarpe/shape.rb +++ b/lib/scarpe/shape.rb @@ -13,6 +13,7 @@ def initialize(left: nil, top: nil, path_commands: nil, &block) super() create_display_widget + instance_eval(&block) if block_given? end end diff --git a/lib/scarpe/slot.rb b/lib/scarpe/slot.rb new file mode 100644 index 000000000..b8f3e1f6a --- /dev/null +++ b/lib/scarpe/slot.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class Scarpe::Slot < Scarpe::Widget + include Scarpe::Background + include Scarpe::Border +end diff --git a/lib/scarpe/stack.rb b/lib/scarpe/stack.rb index eb17264d9..2442f1666 100644 --- a/lib/scarpe/stack.rb +++ b/lib/scarpe/stack.rb @@ -1,13 +1,7 @@ # frozen_string_literal: true -require "net/http" -require "openssl" -require "nokogiri" - class Scarpe - class Stack < Scarpe::Widget - include Scarpe::Background - include Scarpe::Border + class Stack < Scarpe::Slot include Scarpe::Spacing display_properties :width, :height, :margin, :padding, :scroll, :margin_top, :margin_left, :margin_right, :margin_bottom, :options @@ -21,50 +15,8 @@ def initialize(width: nil, height: "100%", margin: nil, padding: nil, scroll: fa super create_display_widget - # Create the display-side widget *before* instance_eval, which will add child widgets with their display widgets - instance_eval(&block) if block_given? - end - end - - class Widget - def download(url) - Thread.new do - begin - uri = URI(url) - http = Net::HTTP.new(uri.host, uri.port) - http.use_ssl = true - http.verify_mode = OpenSSL::SSL::VERIFY_NONE - - request = Net::HTTP::Get.new(uri.request_uri) - - http.request(request) do |response| - case response - when Net::HTTPSuccess - # content = response.body - - # headers = response.header - - get_title(content) - else - Scarpe.error("Failed to download content. Response code: #{response.code}") - end - end - rescue StandardError => e - Scarpe.error("Error occurred while downloading: #{e.message}") - end - end - end - - private - - def get_title(content) - doc = Nokogiri::HTML(content) - - title = doc.at_css("title")&.text&.strip || "" - - # headings = doc.css("h1").map(&:text) - - title + # Create the display-side widget *before* running the block, which will add child widgets with their display widgets + Scarpe::App.instance.with_slot(self, &block) if block_given? end end end diff --git a/lib/scarpe/widget.rb b/lib/scarpe/widget.rb index e229aa526..52197ab2b 100644 --- a/lib/scarpe/widget.rb +++ b/lib/scarpe/widget.rb @@ -12,15 +12,11 @@ class Widget < DisplayService::Linkable include Scarpe::Colors class << self - attr_accessor :widget_classes, :alias_name - - def alias_as(name) - self.alias_name = name - end + attr_accessor :widget_classes def inherited(subclass) - self.widget_classes ||= [] - self.widget_classes << subclass + Scarpe::Widget.widget_classes ||= [] + Scarpe::Widget.widget_classes << subclass super end @@ -30,7 +26,7 @@ def dsl_name end def widget_class_by_name(name) - widget_classes.detect { |k| k.dsl_name == name.to_s || k.alias_name.to_s == name.to_s } + widget_classes.detect { |k| k.dsl_name == name.to_s } end private @@ -56,16 +52,20 @@ def display_property(name) linkable_properties_hash[name] = true end + # Add these names as display properties def display_properties(*names) names.each { |n| display_property(n) } end def display_property_names - linkable_properties.map { |prop| prop[:name] } + parent_prop_names = self != Scarpe::Widget ? self.superclass.display_property_names : [] + + parent_prop_names | linkable_properties.map { |prop| prop[:name] } end def display_property_name?(name) - linkable_properties_hash[name.to_s] + linkable_properties_hash[name.to_s] || + (self != Scarpe::Widget && superclass.display_property_name?(name)) end end @@ -91,9 +91,11 @@ def bind_no_target_event(event_name, &block) bind_shoes_event(event_name:, &block) end - def display_properties + def display_property_values + all_property_names = self.class.display_property_names + properties = {} - self.class.display_property_names.each do |prop| + all_property_names.each do |prop| properties[prop] = instance_variable_get("@" + prop) end properties["shoes_linkable_id"] = self.linkable_id @@ -104,7 +106,7 @@ def create_display_widget klass_name = self.class.name.delete_prefix("Scarpe::").delete_prefix("Shoes::") # Should we save a reference to widget for later reference? - DisplayService.display_service.create_display_widget_for(klass_name, self.linkable_id, display_properties) + DisplayService.display_service.create_display_widget_for(klass_name, self.linkable_id, display_property_values) end attr_reader :parent @@ -177,7 +179,7 @@ def method_missing(name, *args, **kwargs, &block) widget_instance = klass.new(*args, **kwargs, &block) unless klass.ancestors.include?(Scarpe::TextWidget) - widget_instance.set_parent(self) + widget_instance.set_parent Scarpe::App.instance.current_slot end widget_instance diff --git a/lib/scarpe/widgets.rb b/lib/scarpe/widgets.rb index b08b8a801..012347b29 100644 --- a/lib/scarpe/widgets.rb +++ b/lib/scarpe/widgets.rb @@ -11,10 +11,12 @@ require_relative "fill" +require_relative "slot" require_relative "document_root" require_relative "para" require_relative "stack" require_relative "flow" +require_relative "download" require_relative "button" require_relative "image" require_relative "edit_box" diff --git a/lib/scarpe/wv.rb b/lib/scarpe/wv.rb index 8da8b7f89..2b5495bfc 100644 --- a/lib/scarpe/wv.rb +++ b/lib/scarpe/wv.rb @@ -21,10 +21,10 @@ require_relative "wv/arc" require_relative "wv/app" -require_relative "wv/document_root" require_relative "wv/para" require_relative "wv/stack" require_relative "wv/flow" +require_relative "wv/document_root" require_relative "wv/button" require_relative "wv/image" require_relative "wv/edit_box" diff --git a/lib/scarpe/wv/app.rb b/lib/scarpe/wv/app.rb index 19c1f7e81..ea6b7aaaa 100644 --- a/lib/scarpe/wv/app.rb +++ b/lib/scarpe/wv/app.rb @@ -9,9 +9,6 @@ class WebviewApp < WebviewWidget attr_writer :shoes_linkable_id def initialize(properties) - # Is this a thing? Do we care about this? - # opts = @control_interface.app_opts_get_override(opts) - super # It's possible to provide a Ruby script by setting @@ -35,6 +32,8 @@ def initialize(properties) resizable: @resizable, debug: @debug + @callbacks = {} + # The control interface has to exist to get callbacks like "override Scarpe app opts". # But the Scarpe App needs those options to be created. So we can't pass these to # ControlInterface.new. @@ -51,11 +50,11 @@ def init scarpe_app = self @view.init_code("scarpeInit") do - @document_root.request_redraw! + request_redraw! end @view.bind("scarpeHandler") do |*args| - @document_root.handle_callback(*args) + handle_callback(*args) end @view.bind("scarpeExit") do @@ -82,5 +81,24 @@ def destroy @view = nil end end + + # All JS callbacks to Scarpe widgets are dispatched + # via this handler + def handle_callback(name, *args) + @callbacks[name].call(*args) + end + + # Bind a Scarpe callback name; see handle_callback above. + # See Scarpe::Widget for how the naming is set up + def bind(name, &block) + @callbacks[name] = block + end + + def request_redraw! + wrangler = WebviewDisplayService.instance.wrangler + if wrangler.is_running + wrangler.replace(@document_root.to_html) + end + end end end diff --git a/lib/scarpe/wv/control_interface.rb b/lib/scarpe/wv/control_interface.rb index 15a16e3de..4b9d843a8 100644 --- a/lib/scarpe/wv/control_interface.rb +++ b/lib/scarpe/wv/control_interface.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true # The ControlInterface is used for testing. It's a way to register interest -# in important events like redraw, init and shutdown, and to override -# test-relevant values like the options to Shoes.app(). Note that no part -# of the Scarpe framework should ever *depend* on ControlInterface. It's -# for testing, not normal operation. If no ControlInterface were ever -# created or called, Scarpe apps should run fine with no modifications. +# in important events like redraw, init and shutdown, and to configure a +# Shoes app for testing. Note that no part of the Scarpe framework should +# ever *depend* on ControlInterface. It's for testing, not normal operation. +# If no ControlInterface were ever created or called, Scarpe apps should run +# fine with no modifications. # # And if you depend on this from the framework, I'll add a check-mode that # never dispatches any events to any handlers. Do NOT test me on this. @@ -79,16 +79,6 @@ def wrangler # The control interface has overrides for certain settings. If the override has been specified, # those settings will be overridden. - # Override the Shoes app opts like "debug:" and "die_after:" with new ones. - def override_app_opts(new_opts) - @new_app_opts = new_opts - end - - # Called by Scarpe::App to get the override options - def app_opts_get_override(opts) - @new_app_opts || opts - end - # On recognised events, this sets a handler for that event def on_event(event, &block) unless SUBSCRIBE_EVENTS.include?(event) diff --git a/lib/scarpe/wv/document_root.rb b/lib/scarpe/wv/document_root.rb index 9c478be14..a0e380678 100644 --- a/lib/scarpe/wv/document_root.rb +++ b/lib/scarpe/wv/document_root.rb @@ -1,50 +1,8 @@ # frozen_string_literal: true class Scarpe - class WebviewDocumentRoot < Scarpe::WebviewWidget - include Scarpe::WebviewBackground - - def initialize(properties) - @callbacks = {} - - super - end - - def element(&blck) - HTML.render do |h| - h.div(style: style.merge(height: "100%"), &blck) - end - end - - # Bind a Scarpe callback name; see Scarpe::Widget for how the naming is set up - def bind(name, &block) - @callbacks[name] = block - end - - # All JS callbacks to Scarpe widgets are dispatched - # via this handler, which is set up in Scarpe::App - def handle_callback(name, *args) - @callbacks[name].call(*args) - end - - # The document root knows when a frame has finished. It registers end-of-frame callbacks and calls them - # when requested. It also tracks when a redraw has been requested. Note that often frames will be - # very rare if nothing is changing, with seconds or minutes passing in between them. - - def request_redraw! - wrangler = WebviewDisplayService.instance.wrangler - if wrangler.is_running - wrangler.replace(self.to_html) - end - end - - # The document root manages the connection between widgets and the WebviewWrangler. - # By centralising this and wrapping in API functions, we can keep from executing - # random Javascript, mostly. - - # A Widget can request one or more of these as insertion points in the DOM - def get_element_wrangler(html_id) - Scarpe::WebWrangler::ElementWrangler.new(html_id) - end + # A WebviewDocumentRoot is a WebviewFlow, with all the same properties + # and basic behavior. + class WebviewDocumentRoot < Scarpe::WebviewFlow end end diff --git a/lib/scarpe/wv/webview_local_display.rb b/lib/scarpe/wv/webview_local_display.rb index ccbdfdbef..d3c150d53 100644 --- a/lib/scarpe/wv/webview_local_display.rb +++ b/lib/scarpe/wv/webview_local_display.rb @@ -16,6 +16,7 @@ class << self # TODO: re-think the list of top-level singleton objects. attr_reader :control_interface attr_reader :doc_root + attr_reader :app attr_reader :wrangler # This is called before any of the various WebviewWidgets are created. diff --git a/lib/scarpe/wv/widget.rb b/lib/scarpe/wv/widget.rb index c5e900cf9..7aa1727dd 100644 --- a/lib/scarpe/wv/widget.rb +++ b/lib/scarpe/wv/widget.rb @@ -75,6 +75,10 @@ def set_parent(new_parent) @parent = new_parent end + def inspect + "#<#{self.class}:#{self.object_id} @shoes_linkable_id=#{@shoes_linkable_id} @parent=#{@parent.inspect} @children=#{@children.inspect}>" + end + protected # Do not call directly, use set_parent @@ -126,7 +130,7 @@ def rgb_to_hex(color) # This gets a mini-webview for just this element and its children, if any def html_element - @elt_wrangler ||= WebviewDisplayService.instance.doc_root.get_element_wrangler(html_id) + @elt_wrangler ||= Scarpe::WebWrangler::ElementWrangler.new(html_id) end # Return a promise that guarantees all currently-requested changes have completed @@ -154,7 +158,7 @@ def to_html def bind(event, &block) raise("Widget has no linkable_id! #{inspect}") unless linkable_id - WebviewDisplayService.instance.doc_root.bind("#{linkable_id}-#{event}", &block) + WebviewDisplayService.instance.app.bind("#{linkable_id}-#{event}", &block) end # Removes the element from both the Ruby Widget tree and the HTML DOM. @@ -168,7 +172,7 @@ def destroy_self # And so we can't easily cancel one "in flight," and we can't easily pick up the latest # changes... And we probably don't want to, because we may be halfway through a batch. def needs_update! - WebviewDisplayService.instance.doc_root.request_redraw! + WebviewDisplayService.instance.app.request_redraw! end def handler_js_code(handler_function_name, *args) diff --git a/test/test_link.rb b/test/test_link.rb index c49555fcd..6a42815fe 100644 --- a/test/test_link.rb +++ b/test/test_link.rb @@ -16,14 +16,14 @@ def setup def with_mocked_binding(&block) @mocked_disp_service = Minitest::Mock.new - @mocked_doc_root = Minitest::Mock.new - @mocked_disp_service.expect(:doc_root, @mocked_doc_root) - @mocked_doc_root.expect(:bind, nil, [String]) + @mocked_app = Minitest::Mock.new + @mocked_disp_service.expect(:app, @mocked_app) + @mocked_app.expect(:bind, nil, [String]) Scarpe::WebviewDisplayService.stub(:instance, @mocked_disp_service, &block) @mocked_disp_service.verify - @mocked_doc_root.verify + @mocked_app.verify end def test_link_with_url diff --git a/test/test_slots.rb b/test/test_slots.rb new file mode 100644 index 000000000..83cfd0d16 --- /dev/null +++ b/test/test_slots.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "test_helper" + +class TestSlots < LoggedScarpeTest + def test_stack_child + run_test_scarpe_code(<<-'SCARPE_APP', test_code: <<-'TEST_CODE') + Shoes.app do + stack do + para "Hello World" + end + end + SCARPE_APP + on_event(:next_redraw) do + para = find_wv_widgets(Scarpe::WebviewPara)[0] + assert para.parent.is_a?(Scarpe::WebviewStack), "A widget created in a Stack's block should be a child of the stack!" + return_when_assertions_done + end + TEST_CODE + end + + # TODO: we need to make sure that self is a Scarpe::App inside the "shape do" block and that helpers work +end