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

Format markdown with prettier --prose-wrap=always #212

Merged
merged 2 commits into from
Jan 1, 2023

Conversation

bitplane
Copy link
Collaborator

Not being able to read the docs in the console was bugging me, so I've added --prose-wrap=always to the precommit config.

Makes the readme's readable in less, easier to grep, and (IMO) look much nicer in a terminal. Also stops markdownlint from barfing so much in vscode.

@yk
Copy link
Collaborator

yk commented Dec 31, 2022

is this common for markdown files? to me, it seems quite annoying to have sentences split up over multiple lines, because if I want to change one, I need to jump between lines. I usually just rely on my terminal or editor to do proper wrapping. If this is standard, I'm happy to adopt it, but it seems a bit strange.

Copy link
Collaborator

@fozziethebeat fozziethebeat left a comment

Choose a reason for hiding this comment

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

Approved for web and copilot changes.

I'm a fan of line length limits.

@bitplane
Copy link
Collaborator Author

bitplane commented Jan 1, 2023

Yeah it's standard as per markdownlint at least, and is the norm outside of the ML space. Prettify should do the formatting automatically on the pre-commit hook so you don't have to worry about it too much.

I find it nicer to work with, there's something beautiful about having an ASCII art doc as the source code for rich text. diff is easier to read, they work better with grep, they don't deform when you resize your editor or your terminal when you're just paging through them with less, they open easier in simple ad-hoc editors that don't word wrap (because you turned it off because you're glancing at source code with it), or in a web browser when viewed as raw text.

It's also kinda oldskool and feels like a hacker zine, nfo file or license.txt, but that could be a personal bias. The original authors used that format in their examples though:

https://daringfireball.net/projects/markdown/basics.text

Copy link
Collaborator

@yk yk left a comment

Choose a reason for hiding this comment

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

SGTM, Thank you for everyone's inputs :)

Could you resolve the merge conflicts?

Copy link
Collaborator

@andreaskoepf andreaskoepf left a comment

Choose a reason for hiding this comment

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

Needs to be run on recently added READMEs as well..

@bitplane
Copy link
Collaborator Author

bitplane commented Jan 1, 2023

I re-ran it. It might conflict with #239 though, if that goes in first then lemme know and I'll rebase + run again.

Might as well document the rebase process in case it's useful to anyone else in future:

git reset HEAD^1
git checkout -f
git rebase main
python3 -m pre_commit run --all-files
git add .
git commit -m 'run `prettier` with new params'
git push -f

@yk yk merged commit 4841550 into LAION-AI:main Jan 1, 2023
@yk
Copy link
Collaborator

yk commented Jan 1, 2023

I re-ran it. It might conflict with #239 though, if that goes in first then lemme know and I'll rebase + run again.

Might as well document the rebase process in case it's useful to anyone else in future:

git reset HEAD^1
git checkout -f
git rebase main
python3 -m pre_commit run --all-files
git add .
git commit -m 'run `prettier` with new params'
git push -f

could you add this to the main README ?

@bitplane
Copy link
Collaborator Author

bitplane commented Jan 1, 2023

could you add this to the main README ?

Not sure it's that useful outside of this specific issue - all my .md changes were in one commit and I didn't make any changes to their content.

It could be pretty useful added to .pre-commit-config.yaml as comments though, as a warning and solution to against merge conflicts when auto-reformatting lots of files. I'd be tempted to add it as a script, but is dangerous (drop commits, throw away changes, rebase then force push... not the sort of thing you want to run accidentally!)

@bitplane bitplane deleted the prettier-markdown branch January 1, 2023 21:34
bitplane added a commit to bitplane/Open-Assistant that referenced this pull request Jan 1, 2023
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.

4 participants