-
Notifications
You must be signed in to change notification settings - Fork 34
missing instructions in Node Map generation #549
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
Comments
Sorry, a bit confused. Step 6.10 is the following:
In fact, when searching the document (both TR and ED), I can't find a sentence "if node map does not contain an id entry, create one and initialize its value to an empty map". The algorithm takes node map as an input argument, so it definitely must already exist. The places which recursively invoke the algorithm pass the existing node map, so I don't see the specific issue. |
The sentence I wrote ("if node map does not contain an id entry...") is what I suggest to add to step 6.10 . So indeed, it is currently not in the spec :-) Of course node map always exists. What does not always exist, when entering 6.10, is an entry in node map whose key is id. When the algorithm is recursively invoked, passing node map and passing id for active graph, this entry is assumed to exist at step 2. By adding the suggested sentence in 6.10 before the recursive invocation, we ensure that this assumption in the invoked step 2 is satisfied. |
I agree that what @pchampin suggests seems to be needed. (I've made a similar note in my implementation.) I've later on also noticed that this node map generation algorithm mutates the input. Several steps (6.7, 6.9, 6.10, 6.11) says to remove an entry from the element. Therefore, I think step 2, where element is a map, also needs to make a (shallow) copy of this element, which is then subsequently mutated. (In 6.12, the remaining keys of the element are used, so there are other ways of fixing this; making a copy is just the easiest change to the current design.) |
I believe that an instruction is missing from step 6.10 of Node Map Generation, namely:
before recursively invoking the algorithm.
Indeed, in step 2 of the invoked algorithm, that entry is assumed to already exist:
(edited wrong markup, which made the suggested sentence potentially harder to understand)
The text was updated successfully, but these errors were encountered: