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

build: move to the new docs parser #18103

Merged
merged 6 commits into from May 6, 2019

Conversation

Projects
None yet
3 participants
@MarshallOfSound
Copy link
Member

commented May 1, 2019

Switched from:

  • electron-docs-linter --> @electron/docs-parser
  • electron-typescript-definitions --> @electron/typescript-definitions

This makes our definition file way better, we don't lose any information in the docs. Noticeably this means markdown, links and code blocks are persisted in the TS definitions. It is also much better under the hood so future improvements / fixes will be much easier. E.g. readonly support is in a branch somewhere :)

I'm planning on writing a blog post at some point detailing how @electron/docs-parser now works. But from a thousand foot view. doc-linter used to parse the markdown in HTML and then parse the docs as a DOM structure with jquery and selector querying. This had a few issues the biggest being it was very hard to semantically now what was "inside" a heading when the generated HTML just looks like this:

<h3>Method()</h3>
<p>stuff</p>
<p>more stuff</p>
<p>stuff</p>
<h3>Next Method()</h3>

The new docs-parser module parses the markdown file into a markdown AST which does know where things start and end so it makes parsing things way easier and it means we can losslessly maintain the markdown in each block without worrying about HTML entities or escaping or anything like that.

This PR also fixes a bunch of issues with a current documentation, highlights below.

  • app.commandLine.* and app.dock.* are now in separate "module" files. Lumping them in with app.md over-complicated the parser and the read experience.
  • Fixed a bunch of Event documentation not having Returns: (the magic keyword to document the event properties)
  • Added a bunch of - and : and \n so that documentation for properties in deep objects are correctly parsed.

Notes: Switched to a new Typescript Definitions generator. This means that some interface names may have changed, if your Typescript build is failing this is the cause.

@MarshallOfSound MarshallOfSound force-pushed the new-parser branch 6 times, most recently from 52ae92e to d24c458 May 1, 2019

@MarshallOfSound

This comment has been minimized.

Copy link
Member Author

commented May 1, 2019

So the diff in the electron.d.ts file is literally Too Big for the Artifact Comparison check to deal with. So here is the diff:

https://gist.github.com/MarshallOfSound/14fdce4022c3453eeb24c7520f0c8356

And here is the actual new file:

https://gist.github.com/MarshallOfSound/7773cc6501a303fa6dfb62792d477ba1

@MarshallOfSound MarshallOfSound force-pushed the new-parser branch 2 times, most recently from 0f34737 to 4fd63bd May 2, 2019

@MarshallOfSound MarshallOfSound force-pushed the new-parser branch from 4fd63bd to 07c7172 May 2, 2019

Show resolved Hide resolved docs/api/app.md Outdated
Show resolved Hide resolved docs/api/app.md Outdated
Show resolved Hide resolved docs/api/app.md Outdated
Show resolved Hide resolved docs/api/app.md Outdated
Show resolved Hide resolved docs/api/app.md Outdated
Show resolved Hide resolved docs/api/menu-item.md
Show resolved Hide resolved docs/api/process.md
Show resolved Hide resolved docs/api/structures/trace-categories-and-options.md Outdated
Show resolved Hide resolved docs/api/web-contents.md
Show resolved Hide resolved docs/api/webview-tag.md Outdated

malept and others added some commits May 3, 2019

chore: apply suggestions from code review
Co-Authored-By: MarshallOfSound <samuel.r.attard@gmail.com>
Show resolved Hide resolved docs/api/command-line.md Outdated
chore: update docs/api/command-line.md
Co-Authored-By: MarshallOfSound <samuel.r.attard@gmail.com>
@malept

malept approved these changes May 3, 2019

Copy link
Member

left a comment

Seems legit.

@jkleinsc jkleinsc merged commit a96b6e2 into master May 6, 2019

12 of 13 checks passed

Artifact Comparison
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20190503.39 succeeded
Details
electron-arm64-testing Build #20190503.43 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented May 6, 2019

Release Notes Persisted

Switched to a new Typescript Definitions generator. This means that some interface names may have changed, if your Typescript build is failing this is the cause.

@jkleinsc jkleinsc deleted the new-parser branch May 6, 2019

Kiku-git added a commit to Kiku-git/electron that referenced this pull request May 16, 2019

build: move to the new docs parser (electron#18103)
* build: move to the new docs parser

* chore: remove the bad getTitle param doc

* build: update parser/ts gen deps + fix some docs issues highlighted by GH desktop

* chore: apply suggestions from code review

Co-Authored-By: MarshallOfSound <samuel.r.attard@gmail.com>

* chore: update docs for accidentally removed things

* chore: update docs/api/command-line.md

Co-Authored-By: MarshallOfSound <samuel.r.attard@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.