Skip to content
This repository was archived by the owner on Nov 6, 2023. It is now read-only.

Bring docs into line with Markdown style guide - #14423

Merged
jeremyn merged 5 commits into
masterfrom
unknown repository
Feb 13, 2018
Merged

Bring docs into line with Markdown style guide#14423
jeremyn merged 5 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Jan 25, 2018

Copy link
Copy Markdown

per tradition. More importantly, this gives improved accessibility for
contributors with small screens or large fonts.

It also improves consistency of appearance for contributors, because
while all(?) text editors will show hard line breaks in the same places
as each other, there is no telling where contributor X's text editor
might place a soft break when automatically wrapping a long line.

If anyone reading this is concerned that hard line breaks will decrease
the comprehensibility of "diffs", they are advised to try out the
--word-diff option to git diff.

per tradition. More importantly, this gives improved accessibility for
contributors with small screens or large fonts.

It also improves consistency of appearance for contributors, because
while all(?) text editors will show hard line breaks in the same places
as each other, there is no telling where contributor X's text editor
might place a soft break when automatically wrapping a long line.

If anyone reading this is concerned that hard line breaks will decrease
the comprehensibility of "diffs", they are advised to try out the
`--word-diff` option to `git diff`.
@jeremyn

jeremyn commented Jan 25, 2018

Copy link
Copy Markdown
Contributor

I specifically prefer things the way they are for text/Markdown files, without hard line wraps. With a long line, people can soft wrap (or not) however they want, but with hard wraps, they have to deal with what's included.

@J0WI J0WI added the docs label Jan 26, 2018
@ghost

ghost commented Jan 26, 2018

Copy link
Copy Markdown
Author

With a long line, people can soft wrap (or not) however they want,

This is not quite true. A long line, soft-wrapped or not, is not the same thing as a hard wrapped paragraph. It is different at the byte level. It is also different to interact with. Editor macros such as "jump to end of line" will jump to the end of the paragraph instead unless re-configured. Adjusting the font size might cause the text to re-flow in the editor. Etc.

My point here is not to say that my preference is necessarily more valid; it is to say that there is a hidden assumption in your statement above (i.e. that soft wrapping is necessarily at least as accessible/convenient as hard wrapping), and that this assumption seems to be false :(

but with hard wraps, they have to deal with what's included.

Yes, but this is arguably a benefit rather than a disadvantage, for all the reasons I gave in the pull request :)

Also, Git famously has conventions around line length and hard-wrapping, so the chances are fairly good that people editing files from a Git repository are used to hard-wrapped files and are equipped to edit them.

@jeremyn

jeremyn commented Jan 27, 2018

Copy link
Copy Markdown
Contributor

I disagree with your point that hard line wraps are more accessible to people with small screens/large fonts. Most text on computers is soft wrapped, on webpages or word processors. Reading software is built around that and people cope well enough. I don't see that hard line wraps at some arbitrary length based on the length of old punch cards -- https://softwareengineering.stackexchange.com/a/148678 -- are particularly better for people with small screens/large fonts, and on the other hand it introduces unusual formatting into the text.

As a test, try opening up both

https://raw.githubusercontent.com/sampablokuper/https-everywhere/b3ea87a69dd094cfde2fd8d23380ad0eaa8e2962/docs/en_US/development.md

and

https://raw.githubusercontent.com/EFForg/https-everywhere/0c7b699d0ca1bed52737c6b0b0df2c3b5b75773a/docs/en_US/development.md

in a web browser, make the font large and the browser window narrow, and see which one is more readable to you.

(I grant that are some edge cases where long lines are handled badly, such as GitHub's rendered code blocks that require side scrolling:

- **Source code:** Available via Git with `git clone https://github.com/EFForg/https-everywhere.git`. You can fork and open pull requests using Github at [https://github.com/EFForg/https-everywhere](https://github.com/EFForg/https-everywhere).

but I don't think we need to write our raw Markdown with these situations in mind.)

I assume git's conventions are intended to encourage people to write briefly in the first line, to encourage people to make (programmatically) small commits, and to support its original use case of Linux kernel development, much of which happens over email by the sort of developers who insist on using mutt or other ancient email clients that enforce line lengths.

@ghost

ghost commented Jan 27, 2018

Copy link
Copy Markdown
Author

Most text on computers is soft wrapped, on webpages or word processors. [...]

As a test, try opening up both

https://raw.githubusercontent.com/sampablokuper/https-everywhere/b3ea87a69dd094cfde2fd8d23380ad0eaa8e2962/docs/en_US/development.md

and

https://raw.githubusercontent.com/EFForg/https-everywhere/0c7b699d0ca1bed52737c6b0b0df2c3b5b75773a/docs/en_US/development.md

in a web browser

(My emphasis.)

Apples and oranges. You are talking about web browsers and word processors; I am talking about text editors ;)

The files affected by this PR are not rendered webpages for viewing in web browser software, nor rich text documents for editing in word processor software.

They are plain text Markdown files for editing in text editor software, and I hope it does not seem unreasonable for me to ask that they be treated as such.

@jeremyn

jeremyn commented Jan 27, 2018

Copy link
Copy Markdown
Contributor

Modern text editors can soft wrap files if their user wants, as I'm sure you already know.

You were making a point about accessibility and I'm saying that people who have accessibility requirements have already solved this problem for themselves in whatever way is convenient for them, when they are using web browsers, word processors, text editors, whatever. They do not need for us to hard wrap our files for them to some arbitrary length.

@ghost

ghost commented Jan 28, 2018

Copy link
Copy Markdown
Author

I'm saying that people who have accessibility requirements have already solved this problem for themselves in whatever way is convenient for them

That is a very sweeping statement. Who appointed you their spokesperson?

More seriously, it is not a true statement. I know this because I am a counter-example to it (and I doubt I am the only person who is).

I have accessibility requirements, and I have not solved the problem of editing soft-wrapped plain text files. I find them deeply unergonomic to work with. To make plain text files accessible and ergonomic for me, I hard-wrap them. (Short plain text files/fragments, like these comments, I grit my teeth through. Longer ones, I mostly ... avoid.)

Fortunately, lots of other people hard-wrap as well, not because they are working with punch cards, but because they find it ergonomic, as I do, and/or because it is specified by their style guide.

If you personally loathe hard-wrapping in plain text files, fair enough; if you have found accommodations for soft-wrapping in plain text files that work for you, great; but please do not assume that anyone else necessarily shares those preferences or adaptations.

I would be grateful if this pull request would be honoured. Thank you.

@jeremyn

jeremyn commented Jan 28, 2018

Copy link
Copy Markdown
Contributor

I think we are talking past one another. I'm not disagreeing with you about ergonomics. I agree that reasonably short lines are more comfortable to read for people without special requirements.

If we are talking specifically about accessibility, as in, accommodating people with various disabilities, then I don't, for example, understand how hard wrapped lines at 80 characters would benefit someone with low vision who is relying on screen reader software. Maybe it would actually get in their way, both in their screen reader processing the text and in making sure their own contributions meet the 80-character limit. And, there are other sorts of accessibility situations, such as people with limited mobility, who may have unrelated or even opposed requirements.

I don't like making particular changes to support accessibility that are actually based on one specific person's situation. Maybe the way to frame this issue is that we should adopt the Markdown style from some organization that has, presumably, thought about this sort of thing carefully. Google has a Markdown styleguide here which, among other things, includes line limits.

What do you think? If you agree with this approach, then we can open a discussion issue to see what other people think. There may be widespread agreement, or maybe some other user will argue against it based on their own accessibility needs.

@jeremyn

jeremyn commented Jan 28, 2018

Copy link
Copy Markdown
Contributor

More specifically, I can open an issue proposing we use the Google styleguide for our Markdown documents. After a week or so, if the EFF agrees and no one in the community complains, I can come back here and probably merge this PR as-is. It should not be much of a hassle to you.

@ghost

ghost commented Jan 29, 2018

Copy link
Copy Markdown
Author

Thanks; this is much fairer :) I completely agree that adopting a style guide would be the best way to handle this. Then concerns or suggestions can, at least, be filed against the style guide rather than against individual files. This should keep discussion on topic and make it easier for others to contribute.

At a glance, Google's Markdown style guide looks broadly reasonable. I think it would be a good starting point. I wouldn't necessarily adopt it wholesale.

For instance, it does have at least one stipulation that I balked at: "Wherever possible, shorten your links." I think the EFF will agree ([1], [2]) that URL shorteners threaten privacy and should be treated with caution.

Also, it only explicitly mentions 80 character wrapping in the "Links" section, which is a bit odd.

If you open a discussion, as you proposed above, then hopefully these and any other issues with that style guide can be addressed, and the EFF will then end up with its own Markdown style guide, which will be a close derivative of the Google one but better organised and more privacy-conscious.

Thanks again for this more thoughtful and inclusive approach :)

@jeremyn

jeremyn commented Jan 29, 2018

Copy link
Copy Markdown
Contributor

I made issue #14467 for this and addressed your specific concerns in #14467 (comment).

@jeremyn jeremyn self-assigned this Jan 29, 2018
@jeremyn

jeremyn commented Feb 5, 2018

Copy link
Copy Markdown
Contributor

It's been about a week since I created #14467, and @Hainish is on-board with the changes and no one is opposed, so let's move forward with this.

Because the changes are extensive, this may require multiple rounds of reviews. Basically my plan is to wrap the same files myself, using Vim ( https://stackoverflow.com/q/823754 ), and then diff my results and yours. The files should be the same.

In docs/en_US/development.md as of b3ea87a, the indentation of bullet point items is not consistent. For example, you have this:

- **Translations:** If you would like to help translate HTTPS Everywhere into
  another language, you can do that [through
Transifex](https://www.transifex.com/otf/torproject/).

but you should have this (however see #14423 (comment)):

-   **Translations:** If you would like to help translate HTTPS Everywhere into   
    another language, you can do that [through                                    
    Transifex](https://www.transifex.com/otf/torproject/).

Notice the indentation on the third line. Please fix this here and anywhere else it appears in your changes, and add a comment when that's done.

@jeremyn

jeremyn commented Feb 5, 2018

Copy link
Copy Markdown
Contributor

Note also that the Google styleguide prefers 4-space indentation for bulleted lists (style.md#nested-list-spacing). I've corrected my previous comment to show this.

@jeremyn

jeremyn commented Feb 5, 2018

Copy link
Copy Markdown
Contributor

You don't need to fix all lists to use 4-space indentation, since that's not the purpose of this pull request. 2-space indentation is fine for now, as long as the indentation is consistent for each line in the item, and for every item in the same file.

@ghost

ghost commented Feb 10, 2018

Copy link
Copy Markdown
Author

@jeremyn wrote:

In docs/en_US/development.md as of b3ea87a, the indentation of bullet point items is not consistent.

Good catch, thanks. The odd indentation was due to a Vim (mis)configuration that seems not to be unique to me. With any luck, most contributors will have their text editors configured in a way that doesn't cause this.

You don't need to fix all lists to use 4-space indentation, since that's not the purpose of this pull request. 2-space indentation is fine for now [...]

In for a penny... :) I have now made the lists consistent with the style guide.

@jeremyn

jeremyn commented Feb 10, 2018

Copy link
Copy Markdown
Contributor

On re-review:

You changed some of bullet markers from - to *. Please keep the original -. Never mind this, it looks like the style guide prefers *.

You missed some 2-space indents, particularly in docs/en_US/rulesets.md#testing and docs/en_US/development.md#testing-and-contributing-changes-to-the-source-code.

@ghost

ghost commented Feb 12, 2018

Copy link
Copy Markdown
Author

Argh, my bad. I did it by hand, in haste, instead of automating it, that's why. (More haste, less speed...) Thanks again for your vigilance.

Have now added the Spanish FAQ to this PR, too.

To whoever merges this: please squash the commits when you do so (or ask me to squash them first). The merge commit should probably say something like, "Bring docs into line with Markdown style guide." I've left them unsquashed for now, for ease of review.

Thanks again.

@jeremyn

jeremyn commented Feb 12, 2018

Copy link
Copy Markdown
Contributor

There are still some remaining small issues as of b73ea67. Please only fix these things, without making any other changes:

docs/README.md:

This is okay.

docs/en_US/development.md:

Fix the irregular indentation of the two bullet items under "Once you've tested your changes, ..."

docs/en_US/faq.md:

This is okay.

docs/en_US/rulesets.md:

Fix the irregular indentation of the two bullet items under "You can test rulesets in the browser using a hidden debugging page, ...".

docs/es/faq.md:

Wrap the line that begins "Solía ser caro comprar un certificado ..."


We regularly squash-and-merge commits for a PR. The GitHub default is to use the first commit message and append the pull request number to it, so for example it would currently be: "Wrap docs at 80 characters (#14423)". I can change it to something else, but we should keep the PR number. Please let me know exactly what you want the new commit message to be, except for the number, which I will add anyway. (For example based on your previous comment, it would be "Bring docs into line with Markdown style guide. (#14423)".)

@ghost ghost mentioned this pull request Feb 12, 2018
@ghost

ghost commented Feb 12, 2018

Copy link
Copy Markdown
Author

@jeremyn wrote:

For example based on your previous comment, it would be "Bring docs into line with Markdown style guide. (#14423)".

I think that would be ideal, given that this PR does more than just wrap lines.

Happy to change the title of the PR accordingly, if that's OK with you?

@jeremyn

jeremyn commented Feb 12, 2018

Copy link
Copy Markdown
Contributor

Your changes as of 2e0d7af are ready to merge.

You can change the PR title if you like.

Please confirm you want me to squash-and-merge the commits with this summary:

Bring docs into line with Markdown style guide (#14423)

(This removes the trailing period, which you almost certainly didn't intend in #14423 (comment), but I'm just making sure.)

@ghost ghost changed the title Wrap docs at 80 characters Bring docs into line with Markdown style guide Feb 12, 2018
@ghost

ghost commented Feb 12, 2018

Copy link
Copy Markdown
Author

Confirmed. Thanks again!

@jeremyn
jeremyn merged commit 2808747 into EFForg:master Feb 13, 2018
@jeremyn

jeremyn commented Feb 13, 2018

Copy link
Copy Markdown
Contributor

Thanks, merged.

(And actually it seems that the default commit message is the title of the PR, and not of the first commit, though normally they are the same.)

@jeremyn jeremyn removed their assignment Feb 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants