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

docs(manual): write some content (introduction, observable, etc) #1390

Closed
wants to merge 13 commits into from

Conversation

staltz
Copy link
Member

@staltz staltz commented Feb 25, 2016

This PR has mostly text/content being added to the documentation.

To see this page live, open http://rxjs5-esdoc-manual-content-wip.surge.sh/.

screen shot 2016-02-25 at 16 39 51

@staltz
Copy link
Member Author

staltz commented Feb 29, 2016

Depends on esdoc/esdoc#231 getting fixed.

@benlesh
Copy link
Member

benlesh commented Mar 1, 2016

LGTM

@benlesh
Copy link
Member

benlesh commented Mar 1, 2016

Alternatively, perhaps we can get @Widdershin to give us a workaround for esdoc vomiting on HTML comments? ... I have no idea what that would be... a config maybe? I dunno. I'm just trying to push the effort forward.

@benlesh benlesh added blocked and removed PR: lgtm labels Mar 1, 2016
@benlesh
Copy link
Member

benlesh commented Mar 1, 2016

Marking as blocked because of the esdoc issue.

@staltz
Copy link
Member Author

staltz commented Mar 1, 2016

Indeed blocked. And no response from ESDoc side so far, perhaps I can try solving it myself and submitting a PR. The other option being turning off markdown-doctest for now.

@Widdershin
Copy link
Contributor

Not sure there's anything I can do on the markdown-doctest side (although suggestions are welcome).

Without fully understanding the problem, I made a little fix.

(esdoc Publisher/Builder/util.js)

    sanitizer: (tag) =>{
+      if (tag.match(/<!--.*-->/)) {
+        return tag;
+      }
+
        const tagName = tag.match(/^<\/?(\w+)/)[1];
        if (!availableTags.includes(tagName)) {
          return escape(tag);
        }

        const sanitizedTag = tag.replace(/([\w\-]+)=(["'].*?["'])/g, (_, attr, val)=>{
          if (!availableAttributes.includes(attr)) return '';
          if (val.indexOf('javascript:') !== -1) return '';
          return `${attr}=${val}`;
        });

        return sanitizedTag;
      },

Would really love to avoid turning off markdown-doctest, especially while new docs are being written (would love to encourage people to write examples that actually run 😉)

@staltz
Copy link
Member Author

staltz commented Mar 1, 2016

Would really love to avoid turning off markdown-doctest, especially while new docs are being written

Me too. Was that fix for ESDoc?

@Widdershin
Copy link
Contributor

Yep! Publisher/Builder/util.js

@Widdershin
Copy link
Contributor

It would be awesome if you could see about getting that fix merged @staltz, I'm pretty overloaded right now so I'm not sure when I would have time

@staltz
Copy link
Member Author

staltz commented Mar 1, 2016

Thanks for pointing me in the right direction @Widdershin :)

@staltz
Copy link
Member Author

staltz commented Mar 3, 2016

I submitted a PR to ESDoc:
esdoc/esdoc#239

This PR is blocked until that is merged and a new version is released.

@staltz
Copy link
Member Author

staltz commented Mar 3, 2016

This PR is blocked until that is merged and a new version is released.

Uhmm, I take that back. I'm baffled that the tests just passed in Travis :|

@staltz
Copy link
Member Author

staltz commented Mar 7, 2016

Hello, this PR should now be unblocked

@kwonoj
Copy link
Member

kwonoj commented Mar 7, 2016

Change looks good to me too. Since @Blesh already confirmed changes, I'll check this in later today. (give small time buffer just in case any other suggestion comes in)

@kwonoj kwonoj added the PR: lgtm label Mar 7, 2016
@staltz
Copy link
Member Author

staltz commented Mar 7, 2016

Yeah anyway it's just to kickstart the content, replacing TODO with actual content. We may freely modify/improve it afterwards.

@kwonoj
Copy link
Member

kwonoj commented Mar 8, 2016

Merged with 935212a, thanks @staltz :)

@kwonoj kwonoj closed this Mar 8, 2016
@staltz staltz deleted the esdoc-manual-content branch March 8, 2016 08:43
@lock
Copy link

lock bot commented Jun 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
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.

None yet

4 participants