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.

Sign up for GitHub

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

Jump to bottom

Add reference field to LinkReference, ImageReference #23

Closed
wants to merge 3 commits into from
Closed

Add reference field to LinkReference, ImageReference #23

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8453ad9
Update description of Code (#22)
arobase-che Aug 1, 2018
eedeaca
Add reference property to LinkReference and ImageReference interfaces.
rubys Aug 8, 2018
7825abe
make all `identifiers` capture a raw form if it differs from the iden…
rubys Aug 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 21 additions & 5 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,17 @@ Yields:
### `Code`

`Code` ([`Text`][text]) occurs at block level (see
[`InlineCode`][inlinecode] for code spans). `Code` sports a language
tag (when using GitHub Flavoured Markdown fences with a flag, `null`
otherwise).
[`InlineCode`][inlinecode] for code spans). `Code` supports an
info string and a language tag (when the line with the opening fence
contains some text, it is stored as the info string, the first word
following the fence is stored as the language tag, the rest of the
line is stored as the info string, both are null if missing)

```idl
interface Code <: Text {
type: "code";
lang: string | null;
info: string | null;
}
```

Expand All @@ -180,6 +183,7 @@ Yields:
{
"type": "code",
"lang": null,
"info": null,
"value": "foo()"
}
```
Expand Down Expand Up @@ -719,10 +723,16 @@ its `url` and `title` defined somewhere else in the document by a
`referenceType` is needed to detect if a reference was meant as a
reference (`[foo][]`) or just unescaped brackets (`[foo]`).

wooorm marked this conversation as resolved.
Show resolved Hide resolved
`reference` provides the original raw reference, if the `referenceType` is
`full` and the reference differs from the `identifier`. This enables
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to drop the case where referenceType is full? Although it’s not needed to create the original markdown for collapsed or shortcut references in remark, it’s one less condition in the spec?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it could be useful for linting, or non-HTML and non-markdown outputs?

Copy link
Author

@rubys rubys Aug 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Two thoughts:

  1. I don't know how to trade-off the additional memory usage vs the overhead (conceptual and runtime) of a single if-check.

  2. What I do know is that 50+ tests within the remark repository alone will need to be updated should the check for full and different than identifier be dropped. If there is that much change within the repository, chances are high this change will affect somebody's test suite out there.

As to usefulness, this comment was not meant to be an exhaustive list. I did provide a patch for remark-stringify, so this new property is arguably useful in the generation of markdown (i.e., not just the consumption).

Copy link
Member

Choose a reason for hiding this comment

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

  • For the trade-off, I think that’s fine. How many references are out there really.
  • For the test suite: the remarkjs repo has a script: script/regenerate-fixtures that updates the ASTs for all tests, and you can check the diff if it works or not. I personally don’t see the addition of a property to nodes as a breaking change. Even if someones tests break because of fixtures, I don’t think actual (not sure how to call it) code will break.
  • And usefulness: I meant that the addition of a reference to non-full references could potentially be useful for other things, like linting, even though we’re now discussing stringifying to markdown and html, which doesn’t need it.

compilers and transformers to accurately reconstruct the original input
as a fallback for unresolved references.

```idl
interface LinkReference <: Parent {
type: "linkReference";
identifier: string;
reference: string;
referenceType: referenceType;
}
```
Expand All @@ -736,7 +746,7 @@ enum referenceType {
For example, the following markdown:

```md
[alpha][bravo]
[alpha][Bravo]
```

Yields:
Expand All @@ -745,6 +755,7 @@ Yields:
{
"type": "linkReference",
"identifier": "bravo",
"reference": "Bravo",
"referenceType": "full",
"children": [{
"type": "text",
Expand All @@ -763,10 +774,14 @@ its `url` and `title` defined somewhere else in the document by a
reference (`![foo][]`) or just unescaped brackets (`![foo]`).
See [`LinkReference`][linkreference] for the definition of `referenceType`.

`reference` provides the original raw reference.
See [`LinkReference`][linkreference] for the definition of `reference`.

```idl
interface ImageReference <: Node {
type: "imageReference";
identifier: string;
reference: string;
referenceType: referenceType;
alt: string | null;
}
Expand All @@ -775,7 +790,7 @@ interface ImageReference <: Node {
For example, the following markdown:

```md
![alpha][bravo]
![alpha][Bravo]
```

Yields:
Expand All @@ -784,6 +799,7 @@ Yields:
{
"type": "imageReference",
"identifier": "bravo",
"reference": "Bravo",
"referenceType": "full",
"alt": "alpha"
}
Expand Down