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: compiler: Add a ListResources option for resource embedding #6525

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

Conversation

hunger
Copy link
Member

@hunger hunger commented Oct 11, 2024

... which will list all resources that are not going to get embedded as None in the Document's embedded_file_reosurces.

The idea is to use that field to find all used resources in the live preview so that we know what we can watch.

This is at this point a demo to show what using the Document::embedded_file_resources for resource listing in the live preview would entail for the compiler.

I'd suggest to rename the field itself to resources and to collect this information even when no embedding is requested (most users won't see it anyway) to not have this ListAllResources enum value...

@@ -90,7 +90,9 @@ fn embed_images_from_expression(
let mapped_path =
urls.get(path).unwrap_or(&Some(path.clone())).clone().unwrap_or(path.clone());
*path = mapped_path;
if embed_files != EmbedResourcesKind::Nothing
if embed_files == EmbedResourcesKind::ListAllResources {
global_embedded_resources.borrow_mut().insert(path.clone(), None);
Copy link
Member

Choose a reason for hiding this comment

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

I think that it might be a lot easier to leave global_embedded_resources as it is (don't insert the Option), but instead:

  1. Always call embed_image: let image_ref = embed_image(...);
  2. `if embed_files != ListAllResources { *resource_ref = image_ref; }

No change in data structures needed then.

Copy link
Member Author

Choose a reason for hiding this comment

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

True, that is a lot simpler.

On the other hand it reads all the images and stores them into the document's embedded_file_resources, processing them along the way. That seems a tad wasteful with regards to both time and memory.

Copy link
Member

Choose a reason for hiding this comment

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

No, they're not stored anywhere. They're also not read. That's left to the generators.

... which will list all resources that are not going to get embedded
as `None` in the Document's `embedded_file_reosurces`.

The idea is to use that field to find all used resources in the
live preview so that we know what we can watch.
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.

2 participants