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

Definitions #71

Merged
merged 1 commit into from
Apr 13, 2021
Merged

Definitions #71

merged 1 commit into from
Apr 13, 2021

Conversation

leebyron
Copy link
Owner

@leebyron leebyron commented Apr 11, 2021

This borrows the syntax from a few other markdown extensions for definition lists (<dl>).

Term
: Definition

I've also come up with a variation on that (which as far as I can tell is novel) to allow for a definition within a phrase. That seems surprisingly common in other specs (The whatwg URL spec uses this pattern pervasively - example)

:: Definition of a *term*.

The formatting of these is pretty lightweight. They just get indented and italicized, similar to grammar definitions.

Defined terms can also be reference linked elsewhere by italicizing a case-insensitive string match:

In any paragraph, just reference a *term* to create a definition link.

@leebyron leebyron marked this pull request as draft April 11, 2021 09:39
@leebyron leebyron force-pushed the definitions branch 12 times, most recently from 7022e38 to 0bdf63a Compare April 12, 2021 04:12
@leebyron leebyron marked this pull request as ready for review April 12, 2021 04:23
@leebyron leebyron force-pushed the definitions branch 2 times, most recently from d25c0ca to 27a9c3b Compare April 12, 2021 04:34
@leebyron
Copy link
Owner Author

leebyron commented Apr 12, 2021

@benjie - this is along the lines of what I was referring to in graphql/graphql-spec#846

A full Appendix glossary isn't being auto generated, but they are getting added to the Index at the bottom of the page. My thought is that we could start by using this syntax to ensure terms are getting defined in context throughout the doc, and then determine if an additional Appendix is necessary or if the index links are sufficient

@benjie
Copy link
Contributor

benjie commented Apr 12, 2021

I’m concerned that this seems to rely on a single newline and may have issues with prettier when proseWrap: always is enabled. I have not tested these concerns yet (am on mobile).

I think the plan of defining terms in place and later considering adding a term index is sensible.

@benjie
Copy link
Contributor

benjie commented Apr 12, 2021

Maybe the standalone definitions could use :: as the prefix to make them unambiguous with respect to whitespace.

@leebyron
Copy link
Owner Author

Double line breaks should be significant and not replaced by prettier, right? What are some test cases you're worried about? I can add them in the suite to avoid ambiguity

@leebyron
Copy link
Owner Author

For standalone definition paragraphs, they need to be at the beginning of a block (not directly following another paragraph line)

Prev paragraph

: Definition of *term*

Prettier should never collapse this double newline.

Is the potential failure concern that prettier would collapse this:

Term
: Definition

To this:?

Term : Definition

@leebyron
Copy link
Owner Author

proseWrap: always does in fact collapse the line

I know some other markdown extensions allow line breaks between the term and the definition, e.g.:

Term

: Definition

And perhaps the :: for standalone definition paragraphs would disambiguate

@leebyron
Copy link
Owner Author

Updated to allow empty lines between definitions (kramdown, and others that support this syntax also allow that) and adopting your idea to use :: for paragraph definitions to disambiguate.

@leebyron leebyron force-pushed the definitions branch 5 times, most recently from ca36b73 to d60cb84 Compare April 13, 2021 08:52
@benjie
Copy link
Contributor

benjie commented Apr 13, 2021

Yeah, exactly. Sorry for the lack of detail, you can imagine writing precise whitespace on mobile would be quite a pain! You figured out my meaning, and I think the :: solution is relatively obvious 👍

I've scanned over the test document and the behaviours expressed therein are in line with what I would expect (with the possible exception of :: *just a term*, but honestly I'm on the fence about what to do with that so am fine with the status quo).

If this can wait until the weekend I can hopefully give it a proper test/review, but if you're keen to get it out feel free 👍

@benjie
Copy link
Contributor

benjie commented Apr 13, 2021

Oh! One critical thing: you have A _Term_ can use either \* or \_ italics. for referencing a term, but we'd also need this for defining the term since prettier replaces * with _ when defining italics in general. Might want to add a couple test cases for this.

@leebyron
Copy link
Owner Author

Great call out - that is handled by the parser but there was not a test case - fixed!

I'm considering opening an issue on prettier for the proseWrap: always collapsing definitions. While it's not part of the core commonmark (neither are tables!) it is a syntax used by many extended markdown specs

@leebyron
Copy link
Owner Author

Issue filed: prettier/prettier#10701

@leebyron leebyron merged commit b73333e into main Apr 13, 2021
@leebyron leebyron deleted the definitions branch April 13, 2021 16:07
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.

None yet

2 participants