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

escape tag for url special character #8

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

kevchentw
Copy link

Issue

The generate process failed when tag cantains special character (e.g. "/")

Reproduce

  1. set a tag with "/" such as "CI/CD" on notion
  2. run notablog generate .
  3. Error as below
E/notablog-cli(20200505T234305.045): Error: ENOENT: no such file or directory, open 'public/tag/CI/CD.html'
    at Object.openSync (fs.js:457:3)
    at Object.writeFileSync (fs.js:1282:35)
    at /home/kevink_chen/.nvm/versions/node/v12.16.1/lib/node_modules/notablog/src/render-index.js:34:8
    at Map.forEach (<anonymous>)
    at renderIndex (/home/kevink_chen/.nvm/versions/node/v12.16.1/lib/node_modules/notablog/src/render-index.js:27:19)
    at generate (/home/kevink_chen/.nvm/versions/node/v12.16.1/lib/node_modules/notablog/src/generate.js:90:3)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async cmdGenerate (/home/kevink_chen/.nvm/versions/node/v12.16.1/lib/node_modules/notablog/bin/cli.js:28:5) {
  errno: -2,
  syscall: 'open',
  code: 'ENOENT',
  path: 'public/tag/CI/CD.html'
}

Solution

replace special character with "-" by using squirrelly filter

reference:

dragonman225 and others added 15 commits March 10, 2020 12:27
* @rollup/plugin-typescript v4.0.0 is broken, so fallback to v2.1.0
Since the project is now written in TypeScript, the sources have to be
compiled before using the CLI.
Manually running the build script isn't fun, so I add this NPM script
that executes `rollup -cw` to automatically rebuild the bundle
whenever sources are modified.
NAST.Collection is a plain JS object, but it is not easy to work with,
NTable solve the problem by creating a class to represent the collection
and provide lots of pointers that links related data for easier access.
Cache module owns cache/ in a Notablog starter. It handles cache r/w
and shouldUpdate() query.
Extract duplicate parts of NXxxProperty, NXxxCell to NProperty and NCell.
Refer to NDateTimeProperty and NDateTimeCell.

By the way there is a new function arrayAccessMayFail() to help access
data in a multi-layered array.
Fix some confusing sentences.
@dragonman225
Copy link
Owner

Hi, this regex [&\/\\#, +()$~%.'":*?<>{}] seems to effect a lot of characters, is it really necessary?
And does it means that CI-CD instead of CI/CD will be displayed in the tag, not just the URL and the filename being changed?
Personally, I suggest using encodeURIComponent() for the URL and the filename, and HTML entities for the displayed text.

@kevchentw
Copy link
Author

The tag will display as "CI/CD" since I only escape href only.
<a href="tag/{{@this.value|escapeTag}}.html">{{@this.value}}</a>

The reason for replacing with "-" is for the url readability
e.g.
/tag/CI-CD.html -> replace
/tag/CI%252FCD.html -> encodeURIComponent (encode two times)

For the regex, we can preserve necessary characters such as "%", "/", "?", "&", "#". But I need to check out which character will effect the url links.

@dragonman225
Copy link
Owner

The tag will display as "CI/CD" since I only escape href only.
<a href="tag/{{@this.value|escapeTag}}.html">{{@this.value}}</a>

This sounds great! At first, I was worried that changing the displayed text might confuse the user.

The reason for replacing with "-" is for the url readability
e.g.
/tag/CI-CD.html -> replace
/tag/CI%252FCD.html -> encodeURIComponent (encode two times)

For the regex, we can preserve necessary characters such as "%", "/", "?", "&", "#". But I need to check out which character will effect the url links.

Agreed. Readability is a good reason to adopt character replacing instead of encodeURIComponent.

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.

2 participants