Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung merged PR #1789: URL: https://github.com/apache/apisix-website/pull/1789 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
yzeng25 commented on PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#issuecomment-2132810196 PTAL @SkyeYoung -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
siyaramaa commented on PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#issuecomment-2102198515 Thank you, I have just made the update. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1590493465 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: @siyaramaa Yes. Sorry for replying so late. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1590493465 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: Yes. Sorry for replying so late. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1590493465 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: yes -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1590493376 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} Review Comment: I think it's okay. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
siyaramaa commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1586245778 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} Review Comment: Thanks for the review, @SkyeYoung. I'll move that code up and use `if (!path?.includes(INDEX_URL))` as you suggested. By the way, I'm currently using `INDEX_URL` as `https://github.com/apache/apisix` is that okay? Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
siyaramaa commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1586242910 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: Hi @SkyeYoung, Thanks for explaining. I finally understand how it's supposed to work, I had misunderstood before. The modified logic should redirect the user to the index URL if it doesn't exist. Should I revert it back to the logic of redirecting to GitHub create new file? Thanks again. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1584227559 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: This logic will actually jump to the github url of "Create a new file". The working logic is: The document site uses English as fallback, and when documents in Chinese or other languages do not exist, they will automatically be displayed in English. But at this time, `edit url` will still use the path of the corresponding language. For example, `/xxx` exists, but the document `/zh/xxx` does not exist. Assume that the edit url of the latter is `github/zh/xxx`. Since the document does not exist at this time, the content in `/xxx` will be displayed, but the edit url Still `github/zh/xxx`. Apparently at this point, (at least I thought so,) clicking this link should create this non-existent `/zh/xxx`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1584232401 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: Here is a practical example, please find the edit url link in the page https://apisix.apache.org/zh/docs/apisix/tutorials/manage-api-consumers/ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1584230327 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: If you follow your modified logic, it should jump directly to a non-existent path, causing github to display 404. So I think this current logic needs to be adjusted. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1584227559 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: This logic will actually jump to the github url of "Create a new file". The working logic is: The document site uses English as fallback, and when documents in Chinese or other languages do not exist, they will automatically be displayed in English. But at this time, `edit url` will still use the path of the corresponding language. For example, `/xxx` exists, but the document `/zh/xxx` does not exist. Assume that the edit url of the latter is `github/zh/xxx`. Since the document does not exist at this time, the content in `/xxx` will be displayed, but the edit url Still `github/zh/xxx`. Apparently at this point, (at least I thought so,) clicking this link should create this non-existent `/zh/xxx`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1584227559 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: This logic will actually jump to the github url of "Create a new file". The working logic is: The document site uses English as fallback, and when documents in Chinese or other languages do not exist, they will automatically be displayed in English. But at this time, `edit url` will still use the path of the corresponding language. (For example, /xxx exists, but the document /zh/xxx does not exist. Assume that the edit url of the latter is github/zh/xxx. Since the document does not exist at this time, the content in /xxx will be displayed, but the edit url Still github/zh/xxx. Apparently at this point, at least I thought so, clicking this link should create this non-existent /zh/xxx). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1584217518 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} Review Comment: You can move this part of the code up 24 lines, you should not need to `setIsLoading(false)` this way. Also `if(xxx)` can be written as `if(!path?.includes(INDEX_URL))` 樂 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
SkyeYoung commented on PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#issuecomment-2084493789 Thanks for the contribution! The logic should be fine. cc @bzp2010 @guoqqqi pls review this -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
siyaramaa commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1581818500 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: Sure, Thank you for your review, @pottekkat. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
pottekkat commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1581789130 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: I'm not sure, I will check with @SkyeYoung -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
siyaramaa commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1581768238 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: Hi, It looks like the code isn't creating new files. I think it's attempting to replace the path, for example, from '/edit/#githubrepolink' to '/new/?filename=githubrepolink' if it doesn't exist. But, we're using getPath = () => window.location.hash.slice(1); to retrieve the path, which only gives us the hash - "githubrepolink'". So, it seems that this logic isn't being utilized. Please correct me if I'm wrong. Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
pottekkat commented on code in PR #1789: URL: https://github.com/apache/apisix-website/pull/1789#discussion_r1579920153 ## doc/src/pages/edit.tsx: ## @@ -20,18 +22,26 @@ const Edit: FC = () => { const path = getPath(); setIsLoading(true); + +if (path === '' || !path.includes('https://github.com/apache')) { + setPathExist(false); + setIsLoading(false); + return; +} + fetch(path.replace('github.com', 'raw.githubusercontent.com').replace('/edit', '')) .then((res) => setPathExist(res.status !== 404)) .finally(() => setIsLoading(false)); }, []); const edit = useCallback(() => { -let path = getPath(); +const path = getPath(); + if (!pathExist) { - const pathArr = path.replace('edit', 'new').split('/'); - pathArr[pathArr.length - 1] = `?filename=${pathArr.at(-1)}`; - path = pathArr.join('/'); Review Comment: Not sure if this is ever run but it seems like it is for creating a new file if doesn't exist? Should we replace this logic? Maybe instead we can add to this logic and set a new condition for an empty path. Because path does not exist and path is empty are different. Again, I'm not sure if it is ever run so IDK if it would be any problem. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] Fix: Added redirection to GitHub index when no hash present [apisix-website]
siyaramaa opened a new pull request, #1789: URL: https://github.com/apache/apisix-website/pull/1789 Fixes: #1783 Changes: Added redirection to the GitHub repository index when the path variable is empty. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org