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

Give priority to href over edDraft in reverse lookup #452

Merged
merged 1 commit into from Apr 9, 2018

Conversation

tidoust
Copy link
Collaborator

@tidoust tidoust commented Apr 9, 2018

Fix for #450.

Code that populated the reverse lookup table seemed to be willing to give priority to href over edDraft, but it had 2 bugs:

  1. it checked for edDraft in the properties of the output object not to override a previous entry, but the entry that is inserted is normalizeUrl(edDraft), not edDraft
  2. it did not add all href entries before looking at edDraft.

New test that makes sure that the returned ref is always the one that matches href if there is one added.

Fix for tobie#450.

Code that populated the reverse lookup table seemed to be willing to give
priority to `href` over `edDraft`, but it had 2 bugs:
1. it checked for `edDraft` in the properties of the output object not to
override a previous entry, but the entry that is inserted is
`normalizeUrl(edDraft)`, not `edDraft`
2. it did not add all `href` entries before looking at `edDraft`.

New test that makes sure that the returned ref is always the one that matches
`href` if there is one added.
@tobie tobie merged commit 9ef77bb into tobie:master Apr 9, 2018
@tobie
Copy link
Owner

tobie commented Apr 9, 2018

Good catch! Thanks a bunch. And thanks for the tests!

A new version of Specref updated with these changes should auto-deploy within minutes. You can search www.specref.org to check for it.

Don't hesitate to comment back here if you can't find your changes within 10 minutes; it's a sign something is off and I'll immediately look into it. Thanks!

Specref loosely follows the process described in The Pull Request Hack. You've thus been granted commit access to the repo.

Please read-up on how to make manual changes, get non-trivial changes reviewed by someone (I'm sure you'll be a good judge of when that's necessary) and feel free to ask if something's unclear.

Thanks again!

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 this pull request may close these issues.

None yet

2 participants