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

Code Import #256

Merged
merged 13 commits into from
Oct 11, 2024
Merged

Code Import #256

merged 13 commits into from
Oct 11, 2024

Conversation

heyAyushh
Copy link
Contributor

@heyAyushh heyAyushh commented Jul 5, 2024

Problem

Solves #170

Summary of Changes

  • added a script to import code files

  • does supports dev and build mode for md and mdx files

  • supports this syntax (ignore square brackets)
    [```]language file="filepath.ext" .....
    [`` `]

  • added pre-build script, before building, import code files and then builds contentlayer project

  • added dev script to import code files at development

  • to import code use yarn code-import

Example

markdown file

code file

@heyAyushh heyAyushh requested a review from nickfrosty as a code owner July 5, 2024 18:30
@nickfrosty
Copy link
Collaborator

this is a very cool and nice feature you are adding!

a few things I noticed without performing a full review:

  • you are only checking for files via git
    • not all content files are stored in git. specifically translation files, which are pulled from crowdin at build time and stored in the /i18n/** directory (which is ignored by git)
  • you are only checking .md files
    • we also use .mdx files that will have code snippets in them. so we need to make sure we collect all content files in the /content directory and the /i18n directory
    • I manually adjusted the getMarkdownFiles function to look at the mdx files and as I suspected, the code script injects extra content which breaks the mdx render. this is a must fix! (also to note that any .md file can also use our custom components or standard html components)
  • when I declare an code block with a file metadata that starts with a slash, it does not load the content
    • this should be supported. in fact, I think this should be required in order to auto load an external file like this, it should start with a / that represents the root of the repo. this way things stay consistent and are easier to maintain and test
    • then when we add support for external repos and urls, they can simply start with http:// and it is easily detected too
  • you are proposing all code blocks be updated to have empty contents, which I think will result in the prettier formatting check to fail if saved/committed as such
    • an easy solution here is to add a single line of text that is the file name to load in. and then this content will be replaced by the script
  • you are using the tsx cli tool which gets pretty flakey
    • lets use npx esrun instead since it is more reliable and works on more platforms

coder.ts Outdated Show resolved Hide resolved
coder.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@mikemaccana mikemaccana left a comment

Choose a reason for hiding this comment

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

Looking forward to getting this is - I love being able to use real files in our code examples (and run CI on them!). I'll leave this to @nickfrosty to sign off on, as he has more a strategic idea for .com, but here are some small and hopefully useful code suggestions, everything else looks great.

src/utils/code-import.ts Outdated Show resolved Hide resolved
@heyAyushh heyAyushh reopened this Aug 3, 2024
@heyAyushh
Copy link
Contributor Author

heyAyushh commented Aug 3, 2024

No markdown file will be empty with code blocks,
Instead it will be synced and updated with codeblock everytime build or dev happens.

src/utils/code-import.ts Outdated Show resolved Hide resolved
src/utils/code-import.ts Outdated Show resolved Hide resolved
content/cookbook/wallets/check-publickey.md Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
code/pnpm-lock.yaml Outdated Show resolved Hide resolved
code/package.json Show resolved Hide resolved
.husky/pre-commit Show resolved Hide resolved
coder.ts Show resolved Hide resolved
@heyAyushh
Copy link
Contributor Author

@nickfrosty shall we update this repo to use pnpm instead of yarn?

@heyAyushh
Copy link
Contributor Author

I am thinking of adding a code import chaining feature where we can chain the Lines to add multiple lines of codeblocks from the same file, will that be good idea?

eg. ```file=\code\hello.js#3-4#6-10#11-15

this way code files can exist as a test file instead of the exact code.

coder.ts Outdated Show resolved Hide resolved
coder.ts Show resolved Hide resolved
@nickfrosty nickfrosty merged commit 17cc7d3 into solana-foundation:main Oct 11, 2024
2 checks passed
adpthegreat pushed a commit to adpthegreat/developer-content that referenced this pull request Oct 23, 2024
* code import refactor, sync and watch mode

* husky precommit lint hook

* mdx support

* debug support

* ignore node modules

* optional quotes and line chaining

* husky deprecation changes

* use fs/promises

* ignore contributing.md

* package merge resolve

* ignore pnpm lock

* refactor: scripts

---------

Co-authored-by: nickfrosty <[email protected]>
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