rehypejs / rehype-slug

plugin to add `id` attributes to headings
https://unifiedjs.com
MIT License
198 stars 8 forks source link

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

Closed agrohs closed 1 year ago

agrohs commented 1 year ago

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!

wooorm commented 1 year ago

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.

github-actions[bot] commented 1 year ago

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

wooorm commented 1 year ago

Duplicate of #16

github-actions[bot] commented 1 year ago

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

agrohs commented 1 year ago

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 commented 1 year ago

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

wooorm commented 1 year ago

No, thanks. I think it’s a bad idea. For reasons explained above, and linked here: https://github.com/rehypejs/rehype-slug/issues/16#issuecomment-1387261309

agrohs commented 1 year ago

Code of Conduct

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

wooorm commented 1 year ago

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.