Client side play urls #10

ests · 2016-10-04T15:51:38Z

Assembly links to Play Rust out of source files fetched from the server. This is done just after DOM has loaded - every link with special data attribute will have its href replaced.

The other solution was to use various onclick events, but opening windows via javascript will almost always trigger popup blocking.

See #3 .

Files inside directory that is prefixed with underscore are not accessible for the server, and so cannot be fetched via xhr.

For clients that have js disabled?

It seems that sometimes event is not fired if script is loaded in asynchronous manner, and thus nothing will work.

But browsers will block windows opened this way.

This also triggers popup blocker :(

I think doing this in js is a bad idea, and console shouldn't be used in "production" (eslint doesn't like it). A bettter solution would be writing some Ruby script, that checks all source files for any problems with those special comments. The only drawback is that this *.rb script would need to be invoked manually.

ests · 2016-10-05T08:20:13Z

src/assets/js/play_url.js

+  var RUST_VERSION = "nightly";
+  var HOME_URL = window.location.protocol + "//" + window.location.host + "/";
+
+  document.addEventListener("DOMContentLoaded", function() {


This will fail in IE 8. Alternative solution would be putting whole script inline, inside main HTML file, just before closing </body> tag. Not the nicest solution, but should work everywhere.

Meh. I think we can assume fairly modern browsers.

(Well, I guess we just need to put a call to playUrl() inline, after the body tag? (And export it out of the function, obviously.))

Still, I don't care much.

Sure, if anyone complains, it'll be easy to change.

nikomatsakis · 2016-10-05T12:28:03Z

src/assets/js/play_url.js

+  }
+
+  function addTransformations(text) {
+    var solutionRegex = /\/\/ START SOLUTION\s*(.*)\s*\/\/ END SOLUTION/g;


This isn't quite how start/end solution works. The idea is something like this:

fn foo() { // PROMPT xxx // START SOLUTION yyy // END SOLUTION }

would get transformed to:

fn foo() { xxx }

And it should work exactly that way. I can remove (.*) here, but it will work anyway.

Oh, wait. Maybe I am wrong. =) I was assuming that . in JS regexs doesn't match newlines, but maybe it does?

Regardless, this makes me nervous. In particular, you could have multiple sections:

// START SOLUTION xxx // END SOLUTION yyy // START SOLUTION zzz // END SOLUTION

and a regex engine using maximal matching (the typical default) would gobble up xxx, yyy, and zzz. I suppose you could to (.*?), if JS engines support that, and it'd probably work.

. in JS doesn't match newlines, that's why we need s+. I am testing all regexes here: https://regex101.com/r/1nWjUL/1
I already added some modifications to the regex.

And it reminds me... https://www.xkcd.com/208/

So the one you had I think didn't work if there was more than one line in the solution. I made a modified version \/\/ START SOLUTION(\s|.)*?\/\/ END SOLUTION that seems to work though:

https://regex101.com/r/1nWjUL/2

even better: https://regex101.com/r/1nWjUL/3

Nice catch about multiline solutions! I modify this line, but I don't understand why do you need /n? at the end?

nikomatsakis · 2016-10-05T12:28:47Z

This looks pretty good! I left a few comments.

ests · 2016-10-05T14:12:30Z

src/assets/js/play_url.js

+  }
+
+  function addTransformations(text) {
+    var solutionRegex = /\/\/ START SOLUTION\s+.*\s+\/\/ END SOLUTION/g;


As long as your whitespace formatting (new lines) is OK, there is (hopefully) nothing to worry about this expression.

@nikomatsakis

As suggested by @nikomatsakis. Also requiring at least one character to exist between START END solution (even if it's empty line).

nikomatsakis · 2016-12-27T16:33:18Z

Argh, this fell off my radar. I admit I don't feel qualified to know if this is really The Right Way to go about things, but this seems like a good approach to me.

@ests did you delete the original repo? I was going to poke about and try it locally, but I can't seem to figure out where to merge from.

ests · 2016-12-27T17:46:28Z

@nikomatsakis I was sure this won't be accepted so I was too quick to delete it - sorry. I also don't have it on my laptop. I don't know if this is (was) the right way to go. But now probably the only way to play with it, would be to somehow to generate diff file on github and apply it locally, but I don't know if this is possible (easily).

nikomatsakis · 2016-12-27T21:00:42Z

@ests good thinking. https://patch-diff.githubusercontent.com/raw/nikomatsakis/intorust/pull/10.diff seems to work.

nikomatsakis · 2017-07-17T10:41:21Z

Argh, I've let this sit for way too long! I'm inclined to merge it at this point =) Do these kinds of URLs still work?

ests added 18 commits October 3, 2016 23:03

Exercise link should href to fallback page.

8855a60

Include initial version of js script.

71a46be

Change folder of exercises.

d7dfa22

Files inside directory that is prefixed with underscore are not accessible for the server, and so cannot be fetched via xhr.

Add temporary fallback html file.

625459e

For clients that have js disabled?

Remove async attribute.

c1ab086

It seems that sometimes event is not fired if script is loaded in asynchronous manner, and thus nothing will work.

Naive implementation.

bddc74e

But browsers will block windows opened this way.

Another try with onclick attribute.

236f418

This also triggers popup blocker :(

Worst case scenario: display list of rust files.

2ac5341

Add eslint config file.

97da382

Set initial exercise links to void.

8d3f869

Add CSS for "inactive" exercise links.

c3d5d95

Various cleanups and refactorings.

97c7b79

Add transformations for PROMPT and home.url.

9000ae7

Add PROMPT and SOLUTION transforms.

48abd11

Fix a typo.

9da238b

Remove autogenerated file.

b11fc44

Missing semicolon.

6da22e6

ests commented Oct 5, 2016

View reviewed changes

nikomatsakis reviewed Oct 5, 2016

View reviewed changes

Remove unnecessary capturing group.

b80c654

ests commented Oct 5, 2016

View reviewed changes

Fix regex to support multiline solutions.

b38470b

As suggested by @nikomatsakis. Also requiring at least one character to exist between START END solution (even if it's empty line).

nikomatsakis merged commit 78451e5 into intorust:master Nov 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Client side play urls #10

Client side play urls #10

ests commented Oct 4, 2016

ests Oct 5, 2016

nikomatsakis Oct 5, 2016

nikomatsakis Oct 5, 2016

ests Oct 5, 2016

nikomatsakis Oct 5, 2016

ests Oct 5, 2016 •

edited

Loading

nikomatsakis Oct 5, 2016

ests Oct 5, 2016 •

edited

Loading

nikomatsakis Oct 6, 2016

nikomatsakis Oct 6, 2016

ests Oct 6, 2016

nikomatsakis commented Oct 5, 2016

ests Oct 5, 2016

nikomatsakis commented Dec 27, 2016

ests commented Dec 27, 2016

nikomatsakis commented Dec 27, 2016

nikomatsakis commented Jul 17, 2017

Client side play urls #10

Client side play urls #10

Conversation

ests commented Oct 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ests Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ests Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Oct 5, 2016

Choose a reason for hiding this comment

nikomatsakis commented Dec 27, 2016

ests commented Dec 27, 2016

nikomatsakis commented Dec 27, 2016

nikomatsakis commented Jul 17, 2017

ests Oct 5, 2016 •

edited

Loading

ests Oct 5, 2016 •

edited

Loading