[[Bugfix 14721]] Implement 'go visible <stack>' as an antonym to 'go invisible' by andrewferguson · Pull Request #6641 · livecode/livecode

andrewferguson · 2018-08-16T12:15:40Z

The go visible command has been added, along with a new parser test for the go command (seemingly it did not have any parser tests before?). The documentation and functional tests have been updated to take account of the new option.

The go command has been updated to include the go visible form which automatically sets the visible property of the stack to true when it is opened. This is particularly useful for script-only stacks that display any kind of interface (ie: that have visible controls), as they currently need to manually include a line in the openStack or preOpenStack handler to set the visible property to true.

A new parser test file has been created for the go command, as it did not have one before. Three new tests have been added, to test the basic syntax of go stack "stack", go visible stack "stack" and go invisible stack "stack". This test file is far from complete - there are many other forms of the go command that have not been tested, however I have added in only the three most relevent to the go visible command. Any others can be added separately in a different branch. Several new tests for the go command have been added to the core interface test file. These tests check that the stack actually opens when using the visible and invisible forms and that the visibility of the stack is correctly set.

The dictionary file for the go command has been updated to include the new go visible command. A note has also been added to point out the occasion where the go visible and go invisible commands will not work as expected (when the stack overrides the command by setting its visible property in its script as it opens).

livecodeali · 2018-08-16T13:32:13Z


 	initpoint(sp);
-	if (sp.skip_token(SP_FACTOR, TT_PROPERTY, P_INVISIBLE) == PS_NORMAL)
+	if (sp.skip_token(SP_FACTOR, TT_PROPERTY, P_INVISIBLE) == PS_NORMAL) {


Can you put the opening parentheses here and below on the following line please?

Now no longer necessary - with an enum rather than bools the if becomes a single line again.

livecodeali · 2018-08-16T13:34:17Z


 local sStackFileName
-command _TestCreateStack pBinary
+command _TestCreateStack pBinary pVisible


Can you separate your param names with commas please? (here and in _TestGoInvisibleVisibleStack)

The two boolean variables that represented the visibility of the stack, p_visible and p_explicitly_visible, have been replaced by a single enum p_visibility_type, of type MCGoStackVisibilityType, which has three options: kImplicit (if the regular go <stack> form is used, so the visible property of the stack remains unchanged), kExplicitVisible (if the go visible <stack> form is used, so the visibility of the stack should be set to true) and kExplicitInvisible (if the go invisible <stack> form is used, so the visibility of the stack should be set to false). The visible variable of the MCGo class (declared on line 1740 of cmds.h) has not been removed as it appears to be used by several other parts of the engine.

The parameters on the _TestGoInvisibleVisibleStack and _TestCreateStack commands have been seperated by a comma.

runrevmark · 2018-08-16T16:42:50Z


-void MCInterfaceExecGoCardAsMode(MCExecContext& ctxt, MCCard *p_card, int p_mode, bool p_visible, bool p_this_stack);
-void MCInterfaceExecGoCardInWindow(MCExecContext& ctxt, MCCard *p_card, MCStringRef p_window, bool p_visible, bool p_this_stack);
+enum MCGoStackVisibility { kImplicit, kExplicitVisible, kExplicitInvisible };


Enums should be laid out as:

enum <name> { ... };

i.e. always prefer multi-line constructs, and always ensure braces are on their own line (this is generally true for new code - when merging in with older code it depends on more subjective assessment of readability of the resulting code).

Also the name should be MCInterfaceExecGoVisibility (notionally 'Interface' is a separate module so MCInterface prefixes all things in that module, and this is related to only a specific method - ExecGo - and friends), and the names of the enumeration values should be prefixed by kMCInterfaceExecGoVisibility.

The reason for the above is that all these things exist in the global namespace - so brevity only potentially causes namespace conflicts later on!

P.S. I edited the name slightly when I realized this was pertinent to 'ExecGoCard' - rather than 'GoStack' :)

The readibility of the MCGoStackVisibility enum has been increased by renaming it to MCInterfaceExecGoVisibility, and prepending the new enum name to all the enumeration values. Also the use of braces in the enum definition has been updated to conform to the existing style (note to self: read C++style.md *first* next time).

A release note has been added for the new visible form of the go command.

livecodeali · 2018-08-17T09:39:41Z

 	Boolean marked;
 	Boolean visible;
-	Boolean explicit_visibility = False;
+	MCGoStackVisibility visibilityType = kImplicit;


Don't initialise here, do it in the constructor below where the other members are initialised

Oh and you can remove the visible Boolean!

The visibiltyType variable has been renamed to visibility_type to conform to the coding standards. An unnecessary instance of the MCInterfaceExecGoVisibilityType enum was removed and replaced with a direct reference to the enum. The visibile boolean variable is now redundant and has been removed.

livecodeali · 2018-08-17T15:18:33Z

 		MCdefaultstackptr->checkdestroy();
 	}
-	MCInterfaceExecGo(ctxt, p_card, nil, 0, false, true);
+	MCInterfaceExecGo(ctxt, p_card, nil, 0, false, MCInterfaceExecGoVisibility(kMCInterfaceExecGoVisibilityImplicit));


The cast shouldn't be needed here should it? You can just pass kMCInterfaceExecGoVisibilityImplicit

A cast to the MCInterfaceExecGoVisibility enum has been replaced with a direct pass of the relevent item in the enum.

livecodeali · 2019-04-17T15:16:11Z

@livecode-vulcan review ok b875fbd

livecode-vulcan · 2019-04-17T15:19:25Z

💙 review by @livecodeali ok b875fbd

[[Bugfix 14721]] Implement 'go visible <stack>' as an antonym to 'go invisible' The go visible command has been added, along with a new parser test for the go command (seemingly it did not have any parser tests before?). The documentation and functional tests have been updated to take account of the new option.

livecode-vulcan · 2019-04-17T17:24:41Z

😎 test success b875fbd

try-community-armv6-android-sdk26_ndk16r15: success
try-community-armv7-android-sdk26_ndk16r15: success
try-community-arm64-android-sdk26_ndk16r15: success
try-community-x86-android-sdk26_ndk16r15: success
try-community-x86_64-android-sdk26_ndk16r15: success
try-community-js-emscripten-sdk1.35: success
try-community-universal-ios-iphoneos12.1: success
try-community-universal-ios-iphonesimulator12.1: success
try-community-universal-mac-macosx10.9: success
try-community-x86-linux-debian8: success
try-community-x86_64-linux-debian8: success
try-community-x86-win32: success
try-community-x86_64-win32: success

andrewferguson added 3 commits August 16, 2018 12:57

livecodeali suggested changes Aug 16, 2018

View reviewed changes

andrewferguson added 2 commits August 16, 2018 16:43

[[Bugfix-14721]] Seperate parameters by comma

b46a43f

The parameters on the _TestGoInvisibleVisibleStack and _TestCreateStack commands have been seperated by a comma.

runrevmark reviewed Aug 16, 2018

View reviewed changes

andrewferguson added 2 commits August 17, 2018 10:14

[[Bugfix-14721]] Add release note

e1a84ae

A release note has been added for the new visible form of the go command.

livecodeali reviewed Aug 17, 2018

View reviewed changes

[[Bugfix-14721]] Removed unnecessary cast to enum

b875fbd

A cast to the MCInterfaceExecGoVisibility enum has been replaced with a direct pass of the relevent item in the enum.

livecodeali approved these changes Apr 17, 2019

View reviewed changes

livecode-vulcan added the vulcan/reviewed label Apr 17, 2019

livecodepanos added this to the 9.1.0-dp-1 milestone May 2, 2019

runrevmark merged commit 54162ff into livecode:develop May 8, 2019

Navigation Menu

Uh oh!

[[Bugfix 14721]] Implement 'go visible <stack>' as an antonym to 'go invisible' - #6641

[[Bugfix 14721]] Implement 'go visible <stack>' as an antonym to 'go invisible'#6641
runrevmark merged 9 commits into
livecode:developfrom
andrewferguson:bugfix-14721

andrewferguson commented Aug 16, 2018

Uh oh!

livecodeali Aug 16, 2018

Uh oh!

andrewferguson Aug 16, 2018

Uh oh!

livecodeali Aug 16, 2018

Uh oh!

runrevmark Aug 16, 2018 •

edited

Loading

Uh oh!

livecodeali Aug 17, 2018

Uh oh!

livecodeali Aug 17, 2018

Uh oh!

livecodeali Aug 17, 2018

Uh oh!

livecodeali commented Apr 17, 2019

Uh oh!

livecode-vulcan commented Apr 17, 2019

Uh oh!

livecode-vulcan commented Apr 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

andrewferguson commented Aug 16, 2018

Uh oh!

livecodeali Aug 16, 2018

Choose a reason for hiding this comment

Uh oh!

andrewferguson Aug 16, 2018

Choose a reason for hiding this comment

Uh oh!

livecodeali Aug 16, 2018

Choose a reason for hiding this comment

Uh oh!

runrevmark Aug 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

livecodeali Aug 17, 2018

Choose a reason for hiding this comment

Uh oh!

livecodeali Aug 17, 2018

Choose a reason for hiding this comment

Uh oh!

livecodeali Aug 17, 2018

Choose a reason for hiding this comment

Uh oh!

livecodeali commented Apr 17, 2019

Uh oh!

livecode-vulcan commented Apr 17, 2019

Uh oh!

livecode-vulcan commented Apr 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

runrevmark Aug 16, 2018 •

edited

Loading