Skip to content

Conversation

@mklabs
Copy link

@mklabs mklabs commented Nov 21, 2011

Hello there,

So here is a pull request asking you to review and merge the work @addyosmani and I have done the last few weeks into trying to find a simple mechanism to handle exercises, relying on gists to both have fiddles and embed snippets of code.

This is a little node package exposing a basic cli tool to parse/generate/update repository files, intended to be used right after a nanoc compile.

I've included in the pull request a readme with more informations on what this "build script" is trying to do.

In a nutshell:

  • the script first parse the content of the content/ folder, looking for any files ending with exercises.md.
  • In these markdown files, the script parses the content searching for Use the file and Open the file patterns. They're usually employed to know to which files the exercises are related, to then generate the fiddles folder.
  • Once the script has necessary informations, it starts to generate for each exercises found (a *-exercises.md page may include a few) the corresponding jsfiddle ready folder. With fiddle.js matching the Use the file <path> and fiddle.html matching the Open the file <path>. fiddle.css is always the same (guessed from the relative path found in html). fiddle.details.
  • if config.gists is set to true, the script then uses ngist to create a new private gist based on git config --get github.user and git config --get github.token values.
  • Third and final step, the script tries to update the pages in output/ that were generated by a previous nanoc compile build, with the according jsfiddle link and the gists created as embed code.

Note that I've also included in this pull request a slight modification of the Rakefile. I've added a new rake task to generate the website with fiddles integration (which is simply running nanoc compile && cd fiddles && npm install && ./bin/fiddles).

There is some edge case, and things I'd like to see improved. I'll probably do another pull request to address some of these issues:

  • right now, new gists are created on each build generation. This is not ideal. It would be nice to have the script generate new gists only if there is no folder created yet, and if any try to use git commit/push to update the remote gists.
  • proxy support: currently the script doesn't support generation done behind a proxy. ngist, AFAIK, is not able to issue https request through proxies, it may be a problem to some. I'm considering the use of defunkt's gist tool, seems like it handles proxies quite nicely.

I wanted to paste here the command line result of running the script, but this pull request description is getting really verbose :) (and it's included in fiddles/readme.md anyway).

Cheers,

@addyosmani
Copy link
Member

Thanks so much for your work on this so far (and the pull request), Mickael. @ajpiano, I think it would be useful to have you review this before it gets merged just so we can tweak any changes needed beforehand.

@ajpiano
Copy link
Member

ajpiano commented Nov 21, 2011

Thanks guys - I'll be looking this over like, right now.

Copy link
Member

Choose a reason for hiding this comment

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

This needs to be http://stage.learn.jquery.com while we're in a staging only mode, and then changed to the proper http://learn.jquery.com when we're ready

Copy link
Author

Choose a reason for hiding this comment

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

Probably the configuration needs some more love. I'm thinking about using a real opt parser, and possibly the option of prompting users at the beginning of the script (to override default values). That way, the host value may be easily changed either through command line usage, or when prompted.

Copy link
Member

Choose a reason for hiding this comment

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

Definitely add a normal opt parser, it's not really clear to me how to pass the options to it as is?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, my fault, right now it still needs manual update of the config object in bin/fiddles. I'm considering the use of nopt or optimist, so this will soon change to something better allowing you to tweak the options directly from command line usage.

@ajpiano
Copy link
Member

ajpiano commented Nov 22, 2011

So first off, this is incredible -- really great work so far.

A couple of things I'm noticing as I review it this and try to get it going

  • The pathing to images in fiddles isn't working. This is because currently, nothing in the 'code' directory is actually built into the static content and ever making it a web-accessible directory in http://learn.jquery.com. This might require some re-jiggering of directories, what do you think the optimal layout is here?
  • Not sure why we're outputting the entire content of all 4 gists onto the 'exercises' page in a given directory? The CSS, HTML, etc? It seems to me we should just be replacing all the "open the file, use the file" stuff with "Open this jsFiddle example in another window" with the instructions below it - or just embedding the fiddle outright. If the gists are just scaffolding pieces for the fiddles, perhaps they don't need to be embedded, just linked?

Copy link
Member

Choose a reason for hiding this comment

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

It's probably a lot easier to just pull this code out of the exercise templates themselves than stripping it out here? Unless we're aiming for the 'exercises' to work properlywhen the repository is cloned...

Copy link
Author

Choose a reason for hiding this comment

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

Yep, it could probably be done that way. Like you mentioned, letting the fiddle tool handle this allows one to get the exercises that work when the repo is cloned, or when an exercise is downloaded (thanks to the download archive you also mentioned).

@addyosmani
Copy link
Member

Thanks for the review, Adam!

On the second point, the idea was for us to allow visitors to see all of the code needed for a particular exercise in-page without needing to rely on jsFiddle's embed features - as you know the site's reliability has been sketchy lately which is why we wanted to still let them view code samples using a reliable source (GitHub - includes version history for updates, easy to fork) but also offer the fiddle link. If I've misinterpreted your concern, let me know!.

@ajpiano
Copy link
Member

ajpiano commented Nov 22, 2011

Totally hear you on the jsFiddle reliability stuff - unfortunately with the gists, the pages get LOOOOOOOOONNGGGG really quickly, and most of that is noise like uncompressed CSS and the instructions duplicated inside of the gist. That's why I was saying we could just embed the fiddle and provide links, but not embedded gists.

We could also investigate actually hosting the built example and solution files on the site, or as a download, as another backup, or for people who want to actually do the work outside of jsfiddle. If we aren't planning to have the source from exercises and solutions ever actually work in a standalone browser, then we should strip the script tags out of those pages instead of doing so with the fiddles tool.

Are we planning to do this same process for the solutions as well? Would seem to be nice...?

@addyosmani
Copy link
Member

I think it would make sense to have the same process in place for the solutions. If we don't end up opting for embedded gists, having something like hosted examples/solutions or another backup that tied to this overall process could definitely be helpful. Was there a preference here for how that might be done or what form it might take?

Again, just thinking of the people that might land on a page, click a fiddle link and give up if it times out or anything like that.

@ajpiano
Copy link
Member

ajpiano commented Nov 22, 2011

Seems to me like the preferable thing to have would be the link to the fiddle and a link to download that entire exercise in an archive? I imagine something like that could be built into the nanoc preprocess.

@mklabs
Copy link
Author

mklabs commented Nov 23, 2011

Thanks Adam for the review!

unfortunately with the gists, the pages get LOOOOOOOOONNGGGG really quickly, and most of that is noise like uncompressed CSS and the instructions duplicated inside of the gist.

I totally agree, actually it's something that I should have mentioned in the pull request description. Instead of embedding all the gist's files, we may add a ?file=fiddle.js suffix to the script src, eg. something like:

<script src="https://gist.github.com/7de1f9aab65218225f04.js?file=fiddle.js"></script>

At first I didn't see that option, but using this may reduce all the noise that the pages are getting.

Seems to me like the preferable thing to have would be the link to the fiddle and a link to download that entire exercise in an archive?

Sounds like a reasonable options too. For the download archive, maybe we could rely on the download feature for gists (eg. https://gist.github.com/gists/7de1f9aab65218225f04/download). That way, the script could add the corresponding links.

Are we planning to do this same process for the solutions as well? Would seem to be nice...?

I'm not sure this is the ideal implementation, but I've added some code to handle solutions as well (see here or here). Basically, I'm switching the path depending on the solution mode, looking in solutions/ instead of exercises/ dir.

Basically, I change the path to lookup the js file depending on the solution mode, looking in solutions/ instead of exercises/ dir. Though, you may want to edit the way it's done to handle both sources at once. Right now, it is quite exclusive.

I'll add a few notes and commits related to your other comments.

@mklabs
Copy link
Author

mklabs commented Nov 23, 2011

For the images part:

The pathing to images in fiddles isn't working. This is because currently, nothing in the 'code' directory is actually built into the static content and ever making it a web-accessible directory in http://learn.jquery.com. This might require some re-jiggering of directories, what do you think the optimal layout is here?

First implementation did use inline base64 img to do this, but it was removed in favor of this host/assets mechanism from config (due to the fact that we ideally want it to work cross-browser).

It may require some repo restructure, either by making the img used in exercises available in the static content or other option. Though, in this first version of this fiddle tool, I really wanted it to work as is, without any modifications from your part.

@addyosmani
Copy link
Member

@ajpiano

@mklabs has made further updates to the original PR related to how we handle images. From my perspective the above looks fine to merge, but it would be great to know if you had further thoughts about our approach to this PR as a whole or were mostly happy with the direction it was going. We can tweak it further if needed.

@ajpiano
Copy link
Member

ajpiano commented Dec 14, 2011

A bunch is changing in terms of how the contents of this repo will be making it online (the content is being shoved into wordpress, after being processed by nanoc), so i've been holding off on merging this in until that piece of the puzzle is in place. I'm definitely mostly happy with the direction, and I'll be working on mixing this in to the rest of the build process before we merge it.

@mklabs
Copy link
Author

mklabs commented Dec 15, 2011

Feel free to ping me whenever you need some support with this, or if it needs further work (I'm sure it does).

november-december is a super busy period for me, so it is a bit difficult for me at the moment. Hopefully, that will soon change. Anyway, if the content changes, the script will most likely need some adjustments. That's perfectly fine, if it needs to, I'll be happy to make the necessary changes or help doing so.

@addyosmani
Copy link
Member

For historic reference, we initially discussed the exercise handling in #27. I'm closing that issue as the work here supersedes it.

@addyosmani
Copy link
Member

@ajpiano given there was a relatively non-trivial amount of work done for this PR do we want to try getting it in? I know earlier in the year there were still a lot of moving parts to the project infrastructure wise. Have those settled down? :)

@ajpiano
Copy link
Member

ajpiano commented Jun 30, 2012

My gut instinct here is that this actually makes sense to be adapted into a grunt task, as now everything is being handled by grunt-wordpress and friends. Then the hard work would actually be able to be leveraged by a lot more projects than just the learn site!

@addyosmani
Copy link
Member

Absolutely! That sounds like a very sensible idea. Given @mklabs experience
with grunt I imagine it shouldn't be too huge an amount of work to get this
ported over. I guess the question is what kind of grunt task does this
manifest as? Code embed from multiple services?

Ps: Do we have any ideas about site launch timelines just yet? :) I know
Mickael is helping us out with Yeoman and might not be able to get to this
for a week or two depending on how much free time he has :)

On Jun 30, 2012 4:26 PM, "adam j. sontag" <
reply@reply.github.com<javascript:_e({},
'cvml',
'reply%2Bi-2308177-83745bd0a1d8ee5626a43c719d4e3d2a87abaf2d-110953@reply.github.com');>>
wrote:

My gut instinct here is that this actually makes sense to be adapted into
a grunt task, as now everything is being handled by grunt-wordpress and friends. Then the
hard work would actually be able to be leveraged by a lot more projects
than just the learn site!


Reply to this email directly or view it on GitHub:
#30 (comment)

Addy Osmani

Developer Programs Engineer at Google
Blogger at: http://addyosmani.com
Phone: +44 7501 594 382

@addyosmani
Copy link
Member

@ajpiano Just pinging to find out what the current status on the site architecture is. Would sometime in the next few weeks be realistic for us to look at a grunt task capturing the word in this issue?

@ajpiano
Copy link
Member

ajpiano commented Oct 16, 2012

Hate to be a party pooper, but after the long silence on this issue, I think we're not going to be including interactive exercises as part of the learn.jquery.com project (we discussd this today more at the summit) - we have an exciting project in the works for interactive examples as part of a course, and I think that will supersede this in terms of the jQuery network. However, I think there's still a lot of value in the fundamental idea of a grunt task that takes structured files and and creates the proper gists for consumption by jsfiddle.

Thanks for all your hard work here @mklabs, and sorry that this didn't quite end up working out as we'd hoped for now.

@mklabs
Copy link
Author

mklabs commented Oct 17, 2012

@ajpiano no worries! happy to help whenever I can, not a big deal it doesn't end up into something actually used. Was fun to experiment on the idea.

@addyosmani
Copy link
Member

Thanks for the update @ajpiano. Appreciate the heads up about the other interactive project being worked on for exercises.

Also, really appreciate all the time you put into this @mklabs. Maybe we'll be able to repurpose this work sometime in the future elsewhere :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants