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

feat: add autoDetectPageTitle parameter #33

Closed
wants to merge 4 commits into from

Conversation

loopingz
Copy link

This feature allows to not define the title and use the first line of the markdowns to read the title

# MyTitle

Test

Will read title as MyTitle

@mipatterson
Copy link
Owner

Hey @loopingz,

This is a cool feature idea! I've only gotten a chance to glance at the PR, but I'll try and take a deeper look this week.

Couple of quick thoughts:

  1. I wonder if it would be possible to use the Marked engine to parse leading h1 titles, regardless of the Markdown styles used. For example, there are several ways to define an h1 and not all of them lead off with # .
  2. Eventually, before merging this to develop we'd need to add unit and integration tests for this feature.

@loopingz
Copy link
Author

Agreed for the unit/integration test, for the marked engine, I never played with it so no idea: maybe we can add that as a futur improvement.

I'll have a look

@loopingz
Copy link
Author

This feature also reopen the door to just specify a folder and recursively read it to define section and page.
I'm still unsure what was your intent between Section and Group.

I would love the plugin to be able to recursively read this:

FolderA/
  Child1.md
  Child2.md
  index.md
  SectionB/
     Child3.md

Map to a menu like this

Pages
  Child1
  Child2
  SectionB
    Child3

With Pages mapping to index.md

The configuration would look like

  "pages": {
    "groups": [
      {
        "title": "Pages",
        "source": "./docs/pages/",
      }
    ],
    "useMarkdownTitle": true
  },

@mipatterson
Copy link
Owner

So, this plugin is actually somewhat of a spiritual successor to my old plugin here: https://github.com/mipatterson/typedoc-plugin-markdown-pages

That plugin did allow for recursive folder traversal to detect pages. It seemed really convenient at first, but didn't seem to scale well in actual use, mostly due to the lack of control over titles based on file name. Perhaps with the feature you've proposed to allow defaulting the title based on the contents of the markdown file, it might be more feasible. I'd be interested to know how you would envision handle scenarios where a title could not be parsed from the markdown comments. Throw an error?

@loopingz
Copy link
Author

I was thinking of falling back to the name, but throwing an error is probably better, maybe let the user decide with a parameter. Usually, I think letting the user decide the behavior is better.

The options I would see is:

 onTitleError: "error" | "skip" | "filenameFallback"

error: throw an error and stop the process
skip: display a warning a skip the whole file
filenameFallback: use the filename without the extension

@mipatterson
Copy link
Owner

I do like the idea of allow users to control the error behavior. Another though- I wonder if it would be better to make the option name more generic. Something like autoDetectPageTitle. The reason I'm thinking about this is because I'm starting the work to support more than just Markdown pages.

My original plugin (typedoc-plugin-markdown-pages) only supported Markdown, but one of the reasons I renamed this one was because I was hoping to add support for other page types, like HTML and Plaintext.

Given some of the other features requests that have been cropping up like #26 and #11, I'm actually going to start this work to add support for at least HTML-based pages.

Obviously, HTML pages won't have # tags to glean the page title from, but HTML pages could try to detect the page title from either a <title> tag or from an <h1> tag. Might be better to make this more generic so that we don't need a breaking change in the future to support this.

@loopingz
Copy link
Author

We can rename the parameter to autoDetectPageTitle as you said we can implement something for HTML pages. Pretty easy to implement.

I did work on a plantuml html viewer, I'll link it to issue #11 .

I'll update the PR to reflect the change of parameter name.

Do you have some documentation on what is Section compares to Group? I'm on Gitter for IM if needed.
I would understand having several Group meaning different folders where to store documentation. Then as one page can have children it should be sufficient to generate the tree structure.

@loopingz loopingz changed the title feat: add useMarkdownTitle parameter feat: add autoDetectPageTitle parameter Nov 24, 2020
@loopingz
Copy link
Author

I did update the PR to reflect the latest discussed changes, do you need anything for merging and new release? It would be a great use for me.
If you add the html feature soon, we can upgrade the feature to reflect further changes.

@loopingz
Copy link
Author

loopingz commented Dec 9, 2020

@mipatterson do you have any updates? or schedule when you will be able to review this?

@loopingz
Copy link
Author

Closing the PR and creating a fork

@loopingz loopingz closed this Jun 25, 2021
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

Successfully merging this pull request may close these issues.

2 participants