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

Slugification for copied files #447

Closed
svlauer opened this issue Jul 9, 2023 · 5 comments
Closed

Slugification for copied files #447

svlauer opened this issue Jul 9, 2023 · 5 comments
Labels
enhancement New feature or request
Milestone

Comments

@svlauer
Copy link

svlauer commented Jul 9, 2023

Enter your suggestions in details:

Thank you for creating lume, I love it so far!

One thing that would be great would be a way to slugify also the names of copy()ed files. Because right now, the interaction of copying and slugification seems kind of bad.

Unless I am missing something (which might well be!), right now, the behavior is the following (assuming the slugify plugin is loaded):

  • Suppose in my .src directory contains a path with spaces and Capitals/, which contains:
    • a static file with spaces and Capitals/more spaces and Capitals.bin (with site.copy(['.bin'])
    • a processed file with spaces and Capitals/With Spaces.md
  • I'll be getting two directories in _site:
    • with spaces and Capitals, which contains more spaces and Capitals.bin
    • with-spaces-and-capitals, which contains with-spaces.html
  • Ideally, I would like to have:
    • One directory, with-spaces-and-capitals, which contains more-spaces-and-capitals.bin and with-spaces.html
  • I know I can fiddle with the url of the copy()ed files via the second argument to copy(), but such a solution would be cumbersome and somewhat brittle (e.g., suppose the slugify options change).

I guess one of the reasons for the current behavior is that copy()ed files are not processed at all, hence the slugify plugin has no chance to intervene.

Maybe it would be an option to provide (in the slugify plugin?) a copyAndSlugify() method?

Or is my "ideally" above already possible, I just missed how?

If not, I'd be willing to work on a pull request for this.

@svlauer svlauer added the enhancement New feature or request label Jul 9, 2023
@oscarotero
Copy link
Member

Hi.
Yes, it makes sense. Right now, the plugin only affects to loaded pages, but not filed copied statically. Maybe the plugin should have an option to enable it. I imagine something like:

site.use(slugifyUrls({
  staticFiles: true,
});

For now, as a workaround, you can use the beforeRender event:

site.addEventListener("beforeRender", () => {
  const files = site.files; // the files to be copied statically

  // Change the outputPath property
  files.forEach((file) => {
    file.outputPath = slugify(file.outputPath);
  });
});

@svlauer
Copy link
Author

svlauer commented Jul 10, 2023

Thanks for the quick reply!

site.use(slugifyUrls({
  staticFiles: true,
});

Seems sensible - in principle, I think it would even make sense to have this option activated by default ... but that would be a breaking change.

Thank you for the workaround, that makes things easier for me in the meantime!

@oscarotero oscarotero added this to the 2.0.0 milestone Jul 11, 2023
oscarotero added a commit that referenced this issue Jul 19, 2023
@oscarotero oscarotero removed this from the 2.0.0 milestone Jul 19, 2023
@oscarotero
Copy link
Member

I just made some changes in Lume to slugify static files.
The extensions option now is applied to both loaded files and copied files. By default is .html so I don't think it's a breaking change.

If you want to slugify some files copied statically, for example .pdf files, you can do:

site.use(slugifyUrls({
  extensions: [".html", ".pdf"]
});

To slugify all files, use *:

site.use(slugifyUrls({
  extensions: "*"
});

This change will be available in Lume v1.18.2. It's not released yet, but you can test it by upgrading Lume to the latest development version with deno task lume upgrade --dev.

@svlauer
Copy link
Author

svlauer commented Jul 22, 2023

Works like a charm!

Thank you for making this happen so quickly!

A suggestion/question: Wouldn't it make sense to change the default to extensions: "*" for lume 2? Like, if someone wants slugification, they probably want it for all/most things?
(I might be missing something that makes this a bad idea, and anyways, having an extra line for this in _config.ts is not a big deal, just wondering.)

@oscarotero
Copy link
Member

Yes, good idea. I'll change it to * in v2.
Thanks!

@oscarotero oscarotero added this to the 2.0.0 milestone Jul 22, 2023
oscarotero added a commit that referenced this issue Jul 24, 2023
oscarotero added a commit that referenced this issue Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants