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

Unescaping percent encoding on graph when title is not specified (or post does not exist) #366

Closed

Conversation

tomoyanonymous
Copy link
Contributor

This PR solves the issue mentioned at here jackyzha0/hugo-obsidian#27 .

The posts which has CJK filename and no title metadata on frontmatter, or the wikilink for non-existent word are shown with percent-encoding.

This PR just decodes URI when its displayed.

@jackyzha0
Copy link
Owner

This should be fixed in Quartz 4, please reopen if the issue persists. Quartz 3 will be deprecated shortly

@jackyzha0 jackyzha0 closed this Aug 20, 2023
@tomoyanonymous
Copy link
Contributor Author

Wow, I will take a look later! Thanks.

@tomoyanonymous
Copy link
Contributor Author

I tried v4 and the similar issue still happens. If the title contains CJK characters, backlink and outgoing links in the graph was not linked.

In the plugins/transformers/links.tsx,

const url = new URL(dest, `https://base.com/${curSlug}`)
const canonicalDest = url.pathname
const [destCanonical, _destAnchor] = splitAnchor(canonicalDest)
const simple = simplifySlug(destCanonical as FullSlug)
outgoing.add(simple)

When the URL object was made, URI encoding happens.

By changing like this, it worked correctly in my environment though I am not sure if it is really OK.

const url = new URL(dest, `https://base.com/${curSlug}`)
const canonicalDest = url.pathname
const [destCanonical, _destAnchor] = splitAnchor(canonicalDest)
- const simple = simplifySlug(destCanonical as FullSlug)
+ const simple = decodeURI(simplifySlug(destCanonical as FullSlug)) as SimpleSlug

Can I open another PR?

@jackyzha0
Copy link
Owner

ah, @tomoyanonymous yes feel free to open another PR for this

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

2 participants