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

Image customization: add include() function #3148

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

neocturne
Copy link
Member

Add a simple way to include image customization Lua snippets.
Can be used to split long files, to include the same options multiple
times in different conditional branches, or even to pass values back to
the including file using return.

Relative paths are interpreted relative to the site root (where
image-customization.lua is located). Relative includes would become
confusing when subdirectories are involved (as they are still interpreted
relative to the site root, and proper tracking of current directory for
each include seems fairly complex for a very niche use case), so we simply
reject this case for now; this way, we can implement this however we
want in the future if deemed necessary, without a breaking change.

Suggested by @T-X

$(error ...) already adds a `.` to the end of error messages.
@neocturne neocturne requested a review from blocktrron January 5, 2024 11:12
Error messages generated by Lua usually start with a lowercase letter.
Let's make our own messages match.
Add a simple way to include image customization Lua snippets.
Can be used to split long files, to include the same options multiple
times in different conditional branches, or even to pass values back to
the including file using return.

Relative paths are interpreted relative to the site root (where
image-customization.lua is located). Relative includes would become
confusing when subdirectories are involved (as they are still interpreted
relative to the site root, and proper tracking of current directory for
each include seems fairly complex for a very niche use case), so we simply
reject this case for now; this way, we can implement this however we
want in the future if deemed necessary, without a breaking change.
@neocturne neocturne force-pushed the image-customization-include branch from 57dc935 to fa09986 Compare January 5, 2024 11:14
@github-actions github-actions bot added 3. topic: docs Topic: Documentation 3. topic: build This is about the build system labels Jan 5, 2024
@blocktrron
Copy link
Member

@neocturne Would you mind implementing a sample implementation for the example-site? From the documentation one could assume it works similar to sourcing a shell-script. I think an example is the best way of demonstrating a best-practice use.

The implementation itself looks good 👍

@neocturne
Copy link
Member Author

Makes sense, I wanted to add more examples to the image customization docs anyways.

@blocktrron
Copy link
Member

Shall we wait for these to arrive? I think the PR itself is fine as-is. I'd just wait for a backport to v2023.2 for docs.

@neocturne
Copy link
Member Author

neocturne commented Jan 7, 2024

I think there is no hurry regarding this PR, and I can work on the doc examples in the next days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. status: waiting-on-author Waiting on some action from the author 3. topic: build This is about the build system 3. topic: docs Topic: Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants