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

Some old section permalinks are not valid anymore #2070

Closed
bendtherules opened this issue Jun 30, 2020 · 9 comments · Fixed by #2071
Closed

Some old section permalinks are not valid anymore #2070

bendtherules opened this issue Jun 30, 2020 · 9 comments · Fixed by #2071

Comments

@bendtherules
Copy link
Contributor

bendtherules commented Jun 30, 2020

Some old section permalinks used in the spec are not valid anymore. Can we map them back?

I found these while looking through the engine262 codebase (engine262/engine262#106) . Here is a list of those invalid ids - checked against the current hosted version.

Same gist contains a script to test against any version of the spec - that can be used to figure out when it did exist.

  1. Should I try to make a table showing when these ids existed and what they should map to now?
  2. What is the mechanism to make these links valid again?

I'll be happy to raise a PR or help investigate this further.

@jmdyck
Copy link
Collaborator

jmdyck commented Jun 30, 2020

Some old section permalinks used in the spec are not valid anymore. [...] Here is a list of those invalid ids

I ran git log -S (and, just to make sure, git log -G) on all of these. Most returned no results, suggesting that these strings have never existed on the master branch. Probably they come from a proposal or a PR. E.g., sec-weakref-execution comes from engine262's reference to
https://tc39.es/proposal-weakrefs/#sec-weakref-execution
You should exclude such documents from your collection process. (Or collect them, but be sure to check them against the ids defined in that document, not the main spec.)

Of the ones that do return results, most are just truncations of valid ids. E.g.
sec-proxy-object-internal-methods-and-internal-slots-call-thisargument-argume
is a truncation of
sec-proxy-object-internal-methods-and-internal-slots-call-thisargument-argumentslist
(The truncation appears to be a bug in your collection process; engine262 has 2 occurrences of the correct id, but none of the truncated form.)

And git log -S shows hits for sec-promise. and sec-string.prototype. because they're prefixes of lots of valid ids, but it's pretty unlikely that they themselves were ever ids, because of the trailing dot. (Again, this appears to be a bug in your collection process, as those strings don't appear as complete ids in the engine262 source.)

If you revise your collection process, I suspect the only cases it will find will be:
sec-evaluate-expression-key-property-access
sec-evaluate-identifier-key-property-access
which existed on master for a couple months, between the merges of PR #1646 and PR #1787.

2. What is the mechanism to make these links valid again?

It would be to add an "oldids" attribute to the appropriate HTML element.

@bendtherules
Copy link
Contributor Author

bendtherules commented Jun 30, 2020

@jmdyck Thanks for the detailed look. There were indeed lot of problems with my collection.

I have tried to fix them now and verified the items manually. This is the shorter 6-item list -

// first 3 from proposal ??
sec-numeric-types-bigint-unsighedRightShift
sec-allocatedtypedarray
sec-typedarray-species-create
// 
sec-running-execution-context
sec-evaluate-expression-key-property-access
sec-evaluate-identifier-key-property-acces

First 3 items look like they were recently added. Can those be actually proposal sections? @devsnek.
Rest look like old sections (including the two you mentioned).

@devsnek
Copy link
Member

devsnek commented Jun 30, 2020

- sec-numeric-types-bigint-unsighedRightShift
+ sec-numeric-types-bigint-unsignedRightShift

- sec-allocatedtypedarray
+ sec-allocatetypedarray

- sec-typedarray-species-create
+ typedarray-species-create

- sec-running-execution-context
+ running-execution-context

- sec-evaluate-expression-key-property-access
+ sec-evaluate-property-access-with-expression-key

- sec-evaluate-identifier-key-property-acces
+ sec-evaluate-property-access-with-identifier-key

@bendtherules
Copy link
Contributor Author

@devsnek So first 4 are typos on engine262 repo and rest 2 are still old ids?

@devsnek
Copy link
Member

devsnek commented Jun 30, 2020

@bendtherules looks like it.

@ljharb
Copy link
Member

ljharb commented Jun 30, 2020

a PR to fix those 2 oldIDs would be appreciated.

bendtherules pushed a commit to bendtherules/ecma262 that referenced this issue Jun 30, 2020
For property access with expression and identifier, add oldids - sec-evaluate-property-access-with-expression-key, sec-evaluate-property-access-with-identifier-key

Related to tc39#2070
bendtherules added a commit to bendtherules/ecma262 that referenced this issue Jun 30, 2020
For property access with expression and identifier, add oldids - sec-evaluate-property-access-with-expression-key, sec-evaluate-property-access-with-identifier-key

Related to tc39#2070
@bendtherules
Copy link
Contributor Author

@ljharb Added PR #2071 😀

bendtherules added a commit to bendtherules/ecma262 that referenced this issue Jun 30, 2020
For property access with expression and identifier, add oldids - sec-evaluate-property-access-with-expression-key, sec-evaluate-property-access-with-identifier-key

Related to tc39#2070
@bakkot
Copy link
Contributor

bakkot commented Jul 5, 2020

If you revise your collection process, I suspect the only cases it will find will be:
sec-evaluate-expression-key-property-access
sec-evaluate-identifier-key-property-access
which existed on master for a couple months, between the merges of PR #1646 and PR #1787.

For anyone else confused by why #1787 doesn't appear to touch these IDs, it's because the relevant commit, 384978f, was pulled in apart from the rest of that PR.

ljharb pushed a commit to bendtherules/ecma262 that referenced this issue Jul 5, 2020
For property access with expression and identifier, add oldids - sec-evaluate-property-access-with-expression-key, sec-evaluate-property-access-with-identifier-key

Related to tc39#2070
@jmdyck
Copy link
Collaborator

jmdyck commented Jul 5, 2020

For anyone else confused by why #1787 doesn't appear to touch these IDs, it's because the relevant commit, 384978f, was pulled in apart from the rest of that PR.

Ah right, I forgot. So the 'old' ids from 1646 only existed on master for about 3 weeks.

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

Successfully merging a pull request may close this issue.

5 participants