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

Filename sanitation should only apply to variable parts #1254

Closed
Quicksaver opened this issue Apr 12, 2018 · 7 comments · Fixed by #2897
Closed

Filename sanitation should only apply to variable parts #1254

Quicksaver opened this issue Apr 12, 2018 · 7 comments · Fixed by #2897

Comments

@Quicksaver
Copy link
Contributor

- Do you want to request a feature or report a bug?
I consider it a bug, although I can see how this can be seen as a feature request.

- What is the current behavior?
Filename sanitation, in particular replacing periods with hifens and lowercasing, is applied to the whole slug. It should only be applied to the variable parts, such as {{slug}} or {{name-of-field}}.

There is clearly a need for this (#877). For me this behavior completely breaks any custom i18n functionality as hugo just doesn't like those hifens. And while one could always make the case "maybe the site generators should learn to read files with hifens correctly" (with which I also agree), I don't see how allowing a higher control to devs over the generated filenames could be harmful.

- If the current behavior is a bug, please provide the steps to reproduce.
For example, set a collection slug to {{title}}.{{i18nlanguage}}, where "i18nlanguage" is a special field that contains a locale string like "EN" or "PT", the resulting filename will be something like title-of-my-article-en.md.

- What is the expected behavior?
If I explicitly add a period to the slug, I absolutely expect it to appear in the resulting filename, regardless of any encoding or sanitation preferences: title-of-my-article.en.md

Basically I would expect any value I hardcode in those slugs to take precedence over anything else. If that causes any trouble (with builds, deploys, whatever) it's my job to fix it, I don't expect Netlify CMS to do it for me. 😉

- Please mention your CMS, node.js, and operating system version.

- Please link or paste your config.yml below if applicable.

@Quicksaver
Copy link
Contributor Author

cc @tech4him1 you've worked on slug sanitation recently (#640, it's kinda recent...), care to weigh in?

@tech4him1
Copy link
Contributor

tech4him1 commented Apr 12, 2018

@Quicksaver Sure, I'd be glad to:

First off, for the specific use case you mention (multiple extensions), the current way to do that is to use the extension setting on the collection (e.g. extension: "en.md"), and like you mentioned, that was added in #877. It doesn't accept placeholders however, so you have to create a seperate collection for each language (#716).

For the whole idea of only sanitizing placeholders only, it is extremely relevant to the discussion in #513 (comment). I think what you are proposing is a very good idea, but I'm not quite sure it fits with the current idea of how the slug field works, and it may be better to add it in a different way.

@erquhart Any thoughts?

@Quicksaver
Copy link
Contributor Author

First off, for the specific use case you mention (multiple extensions), the current way to do that is to use the extension setting on the collection

I did see that but somehow I completely missed the most obvious usage for it... That'll teach me to pay attention. Thanks for the pointer! 😄

I think what you are proposing is a very good idea, but I'm not quite sure it fits with the current idea of how the slug field works, and it may be better to add it in a different way.

@tech4him1 hmm, I'm not sure I entirely agree on that one, or at least maybe I'm missing some context? From the docs:

For folder collections where users can create new items, the slug option specifies a template for generating new filenames based on a file’s creation date and title field.

Sanitizing the placeholders I totally agree with, it's harder to foresee and control their output to ensure file paths are valid. I see sanitizing the actual "template" as altering it though, maybe it's a personal opinion, but I don't expect my templates to be overwritten in any way, to me that kind of defeats the purpose of having "templates" in the first place. Maybe we have different ideas of "how the slug field works"?

@erquhart
Copy link
Contributor

@Quicksaver good points. I hadn't considered it that way at all, but it definitely makes sense to leave filename templates as is. That said, it's breaking to just change the behavior, and even at the v2 release I'd be worried about fallout. Consider a large site that's been using Netlify CMS all along and leaning on this behavior suddenly having all of it's filename hyphens become spaces or dots or whatever was in their template.

Perhaps we could add an option (man this really is turning into wordpress 😛) like transformTemplate under slug, and treat the current behavior as default true.

@Quicksaver
Copy link
Contributor Author

@erquhart I agree breakage could definitely happen, although I would say as far as breaking changes go, I see this as pretty mild, since any periods coming placeholders would still be turned to hifens, and rewriting the hardcoded periods in the slug config to hifens should be very straightforward.

Not really a fan of "option flags" like transformTemplate to be honest, wouldn't oppose it either though of course. 😛

This makes me like the path proposal from #513 a bit more actually. Maybe advancing with more complex/complete logic that's being discussed there within path, including my suggestion here of keeping any periods or slashes or whatever hardcoded but in path instead of in slug, and leaving slug as-is as a legacy option if path isn't set. Maybe that's what you were considering from the beginning? That would avoid any breakage and still allow for better "template" behavior for filenames that would need it.

@erquhart
Copy link
Contributor

Yeah, bringing this in a new API a la "path" would be great. The breaking change is certainly mild, but the potential for confusion over weird things like duplicate entries with slightly different filenames is what I'm concerned about, in the case where someone doesn't execute their migration flawlessly.

@stale
Copy link

stale bot commented Oct 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants