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 suggestion: Automating wrapping markdown one-sentence-per-line #337

Open
dstansby opened this issue Mar 25, 2024 · 13 comments · Fixed by #356
Open

Feature suggestion: Automating wrapping markdown one-sentence-per-line #337

dstansby opened this issue Mar 25, 2024 · 13 comments · Fixed by #356
Assignees
Labels
enhancement New feature or request

Comments

@dstansby
Copy link
Member

Is Your Feature Request Related to a Problem? Please Describe

Add an automated way to wrap markdown files at 80 (@dstansby: maybe a bit more??) characters.

Suggested by @paddyroddy in #335 (comment).

Describe the Solution You'd Like

An automated way to wrap markdown files in the pre-commit config.

Describe Alternatives You've Considered

No response

Additional Context

No response

@dstansby dstansby added the enhancement New feature or request label Mar 25, 2024
@samcunliffe samcunliffe removed their assignment Mar 25, 2024
@samcunliffe samcunliffe linked a pull request Apr 17, 2024 that will close this issue
@paddyroddy paddyroddy reopened this Apr 25, 2024
@paddyroddy
Copy link
Member

Identified in #361 proseWrap broke things due to prettier/prettier#9232. This might be fixable by a different markdown parser, or not using the RST-like hyperlink format.

@paddyroddy paddyroddy changed the title Feature suggestion: Wrap markdown at 80 characters Feature suggestion: Automating wrapping markdown at 80 characters Apr 25, 2024
@matt-graham
Copy link
Collaborator

matt-graham commented Jun 13, 2024

I'd vote against autowrapping Markdown. From my perspective:

  • It can make minor changes lead to unnecessarily large diffs.
  • It adds extra faff while editing - either you try to manually reflow text to keep to line length limit or have to apply a reformatter after making edits.
  • Markdown unlike source code is generally naturally reflowable by soft wrapping as indentation is (mostly) not used to indicate structure.

@paddyroddy
Copy link
Member

paddyroddy commented Jun 13, 2024

It can make minor changes lead to unnecessarily large diffs.

I actually wholeheartedly disagree with this bit. When markdown is not wrapped and someone changes a few words in a paragraph, it can be very long.

@samcunliffe
Copy link
Member

I wrap markdown for my own stuff. But just because I quite like to cat flibble.md and read it in a terminal.
No strong opinion other than: if we do decide a policy of wrapping it should be linter-ed.

@matt-graham
Copy link
Collaborator

I wrap markdown for my own stuff. But just because I quite like to cat flibble.md and read it in a terminal.

You may already know but in case it's helpful: as an alternative to cat flibble.md, fmt flibble.md will display word wrapped content of flibble.md without touching file.

@dstansby
Copy link
Member Author

dstansby commented Jun 17, 2024

I'm also -1 on wrapping Markdown at a fixed number of characters, and agree with reasons in #337 (comment). Generally I try and do one-sentence-per-line, for reasons outlined here: https://nick.groenen.me/notes/one-sentence-per-line/, which I think addresses #337 (comment) too, so perhaps we could look for a linter that wraps whole sentences, or just leave this as is with no automatic markdown wrapping?

@paddyroddy
Copy link
Member

I'm also -1 on wrapping Markdown at a fixed number of characters, and agree with reasons in #337 (comment). Generally I try and do one-sentence-per-line, for reasons outlined here: nick.groenen.me/notes/one-sentence-per-line, which I think addresses #337 (comment) too, so perhaps we could look for a linter that wraps whole sentences, or just leave this as is with no automatic markdown wrapping?

@p-j-smith found some one-sentence-per-line linter, but it was impractically slow

@matt-graham
Copy link
Collaborator

Generally I try and do one-sentence-per-line, for reasons outlined here: https://nick.groenen.me/notes/one-sentence-per-line/, which I think addresses #337 (comment) too, so perhaps we could look for a linter that wraps whole sentences, or just leave this as is with no automatic markdown wrapping?

One sentence per line seems a good recommendation to me - if nothing else it'll make me notice, and hopefully be more likely to fix, my tendency towards overly long sentences (this one being a case in point 😅).

@dstansby
Copy link
Member Author

Is consensus here that we

  • would like to wrap markdown at one sentence per line, but there's no technical solution we can find at the moment
  • don't want to wrap at a hard character limit

? If so I'll edit the title of the issue and we can keep on the lookout for a one-sentence-per-line linter

@samcunliffe
Copy link
Member

Not sure if it helps us in the quest for one-sentence-per-line but..

Is a thing.

@samcunliffe
Copy link
Member

Was this what Paul found?
https://github.com/JoshuaKGoldberg/sentences-per-line

Looks to be a markdownlint rule that we could test.

@paddyroddy
Copy link
Member

Was this what Paul found? JoshuaKGoldberg/sentences-per-line

Looks to be a markdownlint rule that we could test.

Just trying to find what it was

@paddyroddy
Copy link
Member

Found it! Yes it was that one, see discussion here https://github.com/UCL-MIRSG/MIRSG/issues/191 (screenshot, as you might not have access)
image

I ended up using Prose Wrap from prettier

@paddyroddy paddyroddy moved this to TODO in @paddyroddy Jul 5, 2024
@paddyroddy paddyroddy removed this from @paddyroddy Jul 5, 2024
@dstansby dstansby changed the title Feature suggestion: Automating wrapping markdown at 80 characters Feature suggestion: Automating wrapping markdown one-sentence-per-line Jul 22, 2024
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.

4 participants