Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add Atom's API docs to the Flight Manual #541

Merged
merged 69 commits into from
Jul 26, 2019
Merged

Add Atom's API docs to the Flight Manual #541

merged 69 commits into from
Jul 26, 2019

Conversation

jasonrudolph
Copy link
Contributor

@jasonrudolph jasonrudolph commented Jun 27, 2019

This pull request aims to bring the Atom API reference docs into the Flight Manual. These docs currently reside at https://atom.io/docs/api, and we'd like for them to move them into this repository so that all of Atom's docs are consolidated in a single place.

TODO

  • Enhance the build to generate API docs at /api/:version/:class (for example: /api/v1.38.2/AtomEnvironment)
  • Enhance the search index to include the API docs
  • Show the list of classes in the left sidebar (to match current behavior on https://atom.io/docs/api)
  • When navigating to the root URL (/api), show the docs for the current stable version (387eeee...de1e02a)
  • Provide drop-down menu for choosing the Atom version whose docs you want to view
  • Compare the output side-by-side with the content at https://atom.io/docs/api and fix any bugs
    • "Essential" classes should include an "Essential" label at the top of the page [screenshot] (60234aa)
    • Code samples should be rendered with syntax highlighting [screenshot]
    • Optional parameter should have an "optional" badge [screenshot] (5a0f390)
    • "Extended" methods should be grouped together in an "Extended Methods" subsection, and they should be hidden by default [screenshot]

/cc @lee-dohm @as-cii

lee-dohm and others added 30 commits April 30, 2019 10:14
Replace the `split_api_json` Rake task with equivalent logic in 
`gulpfile.js`, so that `npm run gulp` continues to provide the 
end-to-end build process.
Prior to this change, the the 'nanoc:compile' gulp task attempted to 
store the complete output of `nanoc:compile` in a string, which resulted 
in a "stdout maxBuffer exceeded" error. By using `spawn` instead of 
`exec`, we can pipe the output of `bundle exec nanoc compile` to STDIO 
and thus avoid trying to fit it all in a string.

Example build failure: 
https://travis-ci.org/atom/flight-manual.atom.io/builds/550937870#L3656
Prior to this change, we were getting the following failure on CI:

> No output has been received in the last 10m0s, this potentially
> indicates a stalled build or something wrong with the build itself.
> ...
> The build has been terminated

This was due to the `run_proofer` task running for more than 10 minutes
with no output. To prevent that situation, this commit introduces two
changes:

1. Split up the proofer work. Invoke HTMLProofer on each directory
separately, so that the `run_proofer` task reports intermediate
progress. Instead of waiting until _all_ directories have been proofed
before outputting any results, it will now output results for each
directory immediately after proofing that directory.

2. Don't run HTMLProofer on the API docs. Checking all external links
for all versions of the API docs would likely take a l-o-n-g time. I
don't think we want our builds taking that long. If we _do_ want to run
the proofer on all versions of the API docs, perhaps it could happen as
part of a nightly build, as opposed to happening on every push.

Example build failure: https://travis-ci.org/atom/flight-manual.atom.io/builds/550971951#L7122
Enhance build to process API JSON files for Atom 1.0.0 and newer
Co-authored-by: Antonio Scandurra <[email protected]>
Note: The code samples emitted by this change are not yet rendered with 
syntax highlighted.
This reverts commit 22558dd and adopts an alternative approach to the
problem. We continue to *NOT* run the link checker on the API docs (for
the reasons described in the commit messages for 22558dd), but we run
the HTMLProofer on the root of the output directory, so that internal
links can be properly resolved.
With these changes, we populate the search index with all instance
methods present in the latest version of Atom. Next up, we'll need to
add classes, class methods, and instance properties to the search index.

Co-Authored-By: Antonio Scandurra <[email protected]>
Co-Authored-By: Antonio Scandurra <[email protected]>
@jasonrudolph jasonrudolph marked this pull request as ready for review July 16, 2019 21:14
@jasonrudolph
Copy link
Contributor Author

@lee-dohm: I think this is looking pretty good now. Would you mind trying out these changes to see if there's anything that's essential to change before we ship?

Note: Given the size of this diff and how long this branch has been in progress, I think we should lean heavily toward getting to a reasonable production "v1" of these changes as soon as possible and deferring any enhancements to separate pull requests. 😇

@lee-dohm
Copy link
Contributor

I'll take a look at this first thing in my morning tomorrow. Thanks so much for all the work driving this forward @jasonrudolph and @as-cii 🙇 I agree that a reasonable v1 is the appropriate action here.

Antonio Scandurra added 2 commits July 17, 2019 11:52
With the introduction of kramdown in 73b0e94, we started interpreting
newline characters as hard wraps, which was causing paragraphs to not
take advantage of all the available screen real estate. Documentation is
usually hard-wrapped for readability purposes when navigating source
code, but those concerns don't apply when viewing the same documentation
in a web browser.

This commit fixes the visual glitch illustrated above by supplying
`hard_wrap: false` to  kramdown-gfm. I verified that this doesn't affect
the rendering of other markdown constructs such as code block syntax
highlighting.
This ensures that Safari doesn't apply a custom style that prevents the 
text box from being displayed correctly.
@lee-dohm lee-dohm marked this pull request as ready for review July 17, 2019 17:55
@lee-dohm
Copy link
Contributor

This looks great! I've marked the PR as ready for review while I dig deeper into it.

@calebmeyer
Copy link
Contributor

calebmeyer commented Jul 17, 2019

From taking a quick look at the website:

  • I like the search, although I wish the indexing were breadth first instead of depth first. Searching "marker" gets you lots of methods under MarkerLayer, but nothing about DisplayMarker.
  • I think our sidebar nav is getting a bit long. I'd almost rather see the headers be collapsible, even if they start expanded. One of the things I liked about the separate API docs was that I could see quite a few of the classes without any scrolling. Now even veterans have to scroll quite a bit to get past the sections on installing atom, and editing text, etc.
  • I didn't see any straight up bugs from clicking around. The mac/windows/linux selector works, all the highlighting looks right, all the links worked, including source code links. 👍

Edit: I'm also in favor of a quick v1. These are definitely not show stoppers. If you'd like, I can open issues for them.

@@ -0,0 +1,58 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we append .erb on these files? It would make them more likely to syntax highlight correctly in whatever editor you're using, including github web.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. Would you mind opening an issue for this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #549

lib/temp.rb Outdated Show resolved Hide resolved
lib/temp.rb Outdated Show resolved Hide resolved
layouts/api-class.html Outdated Show resolved Hide resolved
bindQuicksearchMousedown() {
$(".autocomplete-results a").each(function(_, el) {
$(el).bind("mousedown", function(event) {
event.preventDefault();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just preventing the default and returning? I'm assuming this is so the anchors don't try to visit their hrefs. Should we use a different element like <button> in that case to be more semantically accurate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a valid change but probably should be put off until after this is deployed. Would you mind opening an issue?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #550

@@ -0,0 +1,218 @@
$(document).ready(function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a shorthand for this. Any function given directly to the jQuery function is called on document ready.

Reference

Copy link
Contributor

@calebmeyer calebmeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 🚢

Copy link
Contributor

@lee-dohm lee-dohm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless someone else sees something I missed, this looks good to go to me 🚀

@jasonrudolph
Copy link
Contributor Author

Thanks for the review! I'm gonna roll this out now.

Despite all our testing and code review, I anticipate us discovering at least some issues in real-world use due to the sheer magnitude of this change. To minimize the impact, we'll keep an eye on issues filed over the next few days, and we'll try to respond as quickly as possible to any problems that come up.

@jasonrudolph jasonrudolph merged commit 8185516 into master Jul 26, 2019
@jasonrudolph jasonrudolph deleted the api-docs branch July 26, 2019 13:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants