Spaghetti Time!

[Michael-F-Bryan]

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:

• mdbook::renderer::html_handlebars::helpers::navigation - several 80+ line functions and lots of rightward drift (fixed in Refactor navigation helpers #465)
• mdbook::renderer::html_handlebars::helpers::toc - the entire module is one long 100+ line method (Use template to generate TOC #467)
• mdbook::renderer::html_handlebars::hbs_renderer - several long functions which could be broken up into smaller steps/methods
• mdbook::bin::serve - argument parsing and execution should be broken out into two separate steps
• mdbook::bin::watch - trigger_on_change() is doing a bit too much and should be broken out into smaller functions
• Clippy warnings (Fix last clippy warnings #469)

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]

@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]

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

[azerupi]

@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]

@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]

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

[Michael-F-Bryan]

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]

@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]

@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 😜

---

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]

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

[azerupi]

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! 🙂