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

Space in Filenames in Content Folder #82

Open
lacikawiz opened this issue Oct 28, 2020 · 8 comments
Open

Space in Filenames in Content Folder #82

lacikawiz opened this issue Oct 28, 2020 · 8 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@lacikawiz
Copy link

Hi!

I just discovered your project and I'm very excited about it. It looks like you read my mind about a project that would use Go and Svelte to make static websites as single page apps. I'm a big fan of both Go and Svelte and use them every single day on my projects. So I'll probably get involved in this project, I see very great potential in it.

Anyways, here's my first contribution. I found a bug:

If there is any space in the file name of a content file, then the link to it is broken.

I used the plenti_0.2.39_Linux_64-bit.tar.gz downloaded from the https://plenti.co/ site.

Steps to recreate:

  • Generate new site: plenti new site XYZ
  • Change the name of the a blog post, for example:
cd XYZ/blog
mv perry.json "perry 1.json"
  • Run plenti serve
  • Go to http://localhost:3000
  • Click on the "Perry 1" link on the bottom
  • You will see a 404 page
  • If you refresh the page, the content loads in correctly

Expected behavior: Content loads in when clicking the link.

@jimafisk
Copy link
Member

Good catch @lacikawiz, so pumped you want to get involved!

Currently we're trying to create the slug from the filename here:

reSlugify := regexp.MustCompile("[^a-z0-9/]+")
. There's some extra stuff happening in there because we allow users to override the default routing using replacement patterns from their content source: #14

I'm finishing up some work on themes but can take a look once I'm done with that. I'm also happy to advise if it's something you want to take a stab at coding yourself! I appreciate your detailed bug report 🙌

@jimafisk jimafisk added the bug Something isn't working label Oct 28, 2020
@jimafisk jimafisk added the good first issue Good for newcomers label Nov 5, 2020
@jimafisk
Copy link
Member

Example:

  • content/new\ thing.json
  • layout/content/new\ thing.svelte

First error: Could not add SSR Component: SyntaxError: Unexpected identifier

Component signature in ssrStr has space in it:

/* generated by Svelte v3.29.4 */
/*import { create_ssr_component, escape } from "svelte/internal";*/

var layout_content_new thing_svelte = create_ssr_component(($$result, $$props, $$bindings, slots) => {
	let { name } = $$props;
	if ($$props.name === void 0 && $$bindings.name && name !== void 0) $$bindings.name(name);
	return `<h1>${escape(name)}</h1>`;
});

/*export default Component;*/

Second error: Could not create props: SyntaxError: Unexpected identifier

That's because the routeSignature is layout_content_new thing_svelte.

The filename in the currentContent.contentDetails object also has a space in it:

{ "pager": 1, "path": "/new-thing", "type": "new thing", "filename": "new thing.json", "fields": { "name": "new thang" } }

@jimafisk
Copy link
Member

I'm wondering if the best thing to do here would be to add a build step that checks for any spaces in files within your project, then actually rewrites them on the filesystem so they use hyphens instead. We'd have to rewrite them virtually anyways to make things consistent, so at least this way it's explicit and we could do the conversion one time right at the source instead of adding conversions all over the place for components signatures, layouts, content source, etc.

@BraydenGirard
Copy link

Hey @jimafisk,

Where would be the best spot to add a build step? I can look into this if it is not already underway.

@jimafisk
Copy link
Member

jimafisk commented Mar 3, 2021

Hi @BraydenGirard,

I had started looking into this (#122) but wasn't happy with the approach, so I just closed it. Basically we're allowing people to upload files with whatever naming convention they want and then we're slugifying them to make the filename work as a URL path. I was wondering if it would be better (maybe it's not) to actually rewrite the filename in the source of the project, not just the output. So for example, adding the following files to the /content folder would be rewritten as so:

  • New Event.json => new-event.json
  • my_2nd_post!.json => my-2nd-post.json

Then basically we just have to figure out when this rewrite would happen. The "build" is the central process of every Plenti site, so I figured it made sense to add another step early on in that process. All the different build steps are called from the build cmd. So from there we'd call out to a new function that we should add to the cmd/build folder called slugify.go or something, where we actually rewrite the filename on the filesystem.

If we implement this, it would improve consistency in naming convention, but I do worry that users might feel like we're taking control away from them. There also might be a very good reason why they want to name something in a particular way for organizational purposes and it could be frustrating if we rewrite that automatically. The examples I listed above are very strict, we could mitigate some of these issues by allowing the rewrites to be more flexible since things like underscores are valid in urls.

Would love to hear others thoughts on this approach, as I'm still on the fence whether it's right thing to do. It does feel a little weird to be rewriting "source" files when that's something usually under the user's control.

@jimafisk
Copy link
Member

jimafisk commented Mar 3, 2021

I just talked this through with @stephanieluz and we're thinking rewriting the files in the content folder might not be the best approach. Aside from the feeling of lost control, there are some technical challenges this approach could impose. For instance if you're pulling your content from an external data source (#35), you may have little control over the file naming convention. If you pull a bunch of records and then we rewrite them, it's now extremely difficult to sync those same files up on the next pull. They would get rewritten the same way on the second pull, so if you allowed the slugify rewrite to override existing files then it would technically work. That poses another problem of accidentally rewriting existing content source files if uploading a file that gets slugified in a way that matches an existing filename. These factors have me leaning towards thinking that rewriting the filenames in the content folder is probably not the best path forward. I might ultimately revive the work started on #122 instead. If you're interested in taking that on @BraydenGirard just let me know!

@BraydenGirard
Copy link

BraydenGirard commented Mar 3, 2021

Regarding file names coming from #35 , I think this would not be an issue if you used the key value I proposed. As far as I know every database stores at least one unique property on every entry.

@stephanieluz
Copy link
Collaborator

After speaking with @jimafisk about this a little bit, we think an easy fix would be presenting an error message stating that spaces are not permitted in file names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants