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

Escaped Markdown links with spaces in them are not routed correctly #8867

Closed
5 of 7 tasks
VladNastase opened this issue Apr 4, 2023 · 3 comments · Fixed by #8874
Closed
5 of 7 tasks

Escaped Markdown links with spaces in them are not routed correctly #8867

VladNastase opened this issue Apr 4, 2023 · 3 comments · Fixed by #8874
Labels
bug An error in the Docusaurus core causing instability or issues with its execution good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin.

Comments

@VladNastase
Copy link

Have you read the Contributing Guidelines on issues?

Prerequisites

  • I'm using the latest version of Docusaurus.
  • I have tried the npm run clear or yarn clear command.
  • I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
  • I have tried creating a repro with https://new.docusaurus.io.
  • I have read the console error message carefully (if applicable).

Description

Links that contain spaces have to be enclosed in pointy brackets (<>) according to the CommonMark specification (see here). In Docusaurus, the links are displayed properly, but are routed to the original filename (i.e. they also keep the original file extension). A link such as [Link](<./file 1.md>) will go to https://<yourwebsitehere>/<yourpathhere>/file%201.md if onBrokenLinks is set to warn and will completely fail the build process otherwise.

Reproducible demo

https://vladnastase.github.io/docusaurus-bug/chapter1/doc

Steps to reproduce

  1. Create a file with a space in its name.
  2. Link to it using a Markdown link (do not forget to properly enclose it in pointy brackets).
  3. (Optional) Set onBrokenLinks to warn in docusaurus.config.js.
  4. Build the website and try the link, or observe the error.

Expected behavior

Build succeeds if onBrokenLinks is not set to warn, link works properly otherwise.

In the linked reproduction, links 2 and 3 should go to https://vladnastase.github.io/docusaurus-bug/chapter1/doc%202 and https://vladnastase.github.io/docusaurus-bug/chapter%202/doc1 respectively (instead of https://vladnastase.github.io/docusaurus-bug/chapter1/doc%202.md and https://vladnastase.github.io/docusaurus-bug/chapter%202/doc1.md)

Actual behavior

Build fails if onBrokenLinks is not set to warn, links don't work properly (they keep the original file extension).

Your environment

Self-service

  • I'd be willing to fix this bug myself.
@VladNastase VladNastase added bug An error in the Docusaurus core causing instability or issues with its execution status: needs triage This issue has not been triaged by maintainers labels Apr 4, 2023
@slorber slorber removed the status: needs triage This issue has not been triaged by maintainers label Apr 6, 2023
@slorber
Copy link
Collaborator

slorber commented Apr 6, 2023

Thanks, didn't know about this markdown link escaping rule in CommonMark.

Runnable repro: https://stackblitz.com/~/github.com/VladNastase/docusaurus-bug


Note repro shows:

# Bug example

[This](./doc3.md) link works.

[This](<./doc 2.md>) doesn't work.

[This](<../chapter 2/doc1.md>) also doesn't work.

CleanShot 2023-04-06 at 19 28 00@2x

To complete your repro:

[This](/doc 2.md) does not work

[This](./doc%202.md) works

I think we can consider this a valid bug, even though the linking to .md extensions is something proprietary, we should be able to use spaces in such links.

Using %20 instead of space is a good temporary workaround

@slorber slorber added the good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin. label Apr 6, 2023
@Himmu5
Copy link

Himmu5 commented Apr 9, 2023

Subject: Request to work on Issue #8867
Hi there,

My name is Himanshu Chauhan.

I noticed that there's an open issue [#8867] that I think I can help with. It's about to solve this bug. I'm interested in working on this issue and contributing to the project.

Before I start working on it, I wanted to check with you to see if it's okay for me to work on this issue. If it is, could you provide me with any guidance on how to proceed with the issue? I'm eager to learn more about the project and contribute to it in any way that I can.

Thank you for your time and consideration.

Best regards,
Himanshu Chauhan

@morsko1
Copy link
Contributor

morsko1 commented Apr 11, 2023

The problem is that existing regex for Markdown links doesn't match filenames that contain whitespaces and wrapped with <>.
I have created PR #8874 with modified regex.
I tested these changes locally with files and folders that contain spaces and looks like it fixes the issue.
Here is the link to updated regex where you can check how it matches filenames with or without spaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An error in the Docusaurus core causing instability or issues with its execution good first issue If you are just getting started with Docusaurus, this issue should be a good place to begin.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants