Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring #90

Closed
7 of 12 tasks
azerupi opened this issue Jan 2, 2016 · 4 comments
Closed
7 of 12 tasks

Refactoring #90

azerupi opened this issue Jan 2, 2016 · 4 comments
Labels
C-refactor Category: Code refactoring M-Discussion Meta: Discussion

Comments

@azerupi
Copy link
Contributor

azerupi commented Jan 2, 2016

Refactoring mdBook

The current implementation has some design flaws that causes some limitations. Implementations of new features are sometimes more complex and scattered than I want them to be and mdBook is not flexible enough.

In this post I will try to order my thoughts, reflect on what I want to change and how I might do that. If you have any ideas / feedback to share, feel free to hop in the discussion!

Goals

  1. Book representation
    Internal representation (data structures) should change to allow for multi-language books.
  2. Renderers
    There should be a generic Renderer trait that can be implemented to provide renderers for all kind of formats: html, pdf, epub, ... The core should be renderer agnostic!
    Multiple renderers should be able to run (in parallel?) to export to multiple formats
  3. TOML config file
    The configuration file should switch to TOML
  4. Improve Error handling
    Some errors are handled well, others aren't at all. I would like to keep a list of the errors that happened so that at the end I can say: 3 errors occurred instead of terminating on the first error. Of course some errors, like not being able to read the configuration, will cause termination. But a small parsing error in one file does not warrant to stop the whole rendering. Not clear how I will implement this.
  5. Plug-ins
    Non-core functionality should be available in "plug-in" form. By plug-in is meant an isolated piece of code that can be enabled / disabled by the user without affecting other functionality. Plug-ins could / should be able to specify the renderer they target.
    e.g. syntax highlighting could be a plug-in
  6. Tests
    A lot more tests should be included, at the moment it is difficult because functions are long and do a lot of things at the same time. Those functions should be split up in functions that do one thing only so that they can be tested more easily. Ouput should be tested in some way too, without being to constraining.
  7. Make mdBook's code base overall simpler
    Shorten long functions, group similar functionality in more general purpose functions. Make the code more idiomatic, remove dead code and add more comments.

Details

Book representation

  • Discussion: come up with a good internal representation
  • Write a decent parser for the SUMMARY.md file

Renderers

  • Discussion: come up with a design to allow multiple renderers

TOML config file

  • Discussion: settle on a good and future proof format
  • Implement TOML configuration
  • Switch from the JSON configuration to TOML

Improve Error handling

  • Discussion: come up with a design to store and report better errors

Plug-ins

Tests

  • ???

Other

@Michael-F-Bryan
Copy link
Contributor

This is a really great project and I've used it to generate a small personal wiki I use to keep track of ideas or personal how-to's. Keep up the good work!

For the improved error handling. Would it be possible to use something like channels and threads? Then you just need to iterate through the pages being compiled, compiling each one individually, and any error message encountered is sent back to the receiver and collected into a list. I imagine it'd look something like this:

let (tx, rx) = channel();

// Iterate through the pages, sending any errors encountered
//  through the channel.
for page in pages.iter() {
    thread::spawn(move|| {
        let result = compile(page);
        if result.is_err() {
            tx.send(result.unwrap_err()).unwrap();
        }
    });
}

let errors: Vec<ErrorMessage> = rx.iter().collect();

It won't help you make the error messages themselves any cleaner, but it'll mean you can catch all the error messages at once instead of stopping at the first one.

@azerupi
Copy link
Contributor Author

azerupi commented Sep 27, 2016

Thanks :)

but it'll mean you can catch all the error messages at once instead of stopping at the first one.

Definitely, I want to be able to continue when encountering non-fatal errors. For that I thought I would just store a vec of the errors in the MDBook struct (I think cargo does something similar). After that you could just check if any errors occurred.

Would it be possible to use something like channels and threads?

That's an intriguing idea, is there any advantage in sending them through channels over the vec approach?

@Michael-F-Bryan
Copy link
Contributor

Probably not. My main thought is that it would sandbox the compilation so that if one thread panics it doesn't crash the entire build. You would then be able to recover or at least gather extra debug information.

You might also get a marginal performance increase because you're use multi-threading, but build speeds aren't really a big issue anyway.

@azerupi azerupi added C-refactor Category: Code refactoring and removed Status: In progress labels May 16, 2017
@azerupi azerupi removed this from the 0.1.0 milestone May 18, 2017
@Dylan-DPC-zz
Copy link

Closing this since it is outdated. if there are changes from this that are still to be made, we can create new issues for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-refactor Category: Code refactoring M-Discussion Meta: Discussion
Projects
None yet
Development

No branches or pull requests

3 participants