Image customization: add include() function #3148
Open
+142
−14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
neocturne
force-pushed
the
image-customization-include
branch
from
57dc935 to
fa09986
Compare
github-actions
bot
added
3. topic: docs
|
@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 👍 |
|
Makes sense, I wanted to add more examples to the image customization docs anyways. |
|
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. |
|
I think there is no hurry regarding this PR, and I can work on the doc examples in the next days. |
blocktrron
added
the
2. status: waiting-on-author
neocturne
force-pushed
the
image-customization-include
branch
from
fa09986 to
7de7e58
Compare
$(error ...) already adds a `.` to the end of error messages.
Make the example match our recommended coding style.
Add usage examples, and simply the wording in a few cases.
Also make our messages match standard errors generated by Lua by starting with a lowercase letter.
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
force-pushed
the
image-customization-include
branch
from
7de7e58 to
1e1f269
Compare
|
I have rebased and extended the PR with various examples, as well as some additional error checks. |
neocturne
added
2. status: waiting-on-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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