search

rust-lang / mdBook

Create book from markdown files. Like Gitbook but implemented in Rust
https://rust-lang.github.io/mdBook/
Mozilla Public License 2.0
17.71k stars 1.61k forks source link

Spaghetti Time! #458

Open Michael-F-Bryan opened 6 years ago

Michael-F-Bryan commented 6 years ago

As mdbook has grown and gained features over time, a couple bits and pieces have suffered from rightward drift or turned into spaghetti code. I'm noting down a list of problem areas which need a little TLC.

This is just standard refactoring and code cleanup, so would be great as a first contribution. If you want to help with the cleanup, make a comment here so we can assign it to you. I'm also available to mentor if needed.

Individual modules which could do with some cleanup:

There is already a PR which runs rustfmt over the repo (#438, cheers @prertik!).

There are also existing PRs for the configuration (#457) and book (#409) modules.

Michael-F-Bryan commented 6 years ago

@azerupi or @budziq, let me know if you can think of other places which could do with a little refactoring/cleanup. Also, would it be a good idea to post this issue to the Call For Participation thread on the user forums?

markcol commented 6 years ago

I am interested in working on these issues. I am new to Rust so I would need some mentoring.

azerupi commented 6 years ago

@markcol sure! A lot of changes have been made since the opening of this issue, so I am not sure if everything listed is still relevant. @Michael-F-Bryan will be able to tell you more, as he spearheaded the efforts in the last months.

Anyways, if you need any assistance, don't hesitate to ping me or @Michael-F-Bryan on this issue or on a work-in-progress PR and we will guide you as best we can. Good luck!

Michael-F-Bryan commented 6 years ago

@markcol, one of the things I've been wanting to do is convert update our command line argument parsing from using raw clap to structopt. That should help tidy up a lot of the messy code to do with arguments in the src/bin/ directory and knock off the two mdbook::bin::* items on that checklist.

I'm planning to go through the HTML renderer and restructure it soon, so it's probably not worth addressing the two mdbook::renderer::html_handlebars::* issues.

markcol commented 6 years ago

@Michael-F-Bryan, mind if I give the clap -> structopt conversion a go?

Michael-F-Bryan commented 6 years ago

Sure thing. If you open a PR and prefix its title with "WIP:" we can review your code as you are going and it'll also give us a place to discuss and ask/answer questions.

mwilbur commented 6 years ago

@Michael-F-Bryan are there any more items along these lines? I'm just learning Rust and am itching to get my hands dirty. However, I'm more of an embedded systems guy than a web / HTML / js guy.

Michael-F-Bryan commented 6 years ago

@mwilbur I think the easiest way to get into things would be to simply skim through the code base and if you see a spot that's messy/hard to understand, try to clean it up a little bit. This helps learn how to understand other people's code and you can see what Rust looks like in a "real" codebase... Plus it helps us by cleaning up some accumulated technical debt :stuck_out_tongue_winking_eye:

I wouldn't be worried about not having much web/HTML experience. If anything, you could almost think of mdbook as a super simple compiler, it:

  1. Reads in a bunch of text files which are laid out on disk following a particular convention ("parsing"), then
  2. Does a bit of processing to the parsed data structure ("analysis"). This might be including text from other files or update a couple links, and finally it will
  3. Run the processed Book through a bunch of "renderers" which will transform it into some other format ("codegen")

Sure one of the renderers spits out HTML based on a couple templates, but that's just an implementation detail and there are other 3rd party renderers which may do other things with the book (e.g. mdbook-linkcheck, [termbook-cli] or mdbook-epub).

non-descriptive commented 4 years ago

Is it okay to rewrite ::helpers::toc in the way Zola do it?

azerupi commented 4 years ago

I think that part of the code hasn't really fundamentally changed since the early beginning. It would definitely benefit from some refactoring.

The code from Zola is really clean, if we can get something close to that it would be a big improvement. If you want to tackle this, I would say go for it! :slightly_smiling_face: