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

Warn user when empty files are skipped #221

Merged
merged 3 commits into from
Jul 27, 2022

Conversation

bryophyta
Copy link
Contributor

What?

Logs a warning to tell the user when a file is being skipped from the build because it's empty.

Why?

I was looking around the code for this bit of functionality and noticed that there was an open issue to add some logging. (Closes #180)

The issue was opened a while ago, so I don't know if it's still something you want to add. But because it was a small change I thought I might as well just submit a PR. Feel free to reject it if the issue isn't relevant anymore!

Testing

This PR shouldn't affect functionality, and it doesn't seem like logging messages are something that should have unit tests attached to it. But if it's desirable to add a test here then I'm happy to do so.

On this topic, I was wondering if you have any tips on how to run a locally modified version of lume? I had a quick attempt but I was hitting some issues; I'm fairly new to Deno so I imagine that I'm missing something about how dependencies are resolved..

@oscarotero
Copy link
Member

Hey, thanks for this!

It looks good to me. Just one thing: there's another place to place this warning. Just before executing the preprocessors:
https://github.com/lumeland/lume/blob/master/core/site.ts#L520-L523

I was wondering if you have any tips on how to run a locally modified version of lume?

You can install the local cloned repository by executing deno task install, a task defined here: https://github.com/lumeland/lume/blob/master/deno.json#L5
Once installed, the lume command points to your local repository.

@bryophyta
Copy link
Contributor Author

Thanks for the tips! For some reason I'm having trouble with installing lume (I usually just run ci.ts using Deno). The install works, but then my shell can't find lume in the path. However, it seems quite likely that this is due to some config issue on my end. When I get the chance I'll try to replicate this more carefully and write up an issue if it's still a problem.

I've updated the branch with a warning at the site.ts skip step, as you suggested. Hope it all looks alright. When I was testing it locally I spotted something that I think is a small bug in the url logic, but I'll open this as a separate issue.

@oscarotero
Copy link
Member

Very nice! Just one question before merging. The reason source file is empty doesn't look correct to me. I think output content is empty is more accurated, because the source file might don't be empty, but the generated output does. For example:

export default function () {
    return "";
}

@oscarotero
Copy link
Member

The install works, but then my shell can't find lume in the path

It happens in some environments, that the PATH is not updated automatically on install Deno. This script may help (from the Deno manual):

echo 'export PATH="$HOME/.deno/bin:$PATH"' >> ~/.bashrc

@oscarotero oscarotero merged commit 04ffa86 into lumeland:master Jul 27, 2022
@oscarotero
Copy link
Member

Thank you!

@bryophyta
Copy link
Contributor Author

The reason source file is empty doesn't look correct to me. I think output content is empty is more accurated, because the source file might don't be empty, but the generated output does.

Sorry for the late reply! This makes a lot of sense to me, I've opened #224 to fix it.


It happens in some environments, that the PATH is not updated automatically on install Deno.

Thanks for this! I'll look into that 👍

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.

Notify if a page is empty and wont be exported
2 participants