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

url problem when page title contains an accented letter #20

Open
francoisromain opened this issue Jun 19, 2020 · 6 comments
Open

url problem when page title contains an accented letter #20

francoisromain opened this issue Jun 19, 2020 · 6 comments
Assignees
Labels
enhancement New feature request status: in progress Work is in progress for the bug or feature

Comments

@francoisromain
Copy link

francoisromain commented Jun 19, 2020

Hello and thank you for this useful plugin.

There is a bug in the url when a page title contains an accented letter.

example:

{
          "title": "Développement",
          "pages": [
            {
              "title": "Introduction",
              "source": "./docs-sources/02-developpement/01-introduction.md"
            }
          ]
        }

the url of the page is /pages/Développement/01-introduction.html and the page is undefined.

It works well if I remove the accent.

I would suggest to the slugify title to use in the url.

@mipatterson
Copy link
Owner

@francoisromain thanks for reporting this!

I'm working on a fix now.

@mipatterson
Copy link
Owner

@francoisromain I'm actually having difficulty reproducing the issue locally. I also checked out your camino-api repo, where you mentioned this issue, but the pages in that repo with accents in the titles seem to be working fine there as well. Perhaps you could provide a small sample repo that showcases the issue?

@francoisromain
Copy link
Author

@mipatterson You are right I can't reproduce the bug. I don't know what happened.

I still think it would be good to slugify the title in the url

Exemple:

Thank you

@mipatterson mipatterson added enhancement New feature request status: consideration Feature request or bug fix is being considered and removed bug Something isn't working correctly labels Nov 25, 2020
@mipatterson
Copy link
Owner

@francoisromain

Seems like this feature might be particularly useful in SEO scenarios. What do you think of the following acceptance criteria for this feature?

  • Add a new slugifyUrls boolean option that controls whether or not the URLs are slugified
  • Default new option to true
  • Only slugify the URL of a group, page, or section if an explicit output option wasn't specified by the user.

I was considering using the slugify library.

@mipatterson mipatterson added rfc Request for comments bug Something isn't working correctly labels Nov 25, 2020
@francoisromain
Copy link
Author

francoisromain commented Nov 25, 2020

very cool!

Personnally, I will always want to slugify the urls, and I don't see the need for making it optional. Same thing for the output option: I probably won't use it. It's just me, and I could be missing something. :-)

Maybe slug or url would be more explicit than output.

In our current typescript project, we use this slugify lib: https://github.com/sindresorhus/slugify. I don't remember why we chose this one over the other.

Thank you very much!

@mipatterson
Copy link
Owner

I definitely think it needs to be optional. Otherwise the output property would effectively be useless and users would lose the ability to have total control over the page name, which seems like a step in the wrong direction.

Also, my original thought was that it made sense for the plugin to slugify URLs by default, but I'm not so sure that's the right path forward now. At this point it would be a breaking change. Existing uses might have links out in the wild directly linking to pages, and changing this behavior would break those links. For now, I think it's best that it be an opt-in feature. In the future, I would consider defaulting the new option to true, when there's another breaking change that is unavoidable.

The reason I had chosen output as the property name over url was that each output property was a portion of the URL. I was worried url would imply that the user had to define the entire URL in that property, which isn't the case. slug seems accurate, but didn't seem to be a term that was used widely enough. I was worried people might be confused.

I'll take a look at that slug library and see how it stacks up against the other one.

@mipatterson mipatterson added status: in progress Work is in progress for the bug or feature and removed status: consideration Feature request or bug fix is being considered rfc Request for comments bug Something isn't working correctly labels Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature request status: in progress Work is in progress for the bug or feature
Projects
None yet
Development

No branches or pull requests

2 participants