Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #2154 #2157

Merged
merged 3 commits into from
Oct 31, 2017
Merged

Fixes #2154 #2157

merged 3 commits into from
Oct 31, 2017

Conversation

Dunbaratu
Copy link
Member

@Dunbaratu Dunbaratu commented Oct 29, 2017

Fixes #2154

TODO list before this becomes ready:

  • - user docs need to talk about function's scoping changes and the local file scope changes.
  • - breaking warning drafted about local scope for the file now existing.
  • - breaking warning drafted about parameters having been global before but not now.

This change wraps a scope around a file (similar to putting an opening brace at the top of the file and a closing one at the bottom) so the file's local variables are really local, and so the functions declared in the file have proper scope captured in their closures.

Doc changes needed:

Note, these will need to be fleshed out with good examples - these sentences below are just bullet-point reminders:

  • Files now have a scope. (breaking: Previously saying local foo is 1. or global foo is 1. meant the same thing at the file level. Now they differ as one would expect. lazyglobal will still make globals as it did before when you set an unknown variable name. This only affects what happens when you explicitly say local - now it works - before it just became global anyway because the file has no scope.)
  • Even though it's slightly wrong, to preserve backward compatibility functions declared at the file scope level will be call-able from other files because their function pointer variable will have global scope by default. You can override this by explicitly saying the function is local when you declare it.
  • However, UNLIKE before, even though the function's name can be seen and called from other files, its closure context inside its body will now be local as it should have been, so it can access local variables declared in the same file.
  • With the explicit global keyword, you now should be able to make a function's name globally call-able, even when it is declared deeply nested in other things (this is consistent with using 'global' for variable identifiers). It will still have local scope closure inside its body, however. (This used to not be allowed - the global keyword was forbidden for function declarations.)

TODO list before this becomes ready:

[ ] - user docs need to talk about function's scoping changes.
[ ] - breaking warning drafted about parameters having been global before but not now.
@Dunbaratu Dunbaratu added Breaking Some user scripts that used to work will break (even if just in a minor way). bug Weird outcome is probably not what the mod programmer expected. Not Ready Author is warning Reveiwer not to merge this PR yet. More edits are expected. refactor An infrastructure change that shouldn't change end result. labels Oct 29, 2017
@Dunbaratu Dunbaratu changed the title Fixes #2141 Fixes #2154 Oct 29, 2017
@Dunbaratu Dunbaratu removed the Not Ready Author is warning Reveiwer not to merge this PR yet. More edits are expected. label Oct 30, 2017
@Dunbaratu
Copy link
Member Author

Dunbaratu commented Oct 30, 2017

These doc edits are enough to make it merge-able, but you could argue we need to make a better example to really nail it down. I don't want to make that example right now because it will take a bit of time and this is holding up everyone else working on kOS.

A proper documentation example (maybe in another docs-only PR?) will show examples of all the following in action:

Assuming File1 ran FIle2, give File2 all these example cases in it:

  • some vars local to File2's outer scope that File1 cannot use.
  • global function declared at File2's outer scope - show that File1 can run it, and that this function can see File2's vars even when invoked from File1 (this shows file scope closure).
  • local function declared at File2's outer scope - show that File1 cannot run it.
  • function declared at File2's outer scope that mentions neither global nor local keyword - show that it behaves like it had the global keyword.
  • local function declared inside some braces in File2. Show it cannot be called from FIle1.
  • global function declared inside some braces in File2. Show it can be called from FIle1 despite being in some braces of File2 (the global keyword overriding default behaviour like it does for variables). Show it being able to access local vars of FIle2 even though File1 cannot do so (again proving closure).
  • function declared without a local or global keyword inside braces of FIle2 - show that it behaves just like if local had been put there.

All the above are explained in the doc edits I just did, but there aren't good examples of them shown. I feel like we should make that example, but it will take time to get it just right, and I don't want to hold up you guys any longer with that.

(And I just did spend a long time typing in that example, and it ended up getting wiped out because I was sloppy with git and wiped it before it got committed so I'm demoralized about typing it all in again right now.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Some user scripts that used to work will break (even if just in a minor way). bug Weird outcome is probably not what the mod programmer expected. refactor An infrastructure change that shouldn't change end result.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants