FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tools: fix json doc generate error caused by type loose_item_start by firedfox · Pull Request #5943 · nodejs/node · GitHub

/ node Public

tools: fix json doc generate error caused by type loose_item_start - #5943

Closed
firedfox wants to merge 1 commit into
nodejs:masterfrom
firedfox:fix-json-doc-generate-error-for-loose-item-start
ConversationCommits (1)ChecksFiles changed
Closed

tools: fix json doc generate error caused by type loose_item_start#5943
firedfox wants to merge 1 commit into
nodejs:masterfrom
firedfox:fix-json-doc-generate-error-for-loose-item-start

Conversation

firedfox commented Mar 29, 2016

Copy link
Copy Markdown
Contributor

Pull Request check-list

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

Affected core subsystem(s)

  • tools

Description of change

fixes #5942

tools: fix json doc generate error caused by type loose_item_start
551e000 
Current processList function in tools/doc/json.js does not recognise
{"type":"loose_item_start"}. Fix it.
mscdex added doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory. labels Mar 29, 2016

Fishrock123 commented Mar 29, 2016

Copy link
Copy Markdown
Contributor

cc @nodejs/documentation

silverwind commented Mar 29, 2016

Copy link
Copy Markdown
Contributor

LGTM.

This loose_item_start type looks to have been introduced recently, but I can't seem to find the exact commit, any idea?

firedfox commented Mar 30, 2016

Copy link
Copy Markdown
Contributor Author

It's because of such content that appeared recently in doc:

## fs.readdir(path[, options], callback)

* `path` {String | Buffer}
* `options` {String | Object}
  * `encoding` {String} default = `'utf8'`
* `callback` {Function}

* `path` {String}
* `callback` {Function}

Such format produces loose_item_start:

## Heading

* item0

* item1
[{"type":"list_start","ordered":false},{"type":"loose_item_start"},{"type":"text","text":"item0"},{"type":"list_item_end"},{"type":"loose_item_start"},{"type":"text","text":"item1"},{"type":"space"},{"type":"list_item_end"},{"type":"list_end"}]

eljefedelrodeodeljefe referenced this pull request Mar 30, 2016
fs: Buffer and encoding enhancements to fs API
060e5f0 
This makes several changes:

1. Allow path/filename to be passed in as a Buffer on fs methods
2. Add `options.encoding` to fs.readdir, fs.readdirSync, fs.readlink,
   fs.readlinkSync and fs.watch.
3. Documentation updates

For 1... it's now possible to do:

```js
fs.open(Buffer('/fs/foo/bar'), 'w+', (err, fd) => { });
```

For 2...
```js
fs.readdir('/fs/foo/bar', {encoding:'hex'}, (err,list) => { });

fs.readdir('/fs/foo/bar', {encoding:'buffer'}, (err, list) => { });
```

encoding can also be passed as a string

```js
fs.readdir('/fs/foo/bar', 'hex', (err,list) => { });
```

The default encoding is set to UTF8 so this addresses the
discrepency that existed previously between fs.readdir and
fs.watch handling filenames differently.

Fixes: #2088
Refs: #3519
PR-URL: #5616
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>

eljefedelrodeodeljefe commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

@silverwind I think this is some weird merging mistake during some work on fs. Seems like the older commit was merged first and then git was adding the older more incomplete version as addition. In any case the docs at the above line number are just wrong and would need to be fixed. See comment on @jasnell commit above.

@firedfox's commit seems legit anyhow, since it's just adding the case if it was intentional. LGTM

eljefedelrodeodeljefe mentioned this pull request Mar 30, 2016
Closed

firedfox commented Mar 30, 2016

Copy link
Copy Markdown
Contributor Author

Oh I thought those duplicated comments were added on purpose. Should I make another PR to remove them from doc?

eljefedelrodeodeljefe commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

I am not sure, but it seems very likely.
Sure go ahead. If you don't want to waste your time you could also wait until tomorrow, until the sun has passed West USA for the folks to comment.

firedfox mentioned this pull request Mar 30, 2016
Closed
4 tasks
silverwind pushed a commit that referenced this pull request Mar 30, 2016
tools: fix json doc generation
05b3a0b 
Current processList function in tools/doc/json.js does not recognise
{"type":"loose_item_start"}. Fix it.

PR-URL: #5943
Fixes: #5942
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Robert Lindstädt <robert.lindstaedt@gmail.com>

silverwind commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

Thanks! Landed in 05b3a0b.

silverwind closed this Mar 30, 2016
evanlucas pushed a commit that referenced this pull request Mar 30, 2016
tools: fix json doc generation
dddd365 
Current processList function in tools/doc/json.js does not recognise
{"type":"loose_item_start"}. Fix it.

PR-URL: #5943
Fixes: #5942
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Robert Lindstädt <robert.lindstaedt@gmail.com>

MylesBorins commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

is this something we want for lts?

silverwind commented Mar 30, 2016

Copy link
Copy Markdown
Contributor

@thealphanerd It fixes an edge case that manifested after #5616. I'd say it's nice to have in LTS as a safeguard.

evanlucas mentioned this pull request Mar 31, 2016
Closed
evanlucas pushed a commit that referenced this pull request Mar 31, 2016
tools: fix json doc generation
cb676cf 
Current processList function in tools/doc/json.js does not recognise
{"type":"loose_item_start"}. Fix it.

PR-URL: #5943
Fixes: #5942
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Robert Lindstädt <robert.lindstaedt@gmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 11, 2016
tools: fix json doc generation
c33a23f 
Current processList function in tools/doc/json.js does not recognise
{"type":"loose_item_start"}. Fix it.

PR-URL: #5943
Fixes: #5942
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Robert Lindstädt <robert.lindstaedt@gmail.com>
MylesBorins mentioned this pull request Apr 11, 2016
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Error found when generating JSON doc

6 participants

Back | FazBrowse Home | New Git URL