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

proseWrap: always should not wrap link title #53

Closed
kevgo opened this issue Aug 29, 2021 · 2 comments · Fixed by #54
Closed

proseWrap: always should not wrap link title #53

kevgo opened this issue Aug 29, 2021 · 2 comments · Fixed by #54
Labels
enhancement New feature or request

Comments

@kevgo
Copy link
Contributor

kevgo commented Aug 29, 2021

Thanks for dprint! I love it and hope it takes over the formatting world and we never have to wait for Prettier again! I would like to propose to never wrap links even if textWrap is set to always.

Details

Prettier never wraps links even if their title is longer than 80 characters. In this example in the Prettier playground the link is only on line 2. Dprint currently wraps links: in the equivalent example in the dprint playground the link ends up on lines 2-3 in the formatted output.

Rationale

Keeping links on a single line makes processing markdown text line by line so much easier, for example grepping with regexes, showing only matching lines in search results, or when syntax highlighting. I'd also argue that keeping the link on one line improves readability of the formatted output since links could be considered a logical unit in the content. It is easier to read Markdown when links stay "together". This change wouldn't affect HTML rendered from the Markdown.

Implementation

If you agree and provide some guidance around how to implement this, I'm happy to take a stab at it.

@dsherret
Copy link
Member

dsherret commented Aug 29, 2021

This seems reasonable given that prettier doesn't wrap (never noticed that... I'll investigate later if this happens for other things). I will run it by a few people.

To implement this, I think we would just need to use the Context to mark "we're in a link now" in parse_inline_link (similar to is_in_list_count on Context) then in get_space_or_newline_base_on_config we could maybe rename that function then have it also take into account whether it's inside a link (so when it's inside a link it would return " " instead of Signal::SpaceOrNewLine).

Also, thanks, I'm glad you like it and thanks for sponsoring 🙂

@dsherret
Copy link
Member

dsherret commented Sep 2, 2021

Discussed this with the deno team today and will do this for reasons outlined in the main post.

@dsherret dsherret added the enhancement New feature or request label Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants