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

Fix included files #36

Merged
merged 4 commits into from
Sep 16, 2023
Merged

Fix included files #36

merged 4 commits into from
Sep 16, 2023

Conversation

jpenilla
Copy link
Member

@jpenilla jpenilla commented Sep 13, 2023

Fixes included files and adds a test with an imported macro.

The MultiDirectoryLoader treats the source/included directories as if they were merged for resolving macros.

Questions:

  • Do we want to have an error when there are multiple files with the same relative path?
  • Is this todo in GenerateWorkerInvokerImpl resolved by these changes
    // By default, resolves FS paths
    // todo: restrict inputs to inputs and includes
  • Do we want to automatically search for ${templateName}.peb if templateName can't be found in a directory? The Pebble docs show imports being used without a .peb extension.

@jpenilla jpenilla linked an issue Sep 14, 2023 that may be closed by this pull request
@jpenilla jpenilla changed the title Add disabled failing test for importing macros from included files Fix included files Sep 14, 2023
@zml2008
Copy link
Member

zml2008 commented Sep 15, 2023

looks nice, thanks! the .peb thing looks like something we should do if you've got some time

@jpenilla
Copy link
Member Author

Updated to make the .peb extension in imports optional

@zml2008 zml2008 self-assigned this Sep 16, 2023
@zml2008 zml2008 added this to the 2.1.0 milestone Sep 16, 2023
@zml2008 zml2008 added this pull request to the merge queue Sep 16, 2023
Merged via the queue into KyoriPowered:main/2.x with commit bfd1033 Sep 16, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test coverage: includes
2 participants