Break link definitions onto multiple lines when needed #3531

j-f1 · 2017-12-19T15:26:42Z

--parser markdown

Input:

[just-url]: https://example.com
[url-with-short-title]: https://example.com "title"
[url-with-long-title]: https://example.com "a long, long title. It's really really long. Here have words."

[long]: https://example.com/a-long-url/another-segment/yet-another-segment/a-really-long-file-name.php.aspx 
[long-with-title]: https://example.com/a-long-url/another-segment/yet-another-segment/a-really-long-file-name.php.aspx "look a title!"

Output:

[just-url]: https://example.com
[url-with-short-title]: https://example.com "title"
[url-with-long-title]: https://example.com "a long, long title. It's really really long. Here have words."
[long]: https://example.com/a-long-url/another-segment/yet-another-segment/a-really-long-file-name.php.aspx
[long-with-title]: https://example.com/a-long-url/another-segment/yet-another-segment/a-really-long-file-name.php.aspx "look a title!"

Output after this PR:
Preview playground link

[just-url]: https://example.com
[url-with-short-title]: https://example.com "title"
[url-with-long-title]:
  https://example.com
  "a long, long title. It's really really long. Here have words."
[long]:
  https://example.com/a-long-url/another-segment/yet-another-segment/a-really-long-file-name.php.aspx
[long-with-title]:
  https://example.com/a-long-url/another-segment/yet-another-segment/a-really-long-file-name.php.aspx
  "look a title!"

ikatyang · 2017-12-19T15:40:20Z

src/printer-markdown.js

+                printUrl(node.url),
+                node.title ? line : null,
+                node.title ? printTitle(node.title) : null
+              ].filter(x => x)


I think it'd be better this way as it's more intuitive:

concat([ line, printUrl(node.url), node.title === null ? "" : concat([line, printTitle(node.title)]) ])

And also we should add a test case for empty title:

[something]: http://prettier.io ""

It seems like the "" can be removed when the title is blank. Markdown (at least CommonMark) doesn’t render links with empty titles differently from links without titles.

It'll change the AST (at least remark's), we shouldn't touch it.

...sorry, I thought node.title was "" in that case, it seems they're all parsed as null. But results are the same anyway.

ikatyang

~~--> #3531 (comment)~~

ikatyang · 2017-12-20T04:27:31Z

src/printer-markdown.js

+            concat([
+              line,
+              printUrl(node.url),
+              node.title === null


azz · 2017-12-24T00:44:09Z

I worry that this will break some renderers (specifically BitBucket Server). Maybe we could make it respect proseWrap: "preserve"?

ikatyang · 2017-12-24T07:06:03Z

from BitBucket Support - Markdown syntax guide:

Bitbucket Server uses Markdown for formatting text, as specified in CommonMark (with a few extensions)

It seems they should render it correctly, but they actually didn't follow the CommonMark Spec. I'm not sure what should we do here since the link definition does not look like a prose.

This reverts commit 2a8d0dd.

j-f1 · 2018-01-01T22:13:14Z

Rebased to fix the merge conflicts.

j-f1 · 2018-01-04T23:23:04Z

👍/👎?

j-f1 · 2018-02-18T20:32:11Z

Should we merge this or should I 🗑 it?

lipis · 2018-02-19T09:02:19Z

Even GitHub is not rendering them correctly (link is not purple).. I would leave them in a single line.. links are links :)

j-f1 · 2018-02-19T11:16:06Z

@lipis GitHub renders them correctly:

just-url url-with-short-title url-with-long-title long long-with-title

lipis · 2018-02-19T12:38:36Z

the result is correct.. but when they are fenced?

Either case I would prefer to not break them or at least respect the proseWrap option

j-f1 · 2018-02-19T13:27:32Z

Respecting proseWrap is a good idea 👍

lipis · 2018-05-11T14:42:17Z

What's the status with this one?

j-f1 · 2018-05-11T15:39:54Z

I’m not sure — should this get merged as-is or should it be modified to respect proseWrap?

lipis · 2018-05-11T15:41:06Z

I would respect the proseWrap.. WDYT?

lipis · 2018-05-11T15:58:30Z

So shall we add the 1.13 tag for a friendly pressure?

j-f1 · 2018-05-11T16:11:27Z

Sure 😀

Allows to select if Markdown definitions should be broken when 'proseWrap' is set to 'always'. Single line reference links helps preventing potential rendering issues.

Allows to select if Markdown definitions should be broken when 'proseWrap' is set to 'always'. This enables, for example, single line reference links, which can prevent potential rendering issues.

j-f1 mentioned this pull request Dec 19, 2017

Conform the quotes in Markdown link titles to the singleQuote option #3481

Merged

ikatyang reviewed Dec 19, 2017

View reviewed changes

ikatyang requested changes Dec 19, 2017

View reviewed changes

ikatyang reviewed Dec 20, 2017

View reviewed changes

src/printer-markdown.js Outdated

concat([

line,

printUrl(node.url),

node.title === null

Copy link

Member

ikatyang Dec 20, 2017

Choose a reason for hiding this comment

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

ikatyang approved these changes Dec 20, 2017

View reviewed changes

j-f1 added 5 commits January 1, 2018 21:47

Break link definitions onto multiple lines when needed

9fe9806

Simplify the conditional for breaking the title

9a1edd1

Don’t print the title if it’s blank

2a8d0dd

Revert "Don’t print the title if it’s blank"

bc26337

This reverts commit 2a8d0dd.

Add another test

f89a823

j-f1 force-pushed the break-link-definitions branch from 9f192cb to f89a823 Compare January 1, 2018 22:09

j-f1 added 5 commits January 3, 2018 11:03

Fix title printing

db8c64b

Second time’s the charm

b0c0425

Third time’s the charm?

97cce5f

Fix snapshot

6a2b2f5

Prettify

49a0320

ikatyang mentioned this pull request Jan 9, 2018

Markdown nested lists are not indented properly #3679

Closed

Merge branch 'master' into break-link-definitions

1599075

josephfrazier approved these changes Feb 18, 2018

View reviewed changes

j-f1 added this to the 1.13 milestone May 11, 2018

j-f1 added 2 commits May 13, 2018 17:30

Don’t break unless proseWrap is always

4726f1b

Test proseWrap: never on link references

d416124

j-f1 requested review from lipis and ikatyang May 13, 2018 22:12

ikatyang approved these changes May 14, 2018

View reviewed changes

lipis approved these changes May 14, 2018

View reviewed changes

j-f1 merged commit dc68a3d into prettier:master May 14, 2018

j-f1 deleted the break-link-definitions branch June 3, 2018 11:10

ikatyang mentioned this pull request Jun 17, 2018

Bug: Markdown reference links need to always be single-line #4709

Closed

lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Sep 1, 2018

lock bot locked as resolved and limited conversation to collaborators Sep 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Break link definitions onto multiple lines when needed #3531

Break link definitions onto multiple lines when needed #3531

j-f1 commented Dec 19, 2017 •

edited

ikatyang Dec 19, 2017 •

edited

j-f1 Dec 19, 2017 •

edited

ikatyang Dec 19, 2017

ikatyang Dec 20, 2017

ikatyang left a comment •

edited

ikatyang Dec 20, 2017

azz commented Dec 24, 2017

ikatyang commented Dec 24, 2017

j-f1 commented Jan 1, 2018

j-f1 commented Jan 4, 2018

j-f1 commented Feb 18, 2018

lipis commented Feb 19, 2018

j-f1 commented Feb 19, 2018

lipis commented Feb 19, 2018

j-f1 commented Feb 19, 2018

lipis commented May 11, 2018

j-f1 commented May 11, 2018

lipis commented May 11, 2018

lipis commented May 11, 2018

j-f1 commented May 11, 2018

Break link definitions onto multiple lines when needed #3531

Break link definitions onto multiple lines when needed #3531

Conversation

j-f1 commented Dec 19, 2017 • edited

ikatyang Dec 19, 2017 • edited

Choose a reason for hiding this comment

j-f1 Dec 19, 2017 • edited

Choose a reason for hiding this comment

ikatyang Dec 19, 2017

Choose a reason for hiding this comment

ikatyang Dec 20, 2017

Choose a reason for hiding this comment

ikatyang left a comment • edited

Choose a reason for hiding this comment

ikatyang Dec 20, 2017

Choose a reason for hiding this comment

azz commented Dec 24, 2017

ikatyang commented Dec 24, 2017

j-f1 commented Jan 1, 2018

j-f1 commented Jan 4, 2018

j-f1 commented Feb 18, 2018

lipis commented Feb 19, 2018

j-f1 commented Feb 19, 2018

lipis commented Feb 19, 2018

j-f1 commented Feb 19, 2018

lipis commented May 11, 2018

j-f1 commented May 11, 2018

lipis commented May 11, 2018

lipis commented May 11, 2018

j-f1 commented May 11, 2018

j-f1 commented Dec 19, 2017 •

edited

ikatyang Dec 19, 2017 •

edited

j-f1 Dec 19, 2017 •

edited

ikatyang left a comment •

edited