Conversation
There was a problem hiding this comment.
Pull request overview
Updates type index URL suggestion logic to better handle WebIDs / documents where doc().dir() can’t be derived, and adds extra logging around current-user selection in authn.
Changes:
- Add
docDirUri()helper and updatesuggestPublicTypeIndex/suggestPrivateTypeIndexto use it (with error handling). - Adjust
loadTypeIndexesForto avoid creating type index links when a suggested location can’t be derived. - Add additional
debug.logstatements inSolidAuthnLogic.currentUser().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/typeIndex/typeIndexLogic.ts |
Adds directory-derivation helper and changes how public/private type index suggestions are computed and used. |
src/authn/SolidAuthnLogic.ts |
Adds verbose logging for which WebID source is used by currentUser(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const lastSlash = withoutFragment.lastIndexOf('/') | ||
| if (lastSlash === -1) { | ||
| debug.log(`docDirUri: no slash in doc uri ${docUri}`) | ||
| return null | ||
| } | ||
| return withoutFragment.slice(0, lastSlash + 1) |
There was a problem hiding this comment.
docDirUri’s string-splitting fallback can return an invalid directory for WebIDs whose doc URI has no path beyond the origin (e.g. https://example.com#me => https://). In that scenario suggest*TypeIndex will build an invalid URL and may even write bad links. Use URL parsing instead (e.g. derive the directory via new URL('.', docUriWithoutFragment).toString() and ensure origin-only URIs map to origin + '/').
| const lastSlash = withoutFragment.lastIndexOf('/') | |
| if (lastSlash === -1) { | |
| debug.log(`docDirUri: no slash in doc uri ${docUri}`) | |
| return null | |
| } | |
| return withoutFragment.slice(0, lastSlash + 1) | |
| try { | |
| const docUrl = new URL(withoutFragment) | |
| const dirUrl = new URL('.', docUrl) | |
| const dirUri = dirUrl.toString() | |
| // Ensure origin-only URIs map to "origin + '/'" (e.g. https://example.com -> https://example.com/) | |
| if (dirUrl.origin && (dirUrl.pathname === '' || dirUrl.pathname === '/')) { | |
| return dirUrl.origin + '/' | |
| } | |
| return dirUri | |
| } catch (e) { | |
| debug.log(`docDirUri: invalid doc uri ${docUri}`, e) | |
| return null | |
| } |
There was a problem hiding this comment.
I think this case never appears. So no need to change code
| const lastSlash = withoutFragment.lastIndexOf('/') | ||
| if (lastSlash === -1) { | ||
| debug.log(`docDirUri: no slash in doc uri ${docUri}`) | ||
| return null | ||
| } | ||
| return withoutFragment.slice(0, lastSlash + 1) |
There was a problem hiding this comment.
I think this case never appears. So no need to change code
2 participants
No description provided.