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

Feature Request: Merge Attributes and Footnotes Extensions #474

Closed
2 tasks done
markhalliwell opened this issue May 14, 2020 · 12 comments · Fixed by #489
Closed
2 tasks done

Feature Request: Merge Attributes and Footnotes Extensions #474

markhalliwell opened this issue May 14, 2020 · 12 comments · Fixed by #489
Labels
do not close Issue which won't close due to inactivity enhancement New functionality or behavior implemented Change has been implemented
Milestone

Comments

@markhalliwell
Copy link
Contributor

markhalliwell commented May 14, 2020

Given that this project has now consolidated a ton of these rando extensions into itself, I think the last two remaining popular extensions should follow suit:

x-ref:
rezozero/commonmark-ext-footnotes#7

@markhalliwell markhalliwell added the enhancement New functionality or behavior label May 14, 2020
@colinodell
Copy link
Member

I agree with footnotes, not sure about the attributes extension though. While I definitely see the value in having that available here, I'm also very cautious about adding anything that's not rooted within the official CommonMark or GFM specs or implementations.

@colinodell colinodell added the do not close Issue which won't close due to inactivity label May 14, 2020
@colinodell colinodell added this to the v1.5 milestone May 14, 2020
@markhalliwell
Copy link
Contributor Author

Well, going by that rationale, then the footnotes extension isn't in either... either.

FWIW, attributes has over 88k installs, compared to footnotes which has just over 1.3k installs. It's an extension I use all the time if only to do simple things like assigning IDs to headers.

While I understand the desire to not conflate spec implementations, these are just extensions on top of the spec (which is essentially what GFM is as well)

If anything this would only help get it accepted as part of the official spec faster.

@markhalliwell
Copy link
Contributor Author

markhalliwell commented May 14, 2020

Also, #421 isn't either... edit: but I would argue that attributes is more practical/useful than either footnotes or emojis

@colinodell
Copy link
Member

True, the specs don't have them, but the GFM implementation does have footnotes, and their Markdown pipeline does support emojis. For those reasons, I feel like it's reasonable to assume these are as close to a standard as we have right now, and any future official specification of those will likely be based closely on those implementations.

I'm still open to the idea of attributes, but I'd like to better understand some of the various implementations other Markdown flavors are using and where the CommonMark community might be leaning.

@colinodell
Copy link
Member

Maybe for the attributes, we could provide different "flavors" of them? That way, if an official variant comes out, we don't have to break BC - we just offer the new official flavor?

@markhalliwell
Copy link
Contributor Author

markhalliwell commented May 14, 2020

but the GFM implementation does have footnotes

Still, not part of the spec.

their Markdown pipeline does support emojis

Emoji is not implemented as part of our Markdown stack

lol I'm not trying to be nit-picky here, but... come on. 😝


I'm just saying that attributes has just as much right as ^.

These are extensions, not spec.

I really don't know of any "flavors" of attributes. It's a very basic and straight forward syntax.

There are only two variations that I am aware of:

  1. The original and Kramdown, which use a colon after an opening curly bracket:
    https://python-markdown.github.io/extensions/attr_list/
    https://kramdown.gettalong.org/syntax.html#attribute-list-definitions

  2. Everything else (no colon):
    https://github.com/webuni/commonmark-attributes-extension
    https://michelf.ca/projects/php-markdown/extra/#spe-attr
    https://www.markdownguide.org/extended-syntax/#heading-ids
    https://www.npmjs.com/package/markdown-it-attrs
    https://pandoc.org/MANUAL.html#extension-header_attributes

Subsequently, webuni/commonmark-attributes-extension based its implementation on the originals and even supports the colon, but makes it optional:

You can assign any attribute to a block-level element. Just directly prepend or follow the block with a block inline attribute list. That consists of a left curly brace, optionally followed by a colon, the attribute definitions and a right curly brace:

I really don't see this changing or something "new" coming out. If it even makes its way into the spec, it'll likely be what is already widely used; which is this.

@colinodell
Copy link
Member

@rezozero @hason Would either of you be opposed to us bringing a copy your footnote and attribute extensions into the main league/commonmark library under a BSD-3 license? We'd of course keep your copyright and attribution in the file headers just like we did when we brought the Table extension into this project: https://github.com/thephpleague/commonmark/blob/1.5/src/Extension/Table/TableParser.php#L8

Keeping these most-common extensions within the main package also helps us ensure that the extensions stay compatible with newer versions before they're released - important since we're actively working on upcoming releases for 1.5 and 2.0 🎉

Just let me know :)

@hason
Copy link
Contributor

hason commented May 18, 2020

@colinodell OK

@markhalliwell
Copy link
Contributor Author

markhalliwell commented May 18, 2020

Maybe mentioning @ambroisemaupate directly would be better (actual committer of rezozero/commonmark-ext-footnotes), see #474 (comment)

@ambroisemaupate
Copy link

@colinodell Hello that’s an honor if you would like to bring footnotes extension into core lib. This is OK for @rezozero and for me.

Future enhancements would be added via Github PR into league/commonmark repo ?

@markhalliwell
Copy link
Contributor Author

Future enhancements would be added via Github PR into league/commonmark repo ?

Yes

@colinodell
Copy link
Member

Awesome! Thank you both for being open to this. I'll work on bringing these in over the next weekend or two 😉

colinodell added a commit that referenced this issue May 23, 2020
This extension is based on https://github.com/rezozero/commonmark-ext-footnotes,
imported and relicensed with permission from the maintainer:
#474 (comment)

In addition to importing the functionality, a number of configuration
options were added, as well as some other small tweaks.
colinodell added a commit that referenced this issue May 23, 2020
This extension is based on https://github.com/rezozero/commonmark-ext-footnotes,
imported and relicensed with permission from the maintainer:
#474 (comment)

In addition to importing the functionality, a number of configuration
options were added, as well as some other small tweaks.
colinodell added a commit that referenced this issue May 24, 2020
This extension is based on https://github.com/rezozero/commonmark-ext-footnotes,
imported and relicensed with permission from the maintainer:
#474 (comment)

In addition to importing the functionality, a number of configuration
options were added, as well as some other small tweaks.
colinodell added a commit that referenced this issue May 25, 2020
This extension is based https://github.com/webuni/commonmark-attributes-extension,
imported and relicensed with permission from the maintainer:
#474 (comment)
colinodell added a commit that referenced this issue May 25, 2020
This extension is based https://github.com/webuni/commonmark-attributes-extension,
imported and relicensed with permission from the maintainer:
#474 (comment)
colinodell added a commit that referenced this issue May 26, 2020
This extension is based https://github.com/webuni/commonmark-attributes-extension,
imported and relicensed with permission from the maintainer:
#474 (comment)
colinodell added a commit that referenced this issue May 26, 2020
This extension is based https://github.com/webuni/commonmark-attributes-extension,
imported and relicensed with permission from the maintainer:
#474 (comment)
@close-label close-label bot added the implemented Change has been implemented label May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not close Issue which won't close due to inactivity enhancement New functionality or behavior implemented Change has been implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants