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

render readme html scrapped via the server #781

Merged
merged 5 commits into from
Aug 18, 2023
Merged

render readme html scrapped via the server #781

merged 5 commits into from
Aug 18, 2023

Conversation

getneil
Copy link
Contributor

@getneil getneil commented Aug 18, 2023

DEMO: https://www.loom.com/share/71fab2e24ffe442da15102d62d76867c

  • we probably need to make the backend ai generation part more reliable, some packages still dont have an html readme because they dont have a public git repo
  • only works for those packages with generated readme
  • if readme.html failed from s3, gui manually fetches data from github or gitlab
  • show images for relative path src

@getneil getneil requested review from ABevier and ryanml August 18, 2023 00:44
@getneil getneil changed the title render html scrapped via the server render readme html scrapped via the server Aug 18, 2023
@getneil getneil marked this pull request as ready for review August 18, 2023 01:03
@getneil
Copy link
Contributor Author

getneil commented Aug 18, 2023

how do we handle if readme has been updated by the package maintainer? atm, readme will only rebuild on release of new build.

@getneil getneil marked this pull request as draft August 18, 2023 01:42
@getneil
Copy link
Contributor Author

getneil commented Aug 18, 2023

added issues:

or just do all correctly remotely which i prefer
https://github.com/teaxyz/api/issues/81

@getneil getneil marked this pull request as ready for review August 18, 2023 05:24
Copy link
Contributor

@ABevier ABevier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@ABevier ABevier merged commit da1710f into main Aug 18, 2023
6 checks passed
@ABevier ABevier deleted the s3-html-readme branch August 18, 2023 14:02
// TODO: support gitlab too
const changeSrcIfNeeded = (element: HTMLElement | Element) => {
const src = element.getAttribute("src");
if (src?.startsWith("/")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this single condition will cover all the bases for relative URLs. I had found most examples of a relative path in the Openssl, lefthook, and xcodegen libraries. Among those three you get srcs like:

assets/imgs/foo.png
/assets/imgs/foo.png
./assets/imgs/foo.png

I had begun working on a remedy to this issue yesterday before the s3 solution was mentioned/implemented, and a regular expression covered the bases for relative paths we'd run in to in READMEs. Something like:
const isRelativePath = src?.match(/^[^\/]+\/[^\/].*$|^\/[^\/].*$/i)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This regex could probably be simplified, as it covers conditions we won't likely run in to as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryanml ill add that thank you, i missed out on that hehehe

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.

3 participants