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

A namespace environment variable #37

Closed
6 of 8 tasks
jkomoros opened this issue Jul 5, 2023 · 1 comment
Closed
6 of 8 tasks

A namespace environment variable #37

jkomoros opened this issue Jul 5, 2023 · 1 comment

Comments

@jkomoros
Copy link
Owner

jkomoros commented Jul 5, 2023

It's best practice to attach a prefix (e.g. komoroske.com) to variable names in var/let, to store IDs, and memoryIDs. This helps ensure that different seeds from different authors don't stomp on each other's state.

But it's super tedious and annoying to add it everywhere within a file.

Add a namespace environment variable.

The behavior is:

When computing the variable name for var/let, the memory ID for memory, or the store ID for store, if the prefix property is not empty, and the var name is not a known environment name (skip this for storeID and memoryID), and the var does not already include something before a prefix, replace the variable name / memory ID / store ID with prefix:varname.

Another approach is to only do this for varNames etc that start with ..

When used in combination with packet-level environment (issue #38), the convention is to set the prefix at the top-level packet environment and never think about it again.

This expansion is done at the time of the seed being executed, not before. The pro is that we can late-bind the prefix (so, for example, you don't need to prepend it in the packet-level environment if you're also specifying a model, because it won't be prefixed until being used). The con is that static analysis tools that look at the seed graph will need to know about this behavior to appropriately figure out the variable names (and also perform lets/vars in that graph traversal).

Document in README.md how namespace works. Describe the inner workings, but then summarize: "Include a namespace in the environment of each packet. Whenever you need to reference a var, memory, or fact from another author, include author-prefix.com: in front, and otherwise don't think about it."

validateSeedPacket should verify that any namespace that is set does not include :.

How to handle _default_memory et al? In some cases you really do want to overlap in the commons. If you set a namespace, how do you say don't namespace this name for one of memory, or store, or whatever? Maybe the answer is that all of those memory/store ids are namespaced, just with :_default_store or whatever. And that way if you want to do a memory name that's not the same as the namespace you're using you can just manually configure :_default_store. (_default_store should have its name changed if we expect people to every so often type it...)

Originally tracked in #36.

  • Update the types for anything that takes a namespaced name to explicitly expect only up to a single :
  • Add a isNamespaced(input : StoreID | MemoryID | VarID) : boolean
  • Shouldn't type of letMulti be a record<varName, inputValue>`?
  • Make sure that empty store and memory ids work (and just make _default_store etc be '')
  • Update documentation, including base namespace
  • Update the examples to use this where appropriate
  • Document the explicti shared namespace as a commons
  • When did name-limerick break? (broke in cb74ab6, it is a object auto-expansion that's not working)
@jkomoros jkomoros changed the title A prefix environment variable A namespace environment variable Jul 6, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
It doesn't yet work in the way it's described.

Part of #37.
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
If the sub-object only had seed-references, it wouldn't notice it should unroll (it would only unroll if
it was a nested seed).

Added a test to catch this behavior and ensure it works right.

Part of #42. Noticed while working on #37.
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
@jkomoros
Copy link
Owner Author

jkomoros commented Jul 8, 2023

This is now done as of 78d6121

@jkomoros jkomoros closed this as completed Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant