-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[for review] multi-language mdbook #200
Conversation
rename with _ translation links css for translation links
Oh neat! I will dig into the technical details of this later. One comment about the language switcher: it always goes back to the index, rather than switching the page that you're on. Is this intended? |
@steveklabnik For now the translation just links to the index page of each translation, |
Onward to glory. At this point it's over to you. No rush, take your time. I think this is now a decent update. I'm sure there will be bugs but I think I used the binary from this branch enough to catch the obvious ones. Conflicts with master resolved, docs updated, tests are passing with the advanced method of ignoring the failing ones. I'd rather describe them in a separate issue then struggling with them here. On linux i686 something is wrong with |
I am going to go through a couple of topics, but since this is a huuuge change, I am not going to go through every detail. Rendererpub trait Renderer {
fn build(&self, project_root: &PathBuf) -> Result<(), Box<Error>>;
fn render(&self, book_project: &MDBook) -> Result<(), Box<Error>>;
} The renderer should not launch the build and construct The user may specify multiple rendering targets in the configuration file: # "outputs" is a table where each entry is the identifier of a renderer
# containing the configuration options for that renderer
[output]
html = { path = "book/" }
pdf = { path = "pdf/mdBook.pdf" }
# OR alternatively
# [output.html]
# path = "book/"
#
# [output.pdf]
# path = "pdf/mdBook.pdf" In that case, mdBook should render to all specified targets when running Also, mdBook should allow to be used programatically. This means that the So, the way I see it, the renderer should take an (immutable?) reference to the I had some vague idea for a hypothetical feature that I may want to implement in the future: Multi-languageFrom your examples, it seems that there is no "primary language", but in my opinion it is better to make a distinction between the primary language and the translations. The primary language will be used as the landing page and translated chapters can then be mapped to the ones of the primary language. [languages]
en = { name = "English", default = true }
fr = { name = "Français" }
# OR alternatively
# [languages.en]
# name = "English"
# default = true
#
# [languages.fr]
# name = "Français" OthersI really like the chapter TOML headers, a lot more than YAML headers. The only thing I would add is a way to escape it (if it's not already supported). Way ForwardThis is a huge change, it touches almost the whole codebase and unfortunately I can't review this as a whole. I have been trying to go through it for the last couple of days but there is just too much. 😕 I don't want to discourage you, because I really want to merge this, but the only way I see this moving forward is if it is split into multiple smaller PRs and merged incrementally. There are a couple of design decisions that I would like to see changed, the two points above are the most important ones. There are also a couple of small changes I would avoid merging and others would require some modifications or improvements. So considering all this, I think it is better to incrementally add the changes by making smaller PRs focused on one aspect that can be more easily reviewed and discussed. It will be more gratifying for both of us to see small parts of this PR get merged one at the time instead of the whoel thing being blocked by a couple of issues. I now have time to work on it too, so let's coordinate our efforts! |
Re: Renderer and MDBook. When the user calls
So you always have this separation between a representation you build from the I did notice that the renderer function was originally attached to the MDBook So I gave up on that and treated the MDBook as purely the representation of the This made it much easier to think about how to implement features. The MDBook is The Renderer trait is separated to build() and render() because it makes things Re: multiple rendering targets. There is no problem there, since the MDBook is just data, you don't have to The config example you gave above can work that way too. Keep in mind you don't The cli command function is the closest to the user's interaction, determine any
This is a benefit, as target formats have different construction procedures. For every target format, it is very specific what data needs to be prepared The renderer's
This is fine. The tests are written like that.
Imagine how the pieces connect. MDBook is just data. The web server has access You don't have to talk to the server until you are saving files, and then you I can recommend these to whet your appetite: Reagent: Minimalistic React for ClojureScript Lambda Days 2015 - Norbert Wójtowicz - ClojureScript + React.js ClojureScript Release - Rich Hickey Interactive programming Flappy Bird in ClojureScript Clojurescript had an enormous creative effect on me, really enjoyed building UI with it, finally I could stop thinking in Javascript. Other ppl like Vue.js, I suppose it's a matter of temperament or something. The only thing you should stay away from is Angular, don't believe the marketing. Re: multi-language The first language in the So the information is parsed, other features can do logic with it. Re: escaping TOML What you mean is a TOML block between If the block has anything else than whitespace before it (i.e. not the first Re: small PRs I don't see how that would work or why is it necessary. You can't move this in a It is backwards compatible as far as CLI users are concerned. The can update and Test whether the user-visible features work as intended, and if that works, then |
Hm, what should we do about this? It's accumulating conflicts, and I can resolve that, but beyond that? |
I would like to try and check it out this coming week. I share @azerupi 's feeling in general, smaller, incremental PRs are better. I haven't actually looked at the diff here yet, so I can't say specifically, but this is just always true for any open source. |
Yes, I realise that small PRs are easier to review, but this changes the main structs in ways that doesn't really offer a step-by-step progression. There is one big refactoring at the beginning and then the additions are fairly incremental. It is perhaps easier to review by scrolling through the files in the source, rather than using the PR diff. |
@azerupi @steveklabnik I added you both as collaborators to the PR's repo, if you see something you wish to change directly, by all means go ahead and commit it, I'm mostly interested in that the features work, and maybe you see better ways of doing it. Remember the PR is in the |
How do you feel about this? Maybe this multilang business is something cool, but not necessarily a goal for mdbook as a tool. I know that in the next few months I won't have time to contribute much to this PR. So how about cancelling this? It is a loose end now with no apparent direction. This little PR allowed me to learn Rust, it was much easier to get into the language with your encouragement. Thank you for the time you spent on feedback and discussion, it kept me motivated. So I don't mind if you'd rather cancel this and approach the problem at another time. It has been an excellent experience for me, and the valuable results are often not the tangible ones. |
I mostly feel still terrible that I have not had the time to truly dig into a PR this big.
multilang is something we need, generally. That is, at least for The Rust Programming Language, we want this feature. It's just hard to review something so big, and it'll probably be a few more weeks before I can really spend the time to do things other than fix small issues in mdbook.
Let's close it for now, then, as that sounds like the best thing to do. When I start to really dig into this problem, I may base it off of this work, we'll see. Sorry that this has dragged on. I'm glad you got some stuff out of it. Open source is tough. ❤️ |
See a demo: Alice's Adventures in Wonderland, in three languages with mdbook. Chapter excerpts for testing. Markdown sources here.
Use the gambhiro/mdbook/multilang branch for compiling or reading the code.
This PR is for reviewing and discussing a large refactoring which involves:
custom.css
file #178 (custom.css)This PR incorporates PR #147 (new book struct), as I started by merging that and working from there.
This also prepares the ground for #88 (ebooks), which after this can be implemented as another renderer.
Does everything still work?
Let's compose a list of user-level features that I can test and debug:
Rust book
Move
src/img
toassets/img
for the images to be copied. Paths in the Markdown source don't have to change.CLI commands
init
SUMMARY.md
is parsed and missing chapter files are created.gitignore
is created on confirmationbuild
assets/_html-template
assets
watch
serve
:3000
test
Single language book
See src/tests/book-minimal
Works as expected as far as I can tell.
Multi-language book
See src/tests/book-wonderland-multilang
Works as expected as far as I can tell.
Features
Renderer
Renderer is a trait expecting two functions,
.build()
and.render()
:The general idea is that a data structure (
MDBook
) is constructed by parsingthe book's files, and this is given to the renderer's
.render()
function whichdoes whatever it needs to do with it to produce its output format.
.build()
is responsible for calling the necessary functions to constructMDBook
, and.render()
is responsible for writing the output based on thatMDBook
.So
MDBook
is not responsible to rendering or writing anything, but it shouldrepresent all the information that the renderer might need to write all its
output files.
MDBook
has a.render_intent
attribute that is a descriptive enum whichinternal functions can inspect if they need to make decisions based on what
output format has been selected.
Static assets
The application's static assets are embedded in the binary from
data/
using includedir.The book's static assets are expected in
assets/
. Everything will be copied to the book's output folder, except for folders which start with underscore (i.e. user might have_sass
or_html-template
and so on).chapter TOML headers
TOML headers can be added at the beginning of a chapter file, these will be parsed into the attributes of the
Chapter
struct. See babel.md.Multi-language books
See src/tests/book-wonderland-multilang
translation cross-linking
Automatic chapter-to-chapter linking is implemented with incrementally trying harder to find a translation, but never refusing to build the book. I.e. something should always happen, whatever the author does, and more and more should happen as they keep working on their content.
Buidling on the ideas described in #5, finding a translation works step by step this way:
chapter.translation_id
string given in the TOML header (see alice/long-tale)chapter.src_path
(see alice/tears)This covers the following scenarios:
If the translator copy-pastes the SUMMARY.md of the original, changing only the titles, translations will be identified by the
.src_path
.If they rename the file names too, so that the URLs also are in the target language, but the TOC keeps the same sectioning structure, translations will be identified by section numbers.
If they change the sectioning structure too, they can insert a
chapter.translation_id
string in the original and the translation. Any string, not necessarily an UUID. This would maintain cross-links when the original changes file name.If nothing else, they can provide the translation link directly in the TOML header. This breaks when the target file changes file name.
It has to be kept in mind that translations are projects on their own, and can even present the same material in a different structure than the original. The original and its translation are likely to be at different stages at various times. In addition, people have different workflows, they don't necessarily work in a one-two-three disciplined way either.
book.toml
The main language is recognized as the first given in the TOML. Otherwise it has to be marked with
is_main_book = true
.The language code will always be the translation key, the language name can be set optionally.
folder layout
Documentation
Structs
Catching up
The last common commit with master was 8a178e3, I will have to catch up with the updates since then.