ocaml / v2.ocaml.org

Implementation of the ocaml.org website.
http://ocaml.org
Other
323 stars 348 forks source link

The new 99problems page is poor #211

Closed Chris00 closed 10 years ago

Chris00 commented 10 years ago
  1. One cannot spot the questions well anymore.
  2. The solutions are not hidden — a problematic fact because people will see the solution even if they do not want to.
  3. The code colors are uncomfortable for the eyes (and ugly IMHO).
  4. There is too much vertical space between the lines of code.
  5. The examples executing the code are missing — they were important for the learner to test his code before looking to the solution.
amirmc commented 10 years ago

@pw374 I think there was some specific Javascript for this page. Is that still around for the new design?

amirmc commented 10 years ago

Marking this as blocking. One way to fix it might be to go back to html for this page. I'll try that now in a separate branch.

Chris00 commented 10 years ago

More generally, I think, is the question how to use templates to enrich parts of a Markdown document. I do not see how MPP can be used here because the code it generates will be included in the Markdown document (hence, say, ((! begin xxx !)) markdown ((! end xxx !)) will not work). One could distinguish the two ways of making italic and, as was done specifically for TOC, allow, say, *template xxx* to load a plugin to process the markdown AST (using omd as a library, a special version could be compiled defining this special extension). I am thinking out loud, other ideas may be envisioned.

pw374 commented 10 years ago

If you want to have an HTML file with some markdown inside, just do this:

some html
((! cmd omd
some markdown
!))
some html
amirmc commented 10 years ago

I've just realised that my attempts to fix this page are half-baked and not going to work. @pw374 D'you have time to look at this today?

pw374 commented 10 years ago

@pw374 I think there was some specific Javascript for this page. Is that still around for the new design?

I think it's gone, but it's not too hard to get it back

I've just realised that my attempts to fix this page are half-baked and not going to work.

what do you mean?

@pw374 D'you have time to look at this today?

I'm currently looking at this. (but I'm not sure I'll come up with a nice solution today.)

amirmc commented 10 years ago

I don't have a problem manually editing this page if you can give me a couple of examples of how the code snippets should be handled (along with solution buttons). I naively took the code from the original page [1] thinking that would be ok, but of course it wasn't.

[1] https://github.com/ocaml/ocaml.org/blob/master/src/html/tutorials/99problems.html (specifically the script at the top and the code sections)

pw374 commented 10 years ago

You might want to look at http://ocaml.org/tutorials/99problems.html instead of the github page, if you want to know how the javascript works. However, editing 99 problems manually sounds bad (because there's a lot of them, even if some of them don't have any solution and are easier to address).

Chris00 commented 10 years ago

On Fri, 15 Nov 2013 06:28:56 -0800, Philippe Wang wrote:

((! cmd omd some markdown !)) some html

Ah, OK. Maybe one can develop something to try to preserve the Markdown input as much as possible? I can try to propose something but i likely will not be today.

amirmc commented 10 years ago

The JS was only part of the problem. The code itself needs to be evaluated and the output added to the page.

Manual work may be time-consuming but I'm willing to do it if it gets us to a working page. The 99 problems page was one of those held up as needing to function on the new site and I forgot to check on it (even despite Christophe submitting an issue).

amirmc commented 10 years ago

The code itself needs to be evaluated and the output added to the page.

The same is true of the learn/taste.html page

pw374 commented 10 years ago

On 15 Nov 2013, at 14:48, Amir Chaudhry notifications@github.com wrote:

The JS was only part of the problem. The code itself needs to be evaluated and the output added to the page.

When we embed and ship tryocaml, the output can be computed on the client-side, but I agree it'd still be nice to compute it when doing the page-generation.

Chris00 commented 10 years ago

((! cmd omd some markdown !)) some html

BTW, is there a simple way so that the command share a common state? the code on a page may depend on earlier code on the same page. What is the best way to do that with MPP?

pw374 commented 10 years ago

When MPP is used to embed OCaml as a preprocessor language (mpp -l ocaml), there's a global (shared) state by default because it converts the whole document into an OCaml program (the -l option only support OCaml currently because I haven't had the time to implement it for other languages). But sharing the same state for a command is a lot trickier...

OMD should provide a way to share a state at some point (i.e., link definitions), it can be made a priority if that's what's needed...

Chris00 commented 10 years ago

I tried various possibilities and here is the one I came up with. Using MPP is not convenient because it executes after the file has been transformed to HTML and one would have to extract the information from the generated HTML which is impractical and brittle. So the solution was to adopt a convention in Markdown. To use a block already present in Markdown (instead of adding my own conventions of what constitutes the delimiters of a solution), I decided to use blockquotes. Not all blockquotes are processed: only those with SOLUTION before them. Example:

SOLUTION
> Recall that `d` divides `n` iff `n mod d = 0`.  This is a naive
> solution.  See the [Sieve of
> Eratosthenes](http://en.wikipedia.org/wiki/Sieve_of_Eratosthenes) for a
> more clever one.
> 
> ```tryocaml
>   let is_prime n =
>     let n = abs n in
>     let rec is_not_divisor d =
>       d * d > n || (n mod d <> 0 && is_not_divisor (d+1)) in
>     n <> 1 && is_not_divisor 2
> ```

The code is also passed to OCaml for evaluation.

Ideally, I would have replaced omd with one version defining these conventions (everything happening at the Markdown level) but I did not want to copy the whole omd_main so I want for a preprocessor outputting the modified Markdown.

The branch pbm contains these enhancement (ATM, you need my branch of omd which fixes several bugs). Note that before publishing it, the whole page should be ported to the new conventions.

Chris00 commented 10 years ago

BTW, the solutions are visible by default in that branch because it makes it easier to fixes things. One just have to uncomment display: none; in the CSS file to enable the expected behavior.

Chris00 commented 10 years ago

You can use version 0.7.0 of omd which incorporates my fixes.

Chris00 commented 10 years ago

@amirmc Are you willing to complete the page transformation?

amirmc commented 10 years ago

I'm happy to do the manual work but I'm a little confused. I've just run make preview on the pbmbranch and the only thing visible on the 99problems page is the table of contents. The same is true of the /learn/taste.html page.

I can see that you've added a few of the blockquotes (thank you) and I'm very happy to add them to the rest but I can't check how they appear. Am I missing something?

amirmc commented 10 years ago

I have mpp.0.1.3 and omd.0.7.0.

Chris00 commented 10 years ago

That's a problem with the current build. A task may fail — likely the compilation of the helper program md_preprocess — and the build itself will succeed, masking the error.

The problem is that the compilation is triggered by

make -f Makefile.from_md "$target"

which is on the right side of a pipe, thus happens in a sub-shell. make fails but the while loop keeps going. Even if one decides to put an exit in the while loop, it only terminates the sub-shell and not gen.bash. It seems that the only way to inform gen.bash that something went wrong is to output the info so that the main script can perform some action... Getting rid of the pipe may be easier.

As for your problem, look at the beginning of the output generated by make. The pbm branch is not without, hum, problems. It is meant to be a proof of concept and needs some more work.

Chris00 commented 10 years ago

I will push soon a fix to get the whole site compile in the pbm branch. The problem is that many codes requesting to actually read_line() were passed to the toplevel...

Chris00 commented 10 years ago

See d918b2f99dbdc45d50c09410c3dd719a571b9e45

amirmc commented 10 years ago

I'm still having trouble building the page and noticed the following in a number of places in the output.

/bin/sh: ./script/code_top: No such file or directory

Although I can see scripts/code_top.ml there's no executable. Could this be related to my problem?

Chris00 commented 10 years ago

Should be fixed 71c0a2f946a72b2ce0d943002323dce8c900cc02

amirmc commented 10 years ago

Interesting. Now I get no pages being built anywhere (just the *.html.toc files), still no code_top executable (not actually sure that matters) but a new type of error (in many places):

if grep -q '*Table of contents*' "site/learn/tutorials/99problems.md" ; then make -f Makefile.from_md "ocaml.org/learn/tutorials/99problems.html.toc" ; fi
omd -r ocaml=script/ocamltohtml -r tryocaml=script/ocamlapplet.bash -otoc -ts 0 "site/learn/tutorials/99problems.md" -o "ocaml.org/learn/tutorials/99problems.html.toc"
make -f Makefile.from_md "ocaml.org/learn/tutorials/99problems.html.tmp"
cd script &&  && \
  ocamlfind ocamlc -o ../"script/code_top" -package netstring -package compiler-libs.toplevel \
    -linkpkg -annot utils.ml code_types.ml code_top.ml
/bin/sh: -c: line 0: syntax error near unexpected token `&&'
/bin/sh: -c: line 0: `cd script &&  && ocamlfind ocamlc -o ../"script/code_top" -package netstring -package compiler-libs.toplevel   -linkpkg -annot utils.ml code_types.ml code_top.ml'
make[3]: *** [script/code_top] Error 2
make[2]: *** [ocaml.org/learn/tutorials/99problems.html] Error 2

on the plus side, this is by far the fastest I've ever seen make run :)

amirmc commented 10 years ago

Digging a little, I think it may be something to do with line 42 in Makefile.common.

pw374 commented 10 years ago

cd script && && \ is (very) wrong.

Line https://github.com/ocaml/ocaml.org/blob/71c0a2f946a72b2ce0d943002323dce8c900cc02/Makefile.common#L15 is not compatible with line https://github.com/ocaml/ocaml.org/blob/71c0a2f946a72b2ce0d943002323dce8c900cc02/Makefile.common#L42

pw374 commented 10 years ago

I'm going to change line 15 to COMPILER_LIBS_PREPARE=true so that it doesn't fail... (However I have no idea what that's for, so I'm counting on its author...)

amirmc commented 10 years ago

I'm now guessing that line 15 from this commit shouldn't be there. I'm going to comment it out and see what happens.

pw374 commented 10 years ago

I'm going to change line 15 to COMPILER_LIBS_PREPARE=true so that it doesn't fail... (However I have no idea what that's for, so I'm counting on its author...)

Well, no, actually I can't (I thought it was on redesign)

pw374 commented 10 years ago

I'm now guessing that line 15 from this commit shouldn't be there. I'm going to comment it out and see what happens.

No don't comment it out! Replace it as I said in https://github.com/ocaml/ocaml.org/issues/211#issuecomment-28733558

amirmc commented 10 years ago

Ok. Will try that instead.

Chris00 commented 10 years ago

On Mon, 18 Nov 2013 12:13:02 -0800, Philippe Wang wrote:

cd script && && \ is (very) wrong.

Absolutely! Fixed in 837b69ac7e3f9364f31b12dc2661ca73b52aebc9

Chris00 commented 10 years ago

On Mon, 18 Nov 2013 12:14:20 -0800, Philippe Wang wrote:

I'm going to change line 15 to COMPILER_LIBS_PREPARE=true so that it doesn't fail... (However I have no idea what that's for, so I'm counting on this author...)

It is possible to compile the toplevel with OCaml 3.12.1 but with some trick because the interface of the Error module is not available.

Chris00 commented 10 years ago

On Mon, 18 Nov 2013 12:08:51 -0800, Amir Chaudhry wrote:

Interesting. Now I get no pages being built anywhere (just the *.html.toc files), still no code_top executable (not actually sure that matters) but a new type of error (in many places):

if grep -q 'Table of contents' "site/learn/tutorials/99problems.md" ; then make -f Makefile.from_md "ocaml.org/learn/tutorials/99problems.html.toc" ; fi omd -r ocaml=script/ocamltohtml -r tryocaml=script/ocamlapplet.bash -otoc -ts 0 "site/learn/tutorials/99problems.md" -o "ocaml.org/learn/tutorials/99problems.html.toc" make -f Makefile.from_md "ocaml.org/learn/tutorials/99problems.html.tmp" cd script && && \

My mistake, forgot to check what happens for OCaml > 3.12. (One may want to check wether compiler-libs are present though.)

amirmc commented 10 years ago

New problem now. Something to do with Dynlink -- and also a findlib warning (I indeed have topdirs.cmi in both locations mentioned)

if grep -q '*Table of contents*' "site/learn/tutorials/99problems.md" ; then make -f Makefile.from_md "ocaml.org/learn/tutorials/99problems.html.toc" ; fi
omd -r ocaml=script/ocamltohtml -r tryocaml=script/ocamlapplet.bash -otoc -ts 0 "site/learn/tutorials/99problems.md" -o "ocaml.org/learn/tutorials/99problems.html.toc"
make -f Makefile.from_md "ocaml.org/learn/tutorials/99problems.html.tmp"
cd script &&  \
  ocamlfind ocamlc -o ../"script/code_top" -package netstring -package compiler-libs.toplevel \
    -linkpkg -annot utils.ml code_types.ml code_top.ml
findlib: [WARNING] Interface topdirs.cmi occurs in several directories: /Users/amir/.opam/4.01.0/lib/ocaml/compiler-libs, /Users/amir/.opam/4.01.0/lib/ocaml
File "code_top.ml", line 1:
Error: Error while linking code_top.cmo:
Reference to undefined global `Dynlink'
make[3]: *** [script/code_top] Error 2
make[2]: *** [ocaml.org/learn/tutorials/99problems.html] Error 2
Chris00 commented 10 years ago

Oh well, I should have tested it with OCaml 4.01.0. Will do that now.

Chris00 commented 10 years ago

Fixed in 1639c5ddafb085deda313e2b955cc76e58179b5f

amirmc commented 10 years ago

Works! I'll carry on with the blockquoting that you started.

amirmc commented 10 years ago

I think I'm more than halfway. I'll finish off the rest tomorrow and we can discuss how to merge this into redesign

Chris00 commented 10 years ago

On Mon, 18 Nov 2013 17:27:24 -0800, Amir Chaudhry wrote:

I think I'm more than halfway. I'll finish off the rest tomorrow and we can discuss how to merge this into redesign

I think we can just merge it in redesign when we are happy with it. I can do it if you'd like.

amirmc commented 10 years ago

Great! I'll let you know when I'm done.

Chris00 commented 10 years ago

OK. Thanks for your work!

amirmc commented 10 years ago

Still having trouble with rebase (c.f https://github.com/ocaml/ocaml.org/commit/615dd75e903728ce7d900032f8f646ac90739906#commitcomment-4639899) so I kept pushing to my fork. The file is now complete but I'd still like to figure out how to do the rebase properly so I'll try again in a while (I have to cycle to lab now).

Specifically, I'm not clear on how I fix the conflicts as git rebase changes the local file and I can't see my version. I know the patch file is in .git/rebase-apply/patch but that doesn't help me figure out the practical steps I need to take. If I can figure out where my file is stored then I can likely copy/paste relevant sections from my commits back in.

Apologies for the n00b-ness. There's lots about git that I'm still figuring out. :)

Chris00 commented 10 years ago

Please have a look to http://git-scm.com/book/en/Git-Branching-Basic-Branching-and-Merging#Basic-Merge-Conflicts

Git changes the local file to make it contain both versions with special delimiters on the parts that are conflicting:

<<<<<
your version
=====
version you are trying to merge
>>>>>

This is so you can see both in your editor and decide how the gather together the changes.

amirmc commented 10 years ago

Aha! It was mostly stupidity on my part. I wasn't actually looking at the file properly so didn't notice the <<<<< in the file. In any case, I used git mergetool which meant I could pick which edits I wanted to keep from each file. I had to go through this twice before it merged ok.

I've pushed to the main repo and forced a push to my fork. If you can check it over that would be helpful and please feel free to merge into redesign when you're ready.

Chris00 commented 10 years ago

Merge done dc728509cc7abdb5f4c8394be8b79501bfe47fef