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

Fix #41 - Add auto_detection option to control hljs' language detection feature #43

Merged
merged 5 commits into from
Sep 18, 2022

Conversation

Phryxia
Copy link
Contributor

@Phryxia Phryxia commented Sep 17, 2022

Related Issue

#41

Changes

  • Add auto_detection option to enable/bypass hljs' language detection
    • When auto_detection is true, it works equivalent to previous version
    • When auto_detection is false, originally matched HTML fragment will be returned without any changes
      • This means there is no class="hljs" at all
    • The default behavior is true to preserve compatibility
  • Add more test cases for added option
  • Fix TypeScript declaration for option as Partial
  • Extract repetitive test input as const
  • Update example and DOCUMENTATION.

For more details, please checkout my self review.

Note

  • I tried to follow all of your code convention, but some of them are ambiguous. Feedback is wanted. @IonicaBizau
  • I couldn't find any blah related proper boilerplates in this repository. But REAMDE says no modification is allowed as it is generated from blah. If I just run blah --readme and this will erase almost everything. We should fix this, or just add manually to README.

t.should("A Showdown extension for highlight the code blocks.", () => {
// Now you can Highlight code blocks
let html = converter.makeHtml(`
const CODEBLOCK_WITH_LANGUAGE = `
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it's repetitively used, I extracted as constant.

\`\`\``

// After requiring the module, use it as extension
const converter = new showdown.Converter({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since converter is not changed, I used const.

Comment on lines +44 to +47
if (!lang && !auto_detection) {
return wholeMatch
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Early return if auto_detection is false and there is no language information. This prevents redundant operations.

auto_detection: boolean
}

declare function showdownHighlight(options?: Partial<ShowdownHighlightOptions>): ShowdownExtension[];
Copy link
Contributor Author

@Phryxia Phryxia Sep 18, 2022

Choose a reason for hiding this comment

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

Since its implementation has default parameter and default destructured value, it should be optional. Also Boolean is not desirable for modern TypeScript declaration. #note

@IonicaBizau IonicaBizau merged commit 700cb4f into Bloggify:master Sep 18, 2022
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