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

Add <include> block for including non-code files #370

Merged
merged 6 commits into from
May 1, 2023

Conversation

xenova
Copy link
Contributor

@xenova xenova commented Apr 29, 2023

Allow markdown files to include other (non-code) files. Useful to ensure minimal duplication across files.

I added a few test cases (copy-pasted and edited from literalinclude)

@xenova
Copy link
Contributor Author

xenova commented Apr 29, 2023

Not too sure what the "canonical test" is used for (defined in literalinclude), but I included it here too :) (perhaps for reference?)

@coyotte508 coyotte508 requested review from mishig25 and sgugger April 30, 2023 15:24
Copy link
Contributor

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

LGTM, but would like for @mishig25 to also have a look since he is the one who wrote that part of the code :-)

@xenova
Copy link
Contributor Author

xenova commented May 1, 2023

Sounds good 👍

I did run the tests on my side, and it worked correctly there. Perhaps I am using an outdated version of black or something :) Will check.

Edit: Yep, wasn't using the correct version. Getting the same result now. Will fix.

xenova added 2 commits May 1, 2023 15:12
Was including the last newline (when this should be removed with .`rstrip()`)
Copy link
Contributor

@mishig25 mishig25 left a comment

Choose a reason for hiding this comment

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

amazing work !

@mishig25 mishig25 merged commit fcf4656 into huggingface:main May 1, 2023
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