UnitedLexCorp / SimpleTalk

An implementation of HyperTalk in Ohm-js
Apache License 2.0
7 stars 1 forks source link

Current of current #206

Closed dkrasner closed 3 years ago

dkrasner commented 3 years ago

Main Points

Previously "current card" was a terminal object specifier which result in two problems:

This PR takes care of this, but making current stack terminal whereas current card partial.

Resolves #167

Tests added.

Like with all things related to the interpreter and grammar, please review the code and try out various examples.

ApproximateIdentity commented 3 years ago

Do we care about allowing spaces between "current" and "card"? The following works:

on click
    add button     to current card of current stack
end click

but the following does not:

on click
    add button to current      card of current stack
end click

I think it could have something to do with the following which does a comparison directly to "current card" as a string (though this is speculation):

https://github.com/UnitedLexCorp/SimpleTalk/pull/206/files#diff-8d8be2b6b66a037209c596f4a3aee487865dacf1228f7be0783b073e4b87dcb5R925

Do we care about this sort of thing? Do we consider whitespace between tokens to all mean the same thing as a single space?

edit: Actually it can't (only) be that line because I can't save the script due to a semantic problem and not an interpreter issue.

darth-cheney commented 3 years ago

@ApproximateIdentity that's a good catch and the problem here is present in other parts of the grammar (and by extension, pieces of the semantics as you've pointed out). The solution generally, to my mind, is to go from this:

PartialSpecifier
      = "target" --partByTarget
      | "current card" --currentCard

to this:

PartialSpecifier
      = "target" --partByTarget
      | "current" "card" --currentCard

This has to do with the difference in Ohm between syntactic and lexical rules. Syntactic rules begin with uppercase letters (like PartialSpecifier above) and implicitly skip whitespace during parsing of their rule definition parts. Lexical rules take explicit whitespace into consideration. Because the above example is a syntactic rule, by splitting the explicit tokens up into two arguments the whitespace is implicitly skipped and you can use as much as you want between the words.

Another perhaps better (or related) option might be to specify a syntactic rule for things with "current" in them and to use that via composition with other rules.

Overall we've been a little careless here with these explicit tokens (see here, here, and here for just a few examples). We should definitely clean this up, since we don't want some parts of an expression to arbitrarily ignore whitespace and others to be very strict about it.

However, I think this issue is not directly relevant to this specific PR and I'm undecided on whether or not it should be taken care of here. Perhaps we can open an issue and make it a different bugfix. What do you think? @ApproximateIdentity @dkrasner

ApproximateIdentity commented 3 years ago

Yeah I think it makes sense to hold off and do a systematic grammar clean of this sort of thing.

darth-cheney commented 3 years ago

@dkrasner (in addition to my comments above):

This is working pretty well. I was even able to do this in a field:

importWorld "layout-examples.html"
add button "hello" to current card of second stack

and it did what I expected

One issue I'm having now is adding a button to the toolbar using this same method. I would expect this to work:

-- The "first stack of first window of current stack" below specifies
-- the stack that is in the toolbar window
add button "hello" to current card of first stack of first window of current stack

But it appears to instead add the button to the current (visible) card.

darth-cheney commented 3 years ago

@dkrasner (in addition to my comments above):

This is working pretty well. I was even able to do this in a field:

importWorld "layout-examples.html"
add button "hello" to current card of second stack

and it did what I expected

One issue I'm having now is adding a button to the toolbar using this same method. I would expect this to work:

-- The "first stack of first window of current stack" below specifies
-- the stack that is in the toolbar window
add button "hello" to current card of first stack of first window of current stack

But it appears to instead add the button to the current (visible) card.

dkrasner commented 3 years ago

I think it makes sense to fix the spacing and open an issue to deal with the rest in a different PR. Good catch there.

@darth-cheney the window issue is coming from how I wrote this

i wasn't careful enough here. There are a number of options again of what "current card" could mean

current card <- always the current card of current stack (no matter where it is) (non terminal) current card of [stack - not "current" and not by id] <- current card of that specific stack in the world, even if you are in a window, i.e. current card of second stack will not reference a second stack in the window if called from there(non terminal) current card of stack id [N] <- current card of that stack, no matter where it is window or otherwise (terminal) current card of [stack] of [window] <- current card of the specified stack in the window

this is the one up for some debate:

current card of current stack <- always the current card of the current stack, even if the context is a window. This would match current card interpretation (terminal)

does that sound right to you?

darth-cheney commented 3 years ago

Hmm yes this is already confusing.

current card of current stack <- always the current card of the current stack, even if the context is a window. This would match current card interpretation (terminal)

This makes sense to me because a stack that is in a window is never itself current, though it does have its own idea of a card that is current.

dkrasner commented 3 years ago

actually on second though this

current card of Nth stack (in the context of a window)

is just as confusing. If you use our model of "go up to the first viable parent" then here it would be the window and so would look for the Nth stack (stack N, or stack by name) in the window.

Basically we need to make a choice. Do we follow the "first viable parent" rule, or do we say "if you are in a window the default context is the parent of the window, so all window related specifiers need to terminate with a window"

The former is more consistent and potentially easiest to keep track of, debug etc. The latter might be more natural since windows are sort of weird... I am not sure here.

dkrasner commented 3 years ago

Ok this fixes up both the window context related scripting @darth-cheney pointed out and also the spacing issues @ApproximateIdentity pointed out (but only for current specifiers, not in general across the grammar)

add button to current  card of current    stack

tell first button of current        card of first stack of first window of first stack to set "background-color" to "yellow"

add field to current card of first stack of window "Toolbox" of current stack

tell button 1 of current card to set "background-color" to "red"

all work as expected in the context of a field on the current card (of the current stack)

tell button 1 of current card of current stack to set "background-color" to "blue"

will reference buttons in the current card of the current stack regardless of where it is run (window or not) b/c current stack is parsed as a terminal specifier only (unlike current card which can be terminal or partial)

running

tell button 1 of current card to set "background-color" to "red"

will do the same thing b/c specifying a current card without any further context is always interpreted as the current card, i.e. it is a terminal as well

tell button 1 of current card in first stack to set "background-color" to "red"

is context dependent (in a window, it's the first stack of the window, in a non-window context it's the first stack of the world)

the game here was to properly use Ohm's greediness to organize the rules and then add a few catchalls for the current card situation

i think it can be cleaner but that depends largely on where we go with issues #207

BUT it is touchy this stuff, so please play around some more (tests passing are tests passing.. )

dkrasner commented 3 years ago

@ApproximateIdentity @darth-cheney

In view of various discussions (in particular #207), I've gone ahead and refactored st-window. It now accepts pretty much everything other than world, stack and card, essentially functioning like a slot for things you want to move around with a title bar.

Although I imagine we'll use st-area almost all the time as the first (and only) subpart0,st-window does not take layout props or assume anything about what you want to put there. If you want to add just a button, or a drawing, or both that's fine. If we want to make it more restrictive down the line we can also do that. One reason to avoid layouts directly on the st-window part/view is that these can clash in unpredictable ways with layouts in a sub-part (like st-area); this mostly happens because a window wraps its content, unlike an area being the direct owner/parent of it.

The grammar is now easier to debug, and has fewer potential hiccups.

The script editor has been updated.

I've gone ahead and made a new toolbox and added a bootstrap2.html file with. If we are happy with this we can copy it to the original bootstrap.html.

The reason to have both files around is that none of our old stacks will load (since the Toolbox there is a window with a stack). Hence, I have gone ahead and saved a version of each of our example stacks without the Toolbox, then ran this little script

importWorld "bootstrap2.html"
tell window "Toolbox" of second stack to copy
tell current stack to paste
deleteModel second stack

that worked just fine. If I missed some example file you care about this is the way to roll, or if you simply don't like my toolbox.

Making the toolbox was also pretty trivial:

add window to current stack
tell first window of current stack to set "title" to "Toolbox" 
tell first window of current stack to set "name" to "Toolbox" 
tell first window of current stack to set "width" to 180

add area to  window "Toolbox" of current stack 

tell first area of  window "Toolbox" of current stack to set "layout" to "list"
tell first area of  window "Toolbox" of current stack to set "layout" to "column"
tell first area of  window "Toolbox" of current stack to set "list-alignment" to "center"
tell first area of  window "Toolbox" of current stack to set "width" to "fill"
tell first area of  window "Toolbox" of current stack to set "height" to "fill"

add button to first area of window "Toolbox" of current stack
tell first button of  first area of window "Toolbox" of current stack to set "width" to "fill"

after that just copy and paste....

PS when you load the examples make sure to hard-refresh (sometimes it takes 2 for some reason); otherwise you might end up loading what is cached in the browser and that will fail.

I have ideas about stack browsing and the like, as well as dealing with errors under the hood and what should actually surface if anything, but these are for another PR.

darth-cheney commented 3 years ago

@dkrasner

Although I imagine we'll use st-area almost all the time as the first (and only) subpart0,st-window does not take layout props or assume anything about what you want to put there. If you want to add just a button, or a drawing, or both that's fine. If we want to make it more restrictive down the line we can also do that. One reason to avoid layouts directly on the st-window part/view is that these can clash in unpredictable ways with layouts in a sub-part (like st-area); this mostly happens because a window wraps its content, unlike an area being the direct owner/parent of it.

Let's kick the can on this. I'm imagining we will eventually want at least some of the layout props for windows. For example, if we want a window to "shrink wrap" its contents, that's just a matter of setting horizontal and/or vertical resizing. However if you are seeing problems with the CSS for this let's make it a separate issue. Area composition should work well enough for now.

Making the toolbox was also pretty trivial:

This is great

I've gone ahead and made a new toolbox and added a bootstrap2.html file with. If we are happy with this we can copy it to the original bootstrap.html.

Let's go ahead an do it. Nothing is sacred.

Question: do we want to even serialize the toolbox back into our example stacks? Or should we leave them alone for the moment and copy/paste as needed from bootstrap when we are using them?

Tests are passing on my end so I say this is good to merge otherwise.

dkrasner commented 3 years ago

@dkrasner

Although I imagine we'll use st-area almost all the time as the first (and only) subpart0,st-window does not take layout props or assume anything about what you want to put there. If you want to add just a button, or a drawing, or both that's fine. If we want to make it more restrictive down the line we can also do that. One reason to avoid layouts directly on the st-window part/view is that these can clash in unpredictable ways with layouts in a sub-part (like st-area); this mostly happens because a window wraps its content, unlike an area being the direct owner/parent of it.

Let's kick the can on this. I'm imagining we will eventually want at least some of the layout props for windows. For example, if we want a window to "shrink wrap" its contents, that's just a matter of setting horizontal and/or vertical resizing. However if you are seeing problems with the CSS for this let's make it a separate issue. Area composition should work well enough for now.

yep, we might re-imagine some more specific css/style-props setup for window.

Making the toolbox was also pretty trivial:

This is great

I've gone ahead and made a new toolbox and added a bootstrap2.html file with. If we are happy with this we can copy it to the original bootstrap.html.

Let's go ahead an do it. Nothing is sacred. ok done

Question: do we want to even serialize the toolbox back into our example stacks? Or should we leave them alone for the moment and copy/paste as needed from bootstrap when we are using them?

well one thing that i wanted to do was to make sure that these examples loaded, otherwise it's a pain in a demo. I figured to add the toolbox b.c. that's what we are used to but i think in general we should make a menu stack, as discussed earlier, and then grab things as needed from there with an import + copy&paste. This is actually a more effective demo anyhow. Tests are passing on my end so I say this is good to merge otherwise.