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

Entries with complex extensions do not appear in collection view #877

Closed
ivanreese opened this issue Dec 6, 2017 · 17 comments Β· Fixed by #1123
Closed

Entries with complex extensions do not appear in collection view #877

ivanreese opened this issue Dec 6, 2017 · 17 comments Β· Fixed by #1123

Comments

@ivanreese
Copy link

- Do you want to request a feature or report a bug?

bug hunt πŸ‘Ύ

- What is the current behavior?

Entries that have a complex extensionΒ β€” eg: html.md β€”Β can be created in the CMS, but do not appear in the collection view.

- If the current behavior is a bug, please provide the steps to reproduce.

Set extension: "html.md" and format: frontmatter. In the CMS, create a new entry in that collection. Entry is committed successfully, with the custom extension. Entry does not appear in the collection view.

- What is the expected behavior?

Entry should appear in the collection view.

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

Netlify CMS version 0.7.6, deployed to Netlify

@tech4him1
Copy link
Contributor

tech4him1 commented Dec 6, 2017

This is caused by our extension filtering function. When it gets the extensions from the file, it only gets the last extension, assuming everything before that is part of the filename. https://github.com/netlify/netlify-cms/blob/9c7c0aeed22cce1fbe01db48e239da49ba3b42ec/src/backends/github/implementation.js#L56 https://github.com/netlify/netlify-cms/blob/834c5f4942ee9f694a900a834c6f672d31761922/src/lib/pathHelper.js#L83-L91

@ivanreese
Copy link
Author

Just to lend a bit of context...

These multi-part extensions are a requirement of Middleman as of v4. Here's the issue where they discuss the motivation for this change: middleman/middleman#1211

@erquhart
Copy link
Contributor

erquhart commented Dec 7, 2017

We should support this.

@truckyforme
Copy link

Same issue here. I'm deploying a multilingual host with HUGO which requires a complex extension. It'd be great to allow this.

@kornichon2000
Copy link

Yes, Hugo multilingual requires complex extensions, and it would be great if Netlify CMSΒ could be fixed to support them.

For example, currently example.fr.md is changed toΒ example-fr.mdΒ - Hugo requires that the extension remains .fr.md

@erquhart
Copy link
Contributor

erquhart commented Jan 2, 2018

We'll take a PR if anyone is up for digging in.

@erquhart erquhart added the hot label Jan 2, 2018
@kornichon2000
Copy link

Hi, I'm afraid my JS skills are fairly limited. However I see two solutions:
a) I think the best solution is to leave the . alone. It's the safest solution as unfortunately it's impossible to determine reliably what is a . for a complex extension or just a . in the middle of the filename.
b) in the function posted by tech4him1, use indexOf instead of lastIndexOf to split the file name into name - extension at the first . occurrence instead of the last one. With the possible regression that people using . in their filenames will confuse the system.

@ivanreese
Copy link
Author

Is dots in the filename something that people actually do? I'm not sure that that's a common enough practice to be something worth supporting.

@truckyforme
Copy link

I'm not an expert and can be easily wrong in my view.
Probably few people, if any, use dots in filenames. However, that shouldn't be a concern. What Kornichon2000 proposes in his first option is a simple solution to the issue of complex extensions and the potential confusion to users who decide to use dots in their filenames.

@tech4him1
Copy link
Contributor

tech4him1 commented Jan 5, 2018

I think a simple fix would be to simple check if the file ends with the extensions specified:

files.filter(file => file.name.endsWith('.' + extension))

We would want to check it with a period prepended to the extension, so that we wouldn't accidently match part of the file name (i_like_tar.gz should not be matched by tar.gz, but i_like.tar.gz should).

@tech4him1
Copy link
Contributor

@kornichon2000 I agree that your last solution is probably not the best option -- we don't really want to introduce regressions here. How do you see implementing your option a?

@kornichon2000
Copy link

@tech4him1 Somewhere in the code there is a function that parses the name string to replace invalid url characters with dashes. However the dot . is a valid url character. I think the solution is to modify the parsing routine so it leaves dots unchanged in the filename. I can't think of a negative side effect.

@erquhart
Copy link
Contributor

erquhart commented Jan 5, 2018

@kornichon2000 there were issues with static site generators or deployments or something that made us rule out periods. Plus slugs traditionally have periods stripped, I'd consider that widely expected behavior.

@tech4him1 endsWith is exactly what I was thinking, agreed.

@tech4him1
Copy link
Contributor

tech4him1 commented Jan 8, 2018

@erquhart @talves Looking at the fileExtension code:
https://github.com/netlify/netlify-cms/blob/834c5f4942ee9f694a900a834c6f672d31761922/src/lib/pathHelper.js#L66-L80
Do you think any of that "special" logic needs re-created, or do you think just checking endsWith is safe? I don't want to cause regressions, but most of this code seems irrelevant since we're already filtering out folders.

@erquhart
Copy link
Contributor

@tech4him1 since we're only using this check in one place and not for general extension checking, just using endsWith should be fine.

@tech4him1
Copy link
Contributor

tech4him1 commented Jan 20, 2018

@erquhart Hmm, yeah.

Probably not a bad idea to run a sanitize function on it as well, in case a user tries to do something weird like .md/my-file.md. Unless we want that...?
EDIT: For right now I don't think we need to run a sanitize function on the user-supplied extension, since we're not for writes:
https://github.com/netlify/netlify-cms/blob/cfbf31b130172d0f2c44fcfa0bcc4d96bd84ef5f/src/reducers/collections.js#L52
We're also assuming a dev is supplying this extension, so we for the most part want to allow "special" configuration like that, vs a untrusted user setting.

@erquhart
Copy link
Contributor

Yep, I'm good with that.

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

Successfully merging a pull request may close this issue.

5 participants