Skip to content
This repository was archived by the owner on Apr 3, 2024. It is now read-only.

Conversation

@ashleygwilliams
Copy link
Contributor

@ashleygwilliams ashleygwilliams commented Aug 10, 2016

fixes #224

to do:

  • update tests
  • clean up util

lib/headings.js Outdated
Token = state.Token
savedTokenConstructor = true
}
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

It occurred to me while doing something else that we can eliminate savedTokenConstructor from this file and just do

if (!Token) {
  Token = state.Token
  tokenUtil.set(Token)
}

@ashleygwilliams
Copy link
Contributor Author

I LOVE DELETING CODE

@ashleygwilliams
Copy link
Contributor Author

119 passing (4s)
  33 failing

@revin
Copy link
Collaborator

revin commented Aug 11, 2016

33, wow. I guess that part of the rendering is well covered 😛

@revin
Copy link
Collaborator

revin commented Aug 12, 2016

152 passing

@ashleygwilliams
Copy link
Contributor Author

niiiiiiiice 👯

@ashleygwilliams
Copy link
Contributor Author

thought -> should the SVG icon be a flag? @revin

@revin
Copy link
Collaborator

revin commented Aug 12, 2016

@ashleygwilliams probably, you're right. We have a slightly now misnamed option prefixHeadingIds that enables or disables the 'user-content-' string that gets prepended to the generated <a> ids, but nothing for skipping the heading processing completely.

Want an option that disables the link generation altogether? Or an option that enables it, default true, and you can set it to false manually if you want? Basically we would be allowing folks to completely skip our githubHeadings plugin completely.

Also since this is already semver-major, should we rename prefixHeadingIds to be more accurate? Something like ... prefixHeadingLinkIds ...? I don't know, seems long, and you could argue that conceptually the IDs are for the headings, just not the <h1>, <h2>, etc. elements themselves. I think I just talked myself into leaving that alone.

@revin
Copy link
Collaborator

revin commented Aug 12, 2016

Oh wait, did you mean disabling just the icon itself, but still doing the rest of the processing (so we'd still generate the slug and link, but the link would be empty)?

@ashleygwilliams
Copy link
Contributor Author

hah yes, the second thing, just the SVG part. something like --without-icons

…e/disable headings' generated links' SVG icons
@revin
Copy link
Collaborator

revin commented Aug 12, 2016

I added this as options.enableHeadingLinkIcons for programmatic usage; maybe we could add the CLI part in #132?

@revin revin mentioned this pull request Aug 13, 2016
11 tasks
@revin revin force-pushed the link-headers branch 2 times, most recently from 056d5a2 to bb5690b Compare August 15, 2016 18:53
@revin revin removed the wip label Aug 15, 2016
@revin revin changed the title [WIP] implement github style link headers implement github style link headers Aug 15, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Linking to headers that contain links

2 participants