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

Style: Wrapping at sentences instead of lines #75

Closed
dlorenc opened this issue Jun 23, 2021 · 13 comments
Closed

Style: Wrapping at sentences instead of lines #75

dlorenc opened this issue Jun 23, 2021 · 13 comments
Labels
process Issue with the process around SLSA itself

Comments

@dlorenc
Copy link

dlorenc commented Jun 23, 2021

This is definitely a preference thing, but I find markdown easier to work with and diff when we wrap at sentences instead of characters. See the OCI style guide for an example: https://github.com/opencontainers/image-spec#markdown-style

Any opposition to switching to this from the current 80 char convention?

@inferno-chromium
Copy link
Contributor

Sure, seems fine, just need to do at a time when there are no other active edits.

@MarkLodato
Copy link
Member

I've been using mdformat, which is Google internal but avoids formatting arguments altogether. I'd prefer to keep using that as long as possible.

@dlorenc
Copy link
Author

dlorenc commented Jun 23, 2021

I've been using mdformat, which is Google internal but avoids formatting arguments altogether. I'd prefer to keep using that as long as possible.

Is there an OSS equivalent we could point contributors to?

@inferno-chromium
Copy link
Contributor

I've been using mdformat, which is Google internal but avoids formatting arguments altogether. I'd prefer to keep using that as long as possible.

Is there an OSS equivalent we could point contributors to?

there is https://mdformat.readthedocs.io/en/stable/ , might be better or worse, not sure. we can try running on current spec and see if major diff.

@dlorenc
Copy link
Author

dlorenc commented Jun 23, 2021

there is https://mdformat.readthedocs.io/en/stable/ , might be better or worse, not sure. we can try running on current spec and see if major diff.

That one doesn't handle line-wrapping explicitly, for similar reasons to the one I outlined:

https://mdformat.readthedocs.io/en/stable/
https://sembr.org/

@joshuagl
Copy link
Member

I've been using mdformat, which is Google internal but avoids formatting arguments altogether. I'd prefer to keep using that as long as possible.

Is there an OSS equivalent we could point contributors to?

AIUI the desire is to present industry consensus, so a big +1 to using open tools

@MarkLodato
Copy link
Member

Sounds good. I'll take a look at one of the open-source mdformat to see if it works, but if not, we can just go with the OCI style guide.

@MarkLodato
Copy link
Member

MarkLodato commented Jun 29, 2021

I found the open-source mdformat needlessly opinionated, so I set up markdownlint in #83. The only main change is consistent use of bullet character.

It doesn't address line length, but I have no objections to wrapping at sentences. Would we still wrap at 80 columns AND sentences, or just have really long lines?

@06kellyjac
Copy link
Contributor

MD013 is the rule for line length.
Although it's not marked "fixable" so it wont get auto fixed running the linter in fix mode

@MarkLodato
Copy link
Member

Thanks, @06kellyjac. By "address line length," I meant that neither mdlint nor mdformat will fulfill @dlorenc's original request to switch to semantic line breaks. Doing so must be done manually and requires some sort of human-readable guidelines rather than tooling.

Personally I find long lines hard to work with and manual wrapping to be more toil than just running gq in Vim (or mdformat internally within Google). But I can see how people who use other editors without auto-wrapping support may find the opposite. As for diffs, both GitHub and git diff --color-words highlight word diffs so that's not an issue for me, but again I can see how others disagree.

I'm happy to go with whatever the consensus is. Just let me know.

By the way, aaf3cc7 is the result of open-source mdformat in case you're interested.

@joshuagl
Copy link
Member

Personally I find long lines hard to work with and manual wrapping to be more toil than just running gq in Vim (or mdformat internally within Google). But I can see how people who use other editors without auto-wrapping support may find the opposite. As for diffs, both GitHub and git diff --color-words highlight word diffs so that's not an issue for me, but again I can see how others disagree.

Agreed. I personally prefer a column limit too and find long lines hard work.

I'm happy to go with whatever the consensus is. Just let me know.

My main vote would be for something tooling can enforce.

@MarkLodato
Copy link
Member

I think the work that remains is to write some little blurb about the style guidelines.

@MarkLodato
Copy link
Member

Fixed by 3282d70.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issue with the process around SLSA itself
Projects
None yet
Development

No branches or pull requests

5 participants