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

Fix browsing file names that contain percent-encoded @ symbol #2200

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jessevanherk
Copy link

@jessevanherk jessevanherk commented Apr 22, 2024

This fixes browsing the Godot docs for @GlobalScope and @GDScript, by adding a short encode method to handle the @ symbol.

The bug was mentioned in #1853 , where the author was unable to determine why they couldn't browse these files.

This PR adds the encoding on the frontend, rather than trying to override filename generation in scrapers. It's possible that this will impact other documentation sources, but I expect those would also need to have encoded the @ as %40 in their own doc filenames. This PR only impacts filename matching, not entries (which already use slugs).

If reviewers knows of other languages that use @ in a similar way, I'd be happy to test against them!

This doesn't use Javascript's encodeURI or encodeURIComponent methods, because encodeURI doesn't replace @, and encodeURIComponent replaces / which we don't want. If there are other URL-reserved characters
that would be useful to encode, those should be easy enough to add to the short method I added here.

QA

Testing done in local instance of devdocs.

  • tested against some selected packaged docs (making sure I didn't break anything):
    • godot 2.1
    • godot 3.5
    • ansible
    • HTML
    • elixir 1.15
  • tested against locally generated godot docs:
    • godot 3.5
    • godot 4.2 (WIP, will be separate PR)

This adds an encode method into the javascript code so that
users can browser the Godot docs for @globalscope and @GDscript.

The bug was mentioned in:
freeCodeCamp#1853

This adds the encoding on the frontend, rather than trying to override
filename generation in scrapers. It's possible that this will impact
other documentation sources, but I expect those would also need to have
encoded the @ as %40 in their own docs.

This doesn't use JS' encodeURI or encodeURIComponent because encodeURI
doesn't include @, and encodeURIComponent includes / which we don't
want. If there are other URL-reserved characters
that would be useful to encode, those should be easy enough to add to
the short method added here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant