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

github markdown block link fixes #191

Open
wants to merge 17 commits into
base: add/query-input-transforms
Choose a base branch
from

Conversation

shekharnwagh
Copy link
Contributor

@shekharnwagh shekharnwagh commented Nov 7, 2024

Description

Important

Depends on PR #198

This PR fixes the broken internal links to other markdown docs when using "GitHub File As HTML" block.

Changes

  • Remove unused patterns for "GitHub File As HTML" block
  • Allow slashes in overrides in url path when there is just one input variable. Earlier the overrides were delimited by slashes (/). For example in url page_slug/$first/$second $first and $second are assigned to first and second input variables respectively. This was a blocker in using the GitHub Doc Markdown file path as url overrides since the file path contains slashes. Now if there is just one input variable, all the url overrides are treated as just one input variable. For example page_slug/$dir/$path/$file will assign $dir/$path/$file to the input variable.
  • If input variables are missing file extension then its added to allow having url overrides without the .md suffix
  • Fix the internal links in the GitHub File As HTML block by replacing all the href attributes with local links ending in .md.

Testing

  • Start the local server using by following the instructions in the Docs.
  • Navigate to the page created in "GitHub File As HTML" example at the url http://localhost:8888/gh.
  • Edit the page to add a "GitHub File As HTML" block and set the url overrides to /gh/${file_path}.
  • Pass the file path of the markdown file in url like http://localhost:8888/gh/${file_path} (e.g. http://localhost:8888/gh/docs/workflows/google-sheets-with-code) and verify that the docs are rendered correctly and the internal links are working.

@shekharnwagh shekharnwagh self-assigned this Nov 7, 2024
@shekharnwagh shekharnwagh changed the title github markdown block link fixes [WIP] github markdown block link fixes Nov 7, 2024
@shekharnwagh shekharnwagh changed the title [WIP] github markdown block link fixes [WIP / Do Not Merge] github markdown block link fixes Nov 7, 2024
@shekharnwagh shekharnwagh changed the title [WIP / Do Not Merge] github markdown block link fixes github markdown block link fixes Nov 8, 2024
@shekharnwagh shekharnwagh marked this pull request as ready for review November 8, 2024 12:31
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maxschmeling What do you think about extracting the query classes to shared code in inc/Integrations/Github directory ? GitHub would have a static response format like Shopify so this is a good candidate for this, but the current implementation is also somewhat specific to markdown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logic to make it work without the .md in the path could go here -

in this file where we could append the .md if it is absent in the path in the input variables. We would need to make it generic though as this query is generic for GitHub and specific to just a markdown file.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should move this to the integrations folder yet, though it might make sense to do that at some point. I think I'd want to do more with it rather than something that is narrowly scoped to what we're using it for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've moved the markdown parsing/url update logic here in the query file and removed the GitHubParser file from integrations DIR.

@shekharnwagh shekharnwagh changed the base branch from trunk to add/query-input-transforms November 15, 2024 09:34
shekharnwagh and others added 8 commits November 15, 2024 19:25
Enhance GitHubGetFileAsHtmlQuery with optional file extension handling. The query now accepts a default extension parameter that automatically appends to file paths when missing. Updated registration to default to '.md' for markdown files, maintaining consistency with ListFilesQuery behavior.
Copy link
Contributor

@maxschmeling maxschmeling left a comment

Choose a reason for hiding this comment

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

I'm wondering if this parser logic belongs in the plugin or if it should go in custom configuration in the site code. I just wonder if it's generic enough. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should move this to the integrations folder yet, though it might make sense to do that at some point. I think I'd want to do more with it rather than something that is narrowly scoped to what we're using it for.

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.

2 participants