[[ Resolve File ]] Implement resolve file relative to object#6656
Conversation
3c3ae80 to
1f36266
Compare
|
@montegoulding I wonder if (when in a widget) if there is no 'relative to' then it should use the widget's script object. Obviously for libraries, there is no current widget to use, so defaultStack makes sense. There's actually an issue in 'image from path' in the canvas library which assumes there is a current widget - perhaps we need a general handler in the engine to get 'current object context'? |
|
That would fit with the send/post behavior which does exactly the same thing. |
|
Ok |
|
@runrevmark I have resolved anomaly 19137 here for the sake of consistency/sanity. |
|
Hmm... I just noticed the mentioned issue for |
97f8b16 to
d5f8466
Compare
|
rebased and fixed some missing intents in the syntax declaration |
|
This needs tests for all the cases touched:
|
|
We already have tests covering send & post in widgets and libraries and execute script in libraries. I've just noticed another anomaly here. |
9319d2b to
b745c00
Compare
|
I have now added the extra tests and think I have covered all the cases that need to be covered here |
e1c8ccd to
b78f5ae
Compare
|
@montegoulding I don't think its quite right to lump all the context tests into a 'ContextObject' test as that is an internal abstraction - it would be better if the tests were split up by the features. In particular, we have a TestExtensionExecuteScript test (which still references anomaly 19137) - that should be made to ensure it covers the cases which this touches; and add the widget cases. There seems to be a duplicate test for resolve file in both the EngineContextTest and the ResolveFileTest (maybe it is slightly different - but if so they should all be in the ResolveFileTest). Similarly, the image from file tests should go in a TestExtensionImageFromFile test handler. |
|
Also can you clarify how anomaly 19137 has been resolved? The widget case is obviously clear - it should be the widget's script object.... However, the suggestion made in the bug report for libraries was that it should be the script object which called the library handler - the rationale there is that execute script is do... If you called do from within an engine method it would use the script object which called the engine method; just as using do from LCS does. Maybe that is too big a change though... |
|
OK I will reorganise the tests. Regarding 19137 I can't recall if I discussed it with you... was a while back. Anyway, my thoughts were:
I can change it to the caller if you like or change the note to saying it is resolved only for widgets, however, I think it would be good if the implicit object follows consistent rules for all syntax. |
|
@montegoulding Okay - that's fine :) The key thing is that libraries can have foreign callbacks and so (as you point out) there is no consistent script object context, so using default card of default stack universally for libraries in this case is fine. As you point out, libraries can always use 'the caller' to explicitly specify they want to execute script in the calling script object :) Perhaps add a note to the anomaly about the resolution (and reason why default card ... makes sense for libraries) - we can consider that anomaly resolved then :) |
This patch implements a new LCB statement to resolve a file path relative to a script object in the same way that relative file paths are resolved in LiveCode script.
116ca9b to
d86e462
Compare
This patch implement `MCEngineCurrentContextObject` to be reused where a function needs a fallback default object. If the function is called by a widget script the particular widget is the default object while if it is called by a library the current card of the defaultStack is the default object. This patch updates the engine module functions to use `MCEngineCurrentContextObject` where appropriate.
d86e462 to
857a2fb
Compare
|
@montegoulding The tests look much better - thanks :) Last thing here can you add a section to the 'breaking changes' doc as there is a non-backwards-compatible change here. |
|
@montegoulding I'm reviewing this as is for the moment as we need it in before we can merge the macstatusmenu feature... If you could submit a seperate PR with the breaking changes update that would be great. |
|
@livecode-vulcan review ok 857a2fb |
|
💙 review by @runrevmark ok 857a2fb |
[[ Resolve File ]] Implement resolve file relative to object This patch implements a new LCB statement to resolve a file path relative to a script object in the same way that relative file paths are resolved in LiveCode script.
|
😎 test success 857a2fb
|
[[ Mac Status Menu ]] Implement mac status menu library This patch is an initial implementation of a mac status menu library. Depends on #6656
This patch implements a new LCB statement to resolve a file path relative to a
script object in the same way that relative file paths are resolved in LiveCode
script.