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

WIP: support regions in @includeCode #2816

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shawninder
Copy link

@shawninder shawninder commented Dec 22, 2024

WORK IN PROGRESS / DO NOT MERGE

This is a proof of concept for supporting regions in @includeCode tags, meaning you could insert part of a file as a code block by referencing a region of that file.

Example

sourceFile.ts

console.log("Code we don't want inserted")
// #region regionName
console.log('Code we want to insert')
// #endregion regionName
console.log("More code we don't want inserted")

documents/some-page.md

Here is an example:
{@includeCode src/sourceFile.ts#regionName}
This is useful for reasons.

output:


Here is an example:

console.log('Code we want to insert')

This is useful for reasons.


Notes

@Gerrit0 if this basic strategy seems OK, I would add the following to this PR before merging:

TO DO

  • Get a simple example working with Typescript
  • Use the appropriate region syntax for different languages as documented in the VS Code documentation for Folding
  • Make sure small variants don't break things (like using //#region NAME instead of // #region NAME)
  • Support regions for Markdown files in the @include tag as well
  • Produce useful (translated) errors where appropriate:
    • when a region is specified but cannot be found
    • when a region is found but empty (perhaps just a warning for this one?)
    • when a region is found more than once in a file
  • Add support for line-number syntax
  • Add tests to validate success, error and warning paths related to these new features
  • Document the above additions in site/tags/include.md
  • Use these new features in the example app (/example)
  • Add an entry in the Changelog describing the changes brought about by this PR

Thoughts? Questions? Advice?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Dec 23, 2024

Yes, this seems reasonable. Might want to split the logic out into its own function at this point.

Add support for line-number syntax

Is there any case in which this would be a better option than using a region? It seems like an objectively worse UX to me...

@shawninder
Copy link
Author

Might want to split the logic out into its own function at this point.

I'll probably be extracting a function to its own file for this, indeed.

Is there any case in which [line-number syntax] would be a better option than using a region? It seems like an objectively worse UX to me...

Agreed. I'm just thinking of the case where you want to include code from a file you don't control or in which you can't add comments to define regions (think JSON, for example, which does't support comments). In this case the line-numbers seem like the only text-based approach (as opposed to using a language-aware AST) that could work…

Can I let you decide if it's worth including?

@Gerrit0
Copy link
Collaborator

Gerrit0 commented Dec 23, 2024

Ah, that's a very fair point, line numbers is probably worth it at some point

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.

Support regions and line numbers within @includeCode tag reference
2 participants