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

Allowing config option to set which headers to extract #1111

Closed
wants to merge 2 commits into from
Closed

Allowing config option to set which headers to extract #1111

wants to merge 2 commits into from

Conversation

bretterer
Copy link
Contributor

Summary
This PR resolves #984
This PR replaces #1004

Currently the header parsing is hard coded to ['h2', 'h3']. This code changes that to give the ability to configure this in the themeOptions config.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

@bretterer
Copy link
Contributor Author

This PR is a cleaned up version of the pr at #1004

There was an issue with the rebase I did causing a bunch of extra files being merged in that were not part of the actual PR. Tried to clean them up, but was not able to.

@ulivz

@bretterer
Copy link
Contributor Author

Happy New Year @ulivz

Just circling back around to this PR (and #1004) to see if this is ready to merge in now. Really need to have this to finish up a project

if (siteConfig.markdown && siteConfig.markdown.extractHeaders) {
headersToExtract = siteConfig.markdown.extractHeaders
}
const headers = extractHeaders(content, headersToExtract, markdown)
Copy link
Member

@ulivz ulivz Jan 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember I have said that siteConfig shouldn't be passed to markdown-loader like a giant brick

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I just change this to pass in siteConfig.markdown instead of all of siteConfig?

@bretterer
Copy link
Contributor Author

@ulivz I think this is what you are talking about now

@ulivz ulivz force-pushed the master branch 5 times, most recently from 316e022 to 1284944 Compare January 29, 2019 11:47
@flozero flozero added complexity: medium Medium complexity topic: markdown Relates to VuePress markdown type: feature request Request to add a new feature version: 1.x Relates to version 1 of VuePress labels Sep 5, 2019
@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

We should take care about the search too with that to be sure all is working well

@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

Hello i'm closing this PR as it's now replace by this one #1730 Thank's for your work all !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: medium Medium complexity topic: markdown Relates to VuePress markdown type: feature request Request to add a new feature version: 1.x Relates to version 1 of VuePress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Ability to specify how deep page headers should go
3 participants