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

Current State of the project #1655

Open
YJDoc2 opened this issue Sep 26, 2021 · 15 comments
Open

Current State of the project #1655

YJDoc2 opened this issue Sep 26, 2021 · 15 comments

Comments

@YJDoc2
Copy link
Contributor

YJDoc2 commented Sep 26, 2021

Hey, sorry to open an issue to ask this, but what is the current state of the project?

I was going to open an issue suggesting some added features such as hooks/syntax for inserting images,references and footnotes, and as I was going through the issues, I felt this might be good for plug-in system, in case you might not want to add it in main code.

When looking for plug-in issues, I came across issue #163 , opened in 2016, and which is still not resolved. As I was going through all issues, I saw many which seemed marked and tagged and few which have opened PR, and were still open without comments. Then again, I saw the commit history, and it seems quite active with style/bugfixes/dependency updates.

Now, I might be very wrong and missing things, and there might be changes in plugin system etc happening on other branch or repos which I have not checked, but I would like to ask what is the state of the project? Is there any way I can help with features or discussion of plug-in system?

In case there is need for more contribution or need more reviews and comments on existing PRs, would you be willing to open this for hacktoberfest, so maybe more people can come and contribute?

Thanks :)

@ehuss
Copy link
Contributor

ehuss commented Sep 26, 2021

Thanks for the interest! This was recently asked in #1589. I don't have a lot of time to devote to mdbook, and unfortunately there isn't anyone else helping with maintenance.

If you want to insert content to an existing book, I think regular preprocessor can do that. Although I don't think it helps with additional assets like images (I think #1222 is related to that, and perhaps discussion can go there).

My perception of Hacktoberfest in past years has been pretty poor, so I probably wouldn't be interested in that. Anyone is welcome to help with reviews, though!

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Sep 26, 2021

Hey, thanks for the reply!
Sorry I didn't check the closed issues, so missed the are we looking for more maintainers one 😅 😅

I'll surely take a look at the second issue for the assets, but I had a more of comprehensive idea, which might be better discussed in its own issue, and a separate PR.

That said, there are quite a few issues and PRs already open, so I'd like to first help with them. I understand that you are busy, so I would like to help, as much as I can.
I feel this is a really great project, and as much as I have skimmed through, there are some great/valuable additions for this in issues/PRs. If you won't mind can you suggest what would be the best way to help you maintain this as an OS contributor? should I go around PRs and see if the authors need any help, or if any PRs are ready and test them first myself? Or do you think there is some other things that would be better done first?

Also I took a look at your comment in the maintainers issue, and I am writing my thoughts on it here :
(Sorry in advance for long description)

  • For the CI issue, I think you're right in saying that there are a lot of ways in setting up things, and there is no meaning to add all of them in the docs. I feel best way to tackle this is to mention the CI stuff as wont-fix in readme, and either make a md page in this repo (or link a separate repo) which will contain links to gists, docs blogs and stuff explaining the CI for various providers. This way anyone wanting to add would need to make a gist/doc/stuff of there own and only make a PR updating an md file in this repo (or PR on the other repo) to add their link.
  • For the design extension I agree with you again that allowing many insertion points would make it harder to extend things later. I feel for this, the solution is a bit complicated ,but would also solve a lot of other issues : as mentioned in [Discussion] Plug-ins #163 (comment) , introducing an AST ,even without an IR. This would allow to :
    • allow a plug-in system to be introduced
    • allow people to easily re-structure the document
    • allow people to make passes, which will selectively update stuff they want, and give that as a plug-in, so the whole theme stuff would be opt-in, rather than baked in the mdbook
    • when required to the extend mdbook later, it will be responsibility of plug-ins to make sure they don't break, rather than mdbook having to verify everything that is baked in works after every change
  • For the issues you explicitly mentioned, they seem quite important and need to be solved as quickly as possible, as you rightly mention. I think the problem with these is interconnected with the problems you mention next :
  • As you say, there are lot of issues, and PRs are drive-by nature. My opinion is that resolving issues and PRs quickly is the best way for this, where by resolving I mean either closing with explanation or by making the required change. When PRs or issues stay open for long time, the people that open them might move on, and would be much less inclined to help later, possibly they become busy with other stuff. Quickly resolving might help maintain there interest , and they might keep helping. Now here I don't mean that for the sake of it new things should be merged without checking, but having a quick reply or discussion on them would help contributors to advance in correct direction. Even my reason for opening this issue was that I did not blindly want to make a PR that might stay open for long time, without seeing any changes, so I wanted to ask first.

Now I understand that it is hard to respond to every PR, and you don't have anyone else helping with maintaining, so I would like to ask you to set up a procedure for OS contributor reviewer : something like a checklist to do before calling you in on a PR :
pull and test on their own local system, get at least N people's comments verifying that PR works, and then tagging you in the PR so you would have to spend (hopefully) less time in checking. That way after the initial time where you set this up, you might need to pay less attention, only when others have tested.

I understand your views on hacktoberfest, and that is your decision to make, which I respect.
Alternatively I will suggest, if you would be fine with it, to ask the official rust twitter account to tweet asking for contributors on this project. I feel there are many who would like to help.

Sorry for the long comment 😅
Thanks again for your reply, and let me know what would be the best way to help contribute to maintain this.
Thanks :)

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Sep 27, 2021

Hey, I was going through the code once, and I feel that this might not even need a full-fledged AST, this might just need to use a custom enum to represent the MD elements given by pulldown_cmark. That way a pipeline can be build which processes the custom elements and which finally get converted to pulldown_cmark elements, which can be sent to renderer.

@ehuss
Copy link
Contributor

ehuss commented Oct 3, 2021

Thanks for expressing interest in helping! It is difficult to say how best to help. I don't think it would be possible to make a checklist since most PRs are different in nature. There are basic things like not breaking backwards compatibility, and some things are written down such as browser compatibility.

Just feel free to pick some PRs and review them. I would suggest starting with bug-fix oriented ones (like #1647 or #1617 in nature). Try to test UI changes on a wide variety of browsers, and try to understand what the impact will be.

If you'd like, we could try to enable triagebot to allow anyone to add labels. One thing we could do from there is do some basic things, like labeling if PRs are bug fixes or new features.

@ehuss
Copy link
Contributor

ehuss commented Oct 3, 2021

I forgot to mention, there are some E-help-wanted issues, those might also be places to start if you'd like to dive in.

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Oct 10, 2021

Hey, I took a look at the issues you mentioned, I checked out # 1617 and have left a review there, please check if possible ; but there is no reply from the original author yet. I have also opened PR #1662, which adds a template book containing various tags as well as short snippets of each language mentioned in the mdBook book, which should help in validating style changes quickly and for changes in highlightjs.

I am looking at triagebot, and might make a PR for its TOML file next week, but as per the wiki, you will need to ask on the t-infra of rust zulip to enable it for this repo.

I also wanted to ask about the versioning policy : are you planning on releasing a major version with breaking changes, or currently just planning to add patches and fixes which will maintain compatibility? As there are some issues such as #1666, #1611, #1330 etc. which are fixable only with some changes on how we structure / parse stuff, which would probably be breaking changes

I had also added pulldown-cmark/pulldown-cmark#6 (comment) in pulldown-cmark about adding a dedicated tag that can be used to extend syntax by others, but as the replies say, and I agree, that would not be a good idea to include in pulldown-cmark itself, and make the implementation non-conforming. Is there any way similar extension can be introduced here, so people can use it to extend mdBook, rather than ask to add changes in the main repo? Let me know if there might be a way possible, I might try working on that next.

I am thinking on working on the triagebot , and maybe look at some other issues next, let me know if you want me to work specifically on some issue/PR next.

Thanks :)

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Oct 12, 2021

One of the possible ways to extend this I have thought is to use custom html elements :
we can define something like <c-ext type="" input="" /> for the way I suggested in the pulldown-cmark comment. This is parsed as Html(Borrowed("<c-ext type="" input="" />")) . We can either map this to some custom element, in which case we might need to change from pulldown-cmark enum to custom enum ; or let the extensions/preprocessors manually check html elements and parse them (we can provide a parser function just for this, which should be simple), and then process as required.
This should help solving the LaTex issues as well , such as #1660

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Oct 19, 2021

Ping! @ehuss 😅 😅

@ehuss
Copy link
Contributor

ehuss commented Oct 24, 2021

I also wanted to ask about the versioning policy : are you planning on releasing a major version with breaking changes, or currently just planning to add patches and fixes which will maintain compatibility?

I normally queue up some breaking changes, and then decide to make a breaking change when there is enough to justify it. It is quite disruptive, especially with plugins, so I think it is good to avoid doing it unless there are compelling reasons.

I have been marking breaking changes with the 0.5 milestone, so that when the time comes, those can be merged/addressed.

Is there any way similar extension can be introduced here, so people can use it to extend mdBook

I would prefer to avoid any nonstandard changes that aren't part of pulldown-cmark. If people want to implement extensions via a preprocessor, that is usually the recommended way.

I am thinking on working on the triagebot , and maybe look at some other issues next, let me know if you want me to work specifically on some issue/PR next.

Yea, feel free to post a PR to enable triagebot, and I can look to getting it enabled.

I'll try to find things to help out with. Personally I'm most interested in supporting the rust-lang documentation, and at the moment there isn't anything pressing that I am aware of.

Ping! @ehuss 😅 😅

Sorry, been busy with the recent Rust release and edition work and other projects.

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Oct 24, 2021

Sorry, been busy with the recent Rust release and edition work and other projects.

Yeah, I saw you commenting on some release issues there, no worries!

So actually from the last time I posted this, I have been thinking a little more about the extensions and trying out some things, and found out, that the way I suggested would be quite ugly when actually implemented in code. So I have been thinking on a different way , please let me know your thoughts on it :
currently the parsed markdown is stored in the chapter elements, and after processing and preprocessors, it is rendered , by default using handlebars into html, in the templates. The current way of preprocessors needs converting the book to plain text json, an again parsing it from the preprocessor to make it in the book structure. This adds an overhead which could be removed if we can run the prerpocessing function directly on the original Book struct.

Now the handlebars provides ways to register helpers, which are called on elements in {{...}}. So I was thinking, we always emit the template and style directory into the book src, and always load templates from there, instead of taking default and conditionally overwriting them as specified. Then we can ask users to specify the plugin path which will be in format of wasm, and using wasi , load the preprocessing function into mdBook at runtime. We can then register it as helper to handlebars, and use the {{...}} as hook point.
For example, let us say a user want to do some special latex math stuff, which mathjax isn't working for, or to overcome some issues with a special math handler. The user can specify the plugin which will take in string, and return string and register it under some name, say my_math. Then in the md file, they can write {{my_math some-weired-math-thing}}. We can load the function from the wasm file they specify and register it to handlebars, and which will process it as user needs. This will save the need to convert the chapter to string and back to structure in preprocessor. The issue is this will limit plug-ins to languages that can target wasm, which in my knowledge are only c,c++ and Rust at the moment(?), and also limit the use of them to html based renderers, or specifically handlebars.

The idea needs much more fleshing out, and might not work in the current mdbook architecture at all, but I would like to hear your opinion on it. Thanks!

Yea, feel free to post a PR to enable triagebot, and I can look to getting it enabled.

I'll try to find things to help out with. Personally I'm most interested in supporting the rust-lang documentation, and at the moment there isn't anything pressing that I am aware of.

I'll try to make the PR in the upcoming week, Thanks 👍

@ehuss
Copy link
Contributor

ehuss commented Oct 24, 2021

It seems a bit heavy to require any sort of wasm system for a preprocessor. I'm not sure I fully understand the use case here, though. I don't think the performance for preprocessors has really been an issue, has it? I would imagine if you want to make a new math extension that used syntax like that, you wouldn't really need to even parse markdown. You can just take the raw text, search for your start sigil ({{my_math or whatever) and replace that with the appropriate HTML code to render the math.

I can see that if a preprocessor does need to parse the input, then roundtripping back to markdown is a pain. For that, I could image it would be useful to pass the pre-parsed structure to the preprocessor. However, that would tie mdbook to pulldown-cmark's structure pretty closely. That may not be a big deal, they don't do breaking releases very often, but something to be aware of.

For math specifically, I'm curious what limitations there are with mdbook-katex. I know the built-in mathjax support is somewhat lacking, but does the katex implementation help?

If you'd like to help review, I just posted #1675 which should close about 4 other PRs. I'm not sure if this will be terribly controversial, but it is the route I would prefer to go.

@YJDoc2
Copy link
Contributor Author

YJDoc2 commented Oct 27, 2021

Hey, I'm not sure that the search-and-replace you suggested works with what I had in mind, But I agree that using wasm for preprocessing is a quite heavy requirement, and there has not really been any open issues which states that parsing and re-parsing is a bottleneck. I kind of noticed that there were few open issues asking for a plugin system, and some suggesting a wasm based plug-in system, which made me think of unifying all of them, but that might not necessarily be a good idea, as you have pointed out.

I feel that passing around a parsed structure would be a better choice, rather than passing raw markdown ; as any (external) preprocessor that needs only raw markdown, can easily work as a find-and-replace thing as you suggested on raw files. Preprocessors would be specifically for conditionally/ programmatically transforming input, based on what is the input, for which I think 99% of time one will need parsed markdown. Even though it will tie mdBook to pulldown-cmark, I think it is a good parser, and unless you have any reason why mdBook might move away from it, it would be safe to tie the preprocessors to it as well.

I haven't checked out mdBook-katex much, but from its readme, it seems to generate static pages, which fits with rest of mdBook ( I don't think there is much dynamic hydration of pages generated by mdBook by default, is there?). It also provides support to various math rendering options, and support for latex-style macros. I will try taking a more detailed look later for what limitations or advantages might be for swapping built-in mathjax with katex.

@rimutaka
Copy link

@ehuss , is this project used by rust-lang to generate any kind of docs or does it exist on its own? I wouldn't mind stepping in if this is something Rust ecosystem depends on.

@ehuss
Copy link
Contributor

ehuss commented May 24, 2022

mdBook is used by all the books listed at https://doc.rust-lang.org/. That is its primary purpose.

Help is always appreciated. I've been trying to fix the overlapping button issue in #1322 (comment). If you have any design experience, I'd like some feedback on the design. Or if you have access to android, that is a platform I cannot test on.

@rimutaka
Copy link

@ehuss , it looks OK and works as intended on my good old Samsung S7.
Screenshot_20220526-102714_Chrome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants