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

Why not support a pluggable option.slug function (or options.slugger implementation)? #17

Closed
4 tasks done
agrohs opened this issue Mar 11, 2023 · 9 comments
Closed
4 tasks done
Labels
👯 no/duplicate Déjà vu 👎 phase/no Post cannot or will not be acted on

Comments

@agrohs
Copy link

agrohs commented Mar 11, 2023

Initial checklist

Problem

While github-slugger is one of the most popular (if not THE most popular) slugging functions, there may be the case where someone wants to use an alternate implemenation such asslugify (or even their own custom slug generator).

Solution

Add a new property to the options object to optionally support sending in your own slug function. If not slug option is passed in, default to the current use of github-slugger

Alternatives

I'm certain there are a number of other approaches here (even creating an alternate plugin), but happy to contribute a PR to add in this option!

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Mar 11, 2023
@wooorm
Copy link
Member

wooorm commented Mar 11, 2023

Hi. Did you see the existing issues? https://github.com/rehypejs/rehype-slug/issues?q=is%3Aissue+sort%3Aupdated-desc+is%3Aclosed
Because this has been asked a couple times.
Allowing such an option harm the ecosystem, as it fragments it. In my opinion, markdown needs standardization, not fragmentation.
There are very good alternatives: this plugin is ±5 lines, which you can write too, to suit your exact needs.
Having that option also means end users get two slug libraries, one of which isn’t used. That’s what 4kb minzipped that isn’t used or needed.

@wooorm wooorm closed this as not planned Won't fix, can't repro, duplicate, stale Mar 11, 2023
@github-actions

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Mar 11, 2023

Duplicate of #16

@wooorm wooorm marked this as a duplicate of #16 Mar 11, 2023
@wooorm wooorm added the 👯 no/duplicate Déjà vu label Mar 11, 2023
@github-actions
Copy link

Hi! Thanks for taking the time to contribute!

Because we treat issues as our backlog, we close duplicates to focus our work and not have to touch the same chunk of code for the same reason multiple times. This is also why we may mark something as duplicate that isn’t an exact duplicate but is closely related.

Thanks,
— bb

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Mar 11, 2023
@agrohs agrohs changed the title Why not support a pluggable option.slug function? Why not support a pluggable option.slug function (or options.slugger implementation)? Mar 11, 2023
@agrohs
Copy link
Author

agrohs commented Mar 11, 2023

Yes, I did search the existing issues and there didn't seem to be the same issue. The one linked as a potential duplicate is related but not the same. I certainly get the perspective of the fact that it is a short only 15 line plugin, but with only an extra 3 lines it could be tremendously more powerful.

A quick example of what I was think to submit (and/or fine to make its own plugin, but this seems generic enough to be pretty helpful to many while still being opinionated by default as it currently stands):

/**
 * @typedef {import('hast').Root} Root
 */

/**
 * @typedef {Function} SlugFunction
 *   Custom `slug` implementation (optional).
 * @param  {string} value
 *   String of text to 'slugify'
 * @return {string}
 *   A unique slug string
 */

/**
 * @typedef {Function} ResetFunction
 *   Custom `reset` implementation (optional).
 * @return {void}
 */

/**
 * @typedef SluggerImplementation
 *   Custom `Slugger` implementation (optional).
 * @property {SlugFunction} slug
 *   The function to call for `slug(string)`
 * @property {ResetFunction} [reset]
 *   The function to call for `reset()`
 */

/**
 * @typedef Options
 *   Configuration (optional).
 * @property {string} [prefix='']
 *   Prefix to add in front of `id`s.
 * @property {SluggerImplementation} [slugger]
 *  The `Slugger` implementation to use.
 */

import Slugger from 'github-slugger'
import {hasProperty} from 'hast-util-has-property'
import {headingRank} from 'hast-util-heading-rank'
import {toString} from 'hast-util-to-string'
import {visit} from 'unist-util-visit'

const slugs = new Slugger()

/**
 * Plugin to add `id`s to headings.
 *
 * @type {import('unified').Plugin<[Options?]|Array<void>, Root>}
 */
export default function rehypeSlug(options = {}) {
  const prefix = options.prefix || ''
  const slugger = options.slugger || slugs

  return (tree) => {
    if (slugger.reset) {
      slugger.reset()
    }

    visit(tree, 'element', (node) => {
      if (headingRank(node) && node.properties && !hasProperty(node, 'id')) {
        node.properties.id = prefix + slugs.slug(toString(node))
      }
    })
  }
}

@agrohs
Copy link
Author

agrohs commented Mar 11, 2023

If that looks interesting/helpful still, let me know and happy to put in place test coverage and submit as a PR @wooorm??

@wooorm
Copy link
Member

wooorm commented Mar 11, 2023

No, thanks. I think it’s a bad idea. For reasons explained above, and linked here: #16 (comment)

@agrohs
Copy link
Author

agrohs commented Mar 11, 2023

Code of Conduct

Examples of behavior that contributes to creating a positive environment include:

  • Using welcoming and inclusive language
  • Being respectful of differing viewpoints and experiences
  • Gracefully accepting constructive criticism
  • Focusing on what is best for the community
  • Showing empathy towards other community members

@wooorm
Copy link
Member

wooorm commented Mar 11, 2023

Quoting the CoC to me is a good example of not being respectful of differing viewpoints and experiences.

I have been kind to you, I am not interested in maintaining your code for the rest of my life, and I don’t want to push your code to millions of people. That does not make your code bad. You can use your code.

I’m locking this conversation as it’s done.

@rehypejs rehypejs locked as resolved and limited conversation to collaborators Mar 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
👯 no/duplicate Déjà vu 👎 phase/no Post cannot or will not be acted on
Development

Successfully merging a pull request may close this issue.

2 participants