Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

fix: remove markdown from content before slice #109

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

maxigimenez
Copy link

Summary

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:
The summary provided can contain a broken markdown.

We faced this issue on https://theheadless.dev when the summary were trimmed based on the configured characters, there were scenarios where the markdow was not valid anymore (please check the image attached).

This PR changes the logic to remove the markdown of the summary first and then slice the content. We introduce the same fix in our config and works as expected.

More info
vuejs/vuepress#2657
checkly/theheadless.dev#84

Comment on lines +140 to +141
removeMd(strippedContent.trim())
.replace(/^#+\s+(.*)/, '')
Copy link
Member

@billyyyyy3320 billyyyyy3320 Oct 31, 2020

Choose a reason for hiding this comment

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

In this case, title will not be removed from summary.

Suggested change
removeMd(strippedContent.trim())
.replace(/^#+\s+(.*)/, '')
removeMd(strippedContent.trim() .replace(/^#+\s+(.*)/, ''))

Copy link
Author

Choose a reason for hiding this comment

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

If the replace is made before removeMd, the content might end up with a broken markdown again. Can you explain why the replace regex is needed?

Copy link
Member

Choose a reason for hiding this comment

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

IMO, It won't end up with a broken markdown. The replace regex is removing title (The first heading 1). We don't need title in summary.

For example, Given below Markdown source:

# Markdown Slot

VuePress implements a content distribution API for Markdown. With this feature, you can split your document into multiple fragments to facilitate flexible composition in the layout component.

Without title
c

With title
b

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late reply, I didn't notice I haven't submit the pending comment

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

Successfully merging this pull request may close these issues.

2 participants