backstage / mkdocs-monorepo-plugin

✚ Build multiple documentation folders in a single Mkdocs. Designed for large codebases.
https://backstage.github.io/mkdocs-monorepo-plugin/
Apache License 2.0
314 stars 75 forks source link

Automatic `nav` generation from a docs directory #35

Closed waltermaldonado closed 3 years ago

waltermaldonado commented 3 years ago

I am a user of this repository and came upon this need in our projects. This feature offers compatibility with the default mkdocs behavior when no nav section is provided but a docs_dir instead in the sub-docs. It will scaffold the navigation based on the files only when no nav section is present on the sub-docs mkdocs.yml file.

Sorry for not writing the tests, but I am on windows here and it got pretty tricky...

Preisschild commented 3 years ago

@waltermaldonado Just tested your MR and it works great!

Just one thing I noticed is that in sub-mkdocs pages the filename is choosen as title instead of the markdown head.

Any idea how to fix this?

waltermaldonado commented 3 years ago

@Preisschild yes, this is a good suggestion! I went with this approach because solved my current problem but it would be really good if we checked if there is a head and use that, and only use the file name if no head is found, what do you think?

To get the title we can just parse the destination file to get it...

Preisschild commented 3 years ago

Instead of parsing it we may could use the mkdocs function that does the nav generation

Preisschild commented 3 years ago

Also, I think we should replace _ & - from the directory names, similary to the way mkdocs does it: https://github.com/mkdocs/mkdocs/blob/dd903d3092a9288f6c1ab2291b0a176a48351611/mkdocs/utils/__init__.py#L342

Preisschild commented 3 years ago

Also, here is the mkdocs function that generates titles: https://github.com/mkdocs/mkdocs/blob/dd903d3092a9288f6c1ab2291b0a176a48351611/mkdocs/structure/pages.py#L132

Preisschild commented 3 years ago

Btw, I opened a PR on your fork with those features: https://github.com/waltermaldonado/mkdocs-monorepo-plugin/pull/1

Preisschild commented 3 years ago

@waltermaldonado I've created E2E tests in your branch: https://github.com/waltermaldonado/mkdocs-monorepo-plugin/pull/2

Preisschild commented 3 years ago

Hi @waltermaldonado.

In case you don't have enough free time to finish the PR up, I added @OrkoHunter 's suggestions and improved a few other things: https://github.com/waltermaldonado/mkdocs-monorepo-plugin/pull/3.

A review (& merge) would be nice.

After thats done, i'll try to fix the conflicts.

Edit: Sorry for tagging a few other folks, fat fingered it.

Preisschild commented 3 years ago

Since I couldn't figure out how to make this myself: @waltermaldonado can you please rebase?

The conflict in __tests__/integration/test.bats is an easy one.

waltermaldonado commented 3 years ago

I think the conflicts are resolved now, please review since I kept all the tests in the merged version.

Preisschild commented 3 years ago

FYI, the merge conflict seems to have deleted a closing } in the test.bats that belongs here: https://github.com/backstage/mkdocs-monorepo-plugin/blob/15058e867edc0f0b5d5c8379c9ba172c3e376494/__tests__/integration/test.bats#L105

Preisschild commented 3 years ago

Just tested it and works great /cc @OrkoHunter ( in case you missed OP's comment )

Preisschild commented 3 years ago

@waltermaldonado @realandersn I applied your suggestions in the PR Author's branch.

https://github.com/waltermaldonado/mkdocs-monorepo-plugin/pull/6

Would be great if we could get this finally merged. Its a great feature!

waltermaldonado commented 3 years ago

@realandersn Thanks for your suggestions, but I believe the changes in the tests files may lead to failures. Regarding the files that may be unneeded, they are in place as an illustration of the proposed feature, so I believe we can keep them.

Preisschild commented 3 years ago

@waltermaldonado all of the tests succeed in my Branch

waltermaldonado commented 3 years ago

@Preisschild can you please merge it with this one?

Preisschild commented 3 years ago

I already did in the PR on your branch.

I also did remove some of the sample files, as realandersn suggested.

waltermaldonado commented 3 years ago

Oh, I'm sorry, I've missed that.