-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add Atom's API docs to the Flight Manual #541
Conversation
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]>
Co-Authored-By: Antonio Scandurra <[email protected]>
Co-Authored-By: Antonio Scandurra <[email protected]>
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]>
@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. 😇 |
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. |
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.
This looks great! I've marked the PR as ready for review while I dig deeper into it. |
From taking a quick look at the website:
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 @@ | |||
{ |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened #549
bindQuicksearchMousedown() { | ||
$(".autocomplete-results a").each(function(_, el) { | ||
$(el).bind("mousedown", function(event) { | ||
event.preventDefault(); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 🚢
There was a problem hiding this 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 🚀
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. |
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
/api/:version/:class
(for example:/api/v1.38.2/AtomEnvironment
)/api
), show the docs for the current stable version (387eeee...de1e02a)/cc @lee-dohm @as-cii