-
Notifications
You must be signed in to change notification settings - Fork 2
Fix editLink for products not accounting for orgName from repositories.json #74
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
Conversation
…epositories.json Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Join our Discord community for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
.vitepress/config.mts
Outdated
| @@ -20,7 +20,8 @@ export default defineConfig({ | |||
| const repositoriesMap: Map<string, RepoData> = new Map(repositories.map((repoData: RepoData) => [repoData.repository, repoData])); | |||
|
|
|||
| const repoName = pageData.relativePath.split('/')[1]; | |||
| const repoData = repositoriesMap.get(repoName); | |||
| const cleanRepoName = repoName?.replace(/\d+-/g, ''); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inlined the variable as requested in commit a6453c2
| const organization = pageData.params?.organization; | ||
|
|
||
| const [_, repoName, ...rest] = filePath.split('/') | ||
| // Use relativePath to get the original repository name (before rewrites) | ||
| const [_, repoName, ...rest] = relativePath.split('/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
что-то не сходится. раз это до рерайтов, значит, здесь имя репозитория будет с цифрами в начале. в filePath цифр уже быть не должно, а значит и replace строкой ниже уже не нужен.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right about the logic. I'm using relativePath (before rewrites) so the repo name has prefixes like 003-extends, which is why I need the replace(/\d+-/g, '') on line 164. This is consistent with transformPageData which also uses relativePath and cleans the name for organization lookup. Would you prefer I use filePath instead and remove the replace, or keep the current consistent approach?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Используй filePath
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to use filePath instead of relativePath and removed the manual cleaning since filePath is already processed by URL rewrites. Commit 816719e
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
.vitepress/config.mts
Outdated
| return `https://github.com/${orgName}/${repoNamePath}/edit/master/docs/product/${restPath}` | ||
| } | ||
| return '' | ||
| // For products (after rewrites, products/ prefix is removed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
нене, вот здесь (и на 160 строке) как раз можно проверить relativePath. там же должно быть и /api и /product по идее.
ну и вырни обратно обший расчет repoName, orgName и restPathю они же одинаковые в таком случае
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to use relativePath for path checks and restored common calculation of repoName, orgName and restPath as requested. Commit 718e717
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vitepress хостит не только /api и /product. Верни проверку на product обратно
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added back specific check for relativePath.startsWith('products/') instead of using catch-all. Now properly handles cases where VitePress hosts paths other than /api and /product by returning empty string for unmatched paths. Commit f9a40a6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added back specific check for relativePath.startsWith('products/') instead of using catch-all. Now properly handles cases where VitePress hosts paths other than /api and /product by returning empty string for unmatched paths. Commit f9a40a6
…s requested Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
.vitepress/config.mts
Outdated
| @@ -20,7 +20,7 @@ export default defineConfig({ | |||
| const repositoriesMap: Map<string, RepoData> = new Map(repositories.map((repoData: RepoData) => [repoData.repository, repoData])); | |||
|
|
|||
| const repoName = pageData.relativePath.split('/')[1]; | |||
| const repoData = repositoriesMap.get(repoName); | |||
| const repoData = repositoriesMap.get(repoName?.replace(/\d+-/g, '')); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move replace to the line with repoName calculation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved replace operation inline with repoName calculation in both transformPageData and editLink pattern. Now uses repoName?.replace(/\d+-/g, '') directly in the assignment. Commit f9a40a6
.vitepress/config.mts
Outdated
| return `https://github.com/${orgName}/${repoNamePath}/edit/master/docs/product/${restPath}` | ||
| } | ||
| return '' | ||
| // For products (after rewrites, products/ prefix is removed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vitepress хостит не только /api и /product. Верни проверку на product обратно
Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
|
@copilot после твоих правок edit link в разделе с описанием продуктов вообще не работает. Api работает. |
The editLink generation for product pages was not correctly using the organization name from
repositories.json, causing external repositories (likenixel2007/extends) to generate incorrect GitHub edit links pointing toautumn-libraryinstead of their actual organization.Issue
For the repository
nixel2007/extends, the generated edit link was:Instead of the correct:
Root Cause
Two issues were causing this problem:
transformPageData was not cleaning repository names - it used the raw name with sorting prefix (e.g.,
003-extends) to look up organization data, butrepositories.jsonmaps clean names (e.g.,extends)editLink pattern was using
pageData.filePath(rewritten path after URL rewrites) instead ofpageData.relativePath(original path before rewrites) to extract repository namesSolution
transformPageDatato clean repository names by removing the sorting prefix (\d+-) before looking up organization dataeditLink patternto usepageData.relativePathfor repository name extraction, ensuring consistency withtransformPageDataTesting
Verified that:
nixel2007/extendsnow generates correct edit links withnixel2007organizationFixes #73.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.