docsifyjs / docsify

🃏 A magical documentation site generator.
https://docsify.js.org
MIT License
27.47k stars 5.67k forks source link

build/emoji.js fails in Windows #2290

Closed trusktr closed 3 months ago

trusktr commented 11 months ago

Bug Report

Steps to reproduce


D:\src\docsifyjs+docsify> node build/emoji.js 
Build emoji
- Fetching emoji data from https://api.github.com/emojis
- Retrieved 1877 emoji entries
- Error: Cannot read properties of null (reading '1')
- ```

#### Current behaviour

fail
#### Expected behaviour

pass

#### Other relevant information

- Docsify version: develop
- Your OS: Windows
- Node.js version: 20.5.0
- npm/yarn version: npm 10.2.0
- Browser version: N/A

#### Please create a reproducible sandbox

#### Mention the docsify version in which this bug was not present (if any)
trusktr commented 11 months ago

After making the change in

I can see the full error in the output, and the script no longer exits with 0 after the error (it should exit non-zero if there's an error):

PS D:\src\lume+lume\packages\docsifyjs+docsify> node .\build\emoji.js
Build emoji
- Fetching emoji data from https://api.github.com/emojis
- Retrieved 1877 emoji entries
file:///D:/src/lume+lume/packages/docsifyjs+docsify/build/emoji.js:47
  const emojiMarkdownStart = emojiMatch[1].trim();
                                       ^

TypeError: Cannot read properties of null (reading '1')
    at writeEmojiPage (file:///D:/src/lume+lume/packages/docsifyjs+docsify/build/emoji.js:47:40)
    at file:///D:/src/lume+lume/packages/docsifyjs+docsify/build/emoji.js:101:3
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Node.js v20.5.0
trusktr commented 11 months ago

I updated the PR so it also fixes the Windows issue. I'm not sure exactly how the issue happens, but I think it might be due to git settings.

I think that the other PR,

may prevent the CRLFs from popping in unexpectedly, but no harm in making the regex more robust just in case the CRLFs get in there somehow.

jhildenbiddle commented 4 months ago

@trusktr --

Can we close this now that #2288 and #2436 have been merged?