v2.ocaml.org
v2.ocaml.org copied to clipboard
Bump OMD to 2.0.0~alpha1
This issue is to upgrade the OCaml scripts which use Omd to the latest version of Omd. This issue is not necessarily very straight forward, but would expose someone to a lot of OCaml code, compiler errors and working in harmony with the type checker.
The main change is in the omd abstract syntax tree (AST) -- for example in scripts/code.ml
we have:
match Omd.of_string ("<pre>" ^ p ^ "</pre>") with
| [ Omd.Html_block(_,_,o) ] -> o
| _ -> assert false
But the AST has changed, and Omd.of_string
now returns a doc
type (https://github.com/ocaml/omd/blob/master/src/omd.mli#L66) so this pattern-matching with the latest Omd returns an error.
Error: This pattern matches values of type Omd.block_desc
but a pattern was expected which matches values of type Omd.block
So we need to convert instances like this to the new AST. For most cases this isn't too bad because the new AST simply wraps most things in a block
type which has a block_desc
and some attributes
. The fix for this line becomes:
match Omd.of_string ("<pre>" ^ p ^ "</pre>") with
| Omd.[{ bl_desc = Html_block o; _ }] -> o
| _ -> assert false
To start developing a solution be sure to update your version of omd i.e. opam install omd.2.0.0~alpha1
.
Hi @patricoferris I haven't taken any issues yet to solve, do you think this would be a good start? 😃
HI @CarolVilarino I'm not sure this is a good first issue -- there's quite a lot of OCaml code to change and this repository doesn't have the dune
build system which makes it even more challenging to update. I think you have commented on some other issues so hopefully you have another one?
@patricoferris can I try working on this?
Hi @Srinithyee I've done a little extra digging on this issue, so let me explain some things and then you can see if you still want to work on it.
The latest omd
binary doesn't have the same level of functionality as the old one. In particular, it cannot generate table of contents like we do now (i.e. omd -otoc...
), so until that functionality is brought back we cannot upgrade to omd.2.0.0~alpha1
without rewriting that ourselves, but there are plans to add it back in perhaps (see https://github.com/ocaml/omd/issues/232).
However, we can still work the the omd
library (the parts I was describing above). We can convert to the new AST and make sure that the scripts compile and then leave that as a draft PR ready to be merged if/when the omd binary gets the support we need.
In summary, the work done on this won't be merged probably any time soon but it is still a contribution I believe you can write about in your Outreachy app :)) Let me know what you think, thanks.
Just seen you are working on https://github.com/ocaml/ocaml.org/issues/1492 -- it's best that you focus on that one I think. If anyone else wants to work on this given the above let me know :))
@Goodiec Since you were asking for a more challenging issue, you can take this one! :)
Just seen you are working on #1492 -- it's best that you focus on that one I think. If anyone else wants to work on this given the above let me know :))
@patricoferris Yes, I'd like to work on #1492. Thanks a lot for your clarification on this issue though :))
@Goodiec Since you were asking for a more challenging issue, you can take this one! :)
OK @gs0510, I'll attempt this. Thank you.
@Goodiec Are you still working on this issue? @gs0510 @patricoferris, If @Goodiec confirms she is not working on this Issue, Can I take this up?
@Goodiec Are you still working on this issue? @gs0510 @patricoferris, If @Goodiec confirms she is not working on this Issue, Can I take this up?
Hi @jyotibalodhi, yes I am.
@Goodiec have you been able to make any progress? Do you need any help? Thanks!
@Goodiec have you been able to make any progress? Do you need any help? Thanks!
Hello @gs0510, sorry this has taken longer than anticipated, been a bit sick. Please could you explain this issue better, I'm not very clear on how to go about it. Thank you
No worries @Goodiec. Did you read the first comment on the issue here: https://github.com/ocaml/ocaml.org/issues/1321#issue-844401468 :)
omd
is a library that the code depends on and the code doesn't compile with the newest version of omd
which is 2.0.0~alpha1
. In the comment, Patrick has also mentioned how you can fix one of the instances where the code fails. Can you go through it and let me know what you don't understand? Thanks!
No worries @Goodiec. Did you read the first comment on the issue here: #1321 (comment) :)
omd
is a library that the code depends on and the code doesn't compile with the newest version ofomd
which is2.0.0~alpha1
. In the comment, Patrick has also mentioned how you can fix one of the instances where the code fails. Can you go through it and let me know what you don't understand? Thanks!
Yes, I did. What I gathered from the issue description is that I have to make some syntax changes to match the latest omd library in the affected places in the scripts but I wasn't sure. I will open a pull request on this soon and get validation that I am doing the right thing. Thank you @gs0510.
Yes that sounds about right :) Thanks @Goodiec! :)
Hello @patricoferris, @gs0510 . After installing the new Omd version, I went ahead to make some changes based on the example given above but trying to build the site using make local gives me the error in the image below:
Please advise on this.
Hi @Goodiec, awesome! I've just been talking about this work as the Omd team look to do a new major release. Absolutely no pressure, but just letting you know your work will be super useful !
This is a compiler error, in particular a type error. You have an if a then b else c
statement. The type of a
is a bool
, the types of b
and c
could be anything, but importantly they must be the same. Consider:
print_endline (if true_or_false then "Hello" else "World")
print_endline
takes a string
and prints it, so the clauses of the if statement better both be the same type. In this case the compiler says:
This expression has type 'weak1 list but an expression was expected of type string
Ignoring the 'weak1
part for now, the problem is that the first clause of the this function is returning a string and the second clause is returning a list. They should be the same type.
I would recommend having a look at the old Omd interface and trying to understand how things map to the latest version.
It is also important to work out what each function is trying to do and map that to the new version of Omd. So for this function that is causing the type error, the code is trying to highlight some OCaml code and return the generated HTML which is the Omd.t
which no longer exists, I would suggest having a look at Omd.doc
type https://github.com/ocaml/omd/blob/master/src/omd.mli#L61.
Please be aware that this is not an easy issue at all, migrating to the new AST is not straightforward so please let me know if you encounter any other uncertainties.
Hi @Goodiec, awesome! I've just been talking about this work as the Omd team look to do a new major release. Absolutely no pressure, but just letting you know your work will be super useful !
This is a compiler error, in particular a type error. You have an
if a then b else c
statement. The type ofa
is abool
, the types ofb
andc
could be anything, but importantly they must be the same. Consider:print_endline (if true_or_false then "Hello" else "World")
print_endline
takes astring
and prints it, so the clauses of the if statement better both be the same type. In this case the compiler says:This expression has type 'weak1 list but an expression was expected of type string
Ignoring the
'weak1
part for now, the problem is that the first clause of the this function is returning a string and the second clause is returning a list. They should be the same type.I would recommend having a look at the old Omd interface and trying to understand how things map to the latest version.
It is also important to work out what each function is trying to do and map that to the new version of Omd. So for this function that is causing the type error, the code is trying to highlight some OCaml code and return the generated HTML which is the
Omd.t
which no longer exists, I would suggest having a look atOmd.doc
type https://github.com/ocaml/omd/blob/master/src/omd.mli#L61.Please be aware that this is not an easy issue at all, migrating to the new AST is not straightforward so please let me know if you encounter any other uncertainties.
Thank you @patricoferris, I will go through the resources you provided and let you know if I encounter any difficulty or have further questions.
Hello @patricoferris, I have been getting more familiar with Ocaml to help me better understand the functions I am to make changes to. Please could you provide me with a resource or resources that could help me understand the Omd interfaces better as I am still having blockers and there is no proper documentation on the library, I really want to work on this issue.
As with most OCaml libraries the documentation is sparse. The best bet is to get comfortable with reading .mli
files which describe the interfaces and also using the library. For example, you can open utop -require omd
and start parsing strings to see the generated AST.
utop # Omd.of_string "# Hello World";;
- : Omd.doc =
[{Omd.bl_desc =
Omd.Heading (1, {Omd.il_desc = Omd.Text "Hello World"; il_attributes = []});
bl_attributes = []}]
Unfortunately that's the nature of programming in OCaml, as you get more and more familiar with common practices the .mli
files actually end up being very useful indeed, not a complete replacement for written documentation, but 8/10 they are enough to get going. If you have any specific questions though, feel free to ask them here :))