The-turing-way: Auto-formatting of our own markdown text

[masonlr]

Summary

https://prettier.io/ is becoming pretty mainstream for applying opinionated formatting to code. We could apply this to the markdown source for the book.

We each will have different stylistic ways of editing markdown: for example,

1. You can make a list using - or * as the bullet
2. You can apply italic using *italic* or _italic_
3. New writers can be confused by `inline` and ```inline```

If we regularly pass the markdown source through a formatter then we get the benefits of:

1. Cleaner git diffs (as formatting changes will be less likely to appear)
2. Reducing confusion for authors new to markdown, as there is only one opinionated way of formatting the text. New authors can learn this style by looking at the existing text as a guiding example.

What needs to be done?

• [ ] Prolific authors would use a prettier.io plugin (there is support for VS Code, Atom, Sublime, Vim, Emacs, etc, etc). Formatting can be applied each time you save a markdown file.
• [ ] Long term we could run files through the prettier CLI as part of continuous integration checks. (Though, I don't see this as vital if we adopt the plugin strategy above.)

Who can help?

• @sgibson91, @KirstieJane

---

Updates

[masonlr]

@sgibson91 , the above could help if future issues similar to https://github.com/alan-turing-institute/the-turing-way/issues/459 keep appearing.

[sgibson91]

Looooooove this idea!! It also takes the stress of a contributor if they're not familiar with markdown.

I'm trying to find some notes the Binder team made on web hooks for running auto-linting. I will be back when I track them down!

[masonlr]

@sgibson91 this comment below in the CONTRIBUTING.md file would no longer be relevant. You can set prettier to handle line wrapping, etc.

Also when writing in Markdown, please start each new sentence on a new line. While this formats in the same way as if the new line wasn't included, it makes the diffs produced during the pull request review easier to read!

[sgibson91]

I 100% endorse this.

[sgibson91]

Hi @masonlr. I found the notes from the Binder meeting where they discussed using Black for auto-formatting of code. I'm sharing this as the discussion revolved around enabling a pre-commit web-hook on the repo, meaning that the auto-formatting was run for every commit every person made to the repo. What are your thoughts on this kind of implementation? A CI check on a PR is also a great implementation!

[masonlr]

(If you're in the python world, highly recommend Black and setting it up to run every time you save a file. Once you get used to the look, it almost acts as an error checking stage prior to you running the code.)

[masonlr]

I'd be hesitant to prevent merges based on syntax, because it might be a complexity barrier to new contributors. In time, as core contributors make edits the auto-formatting would eventually filter through to the text. But, this is totally your call.

Also, there are some finicky things that you may not want auto-formatted. To solve these, you can use

<!-- prettier-ignore -->
Do   not    format   this

and

<!-- prettier-ignore-start -->
<!-- SOMETHING AUTO-GENERATED BY TOOLS - START -->

| MY | AWESOME | AUTO-GENERATED | TABLE |
|-|-|-|-|
| a | b | c | d |

<!-- SOMETHING AUTO-GENERATED BY TOOLS - END -->
<!-- prettier-ignore-end -->

See docs here: https://prettier.io/docs/en/ignore.html

[masonlr]

pre-commit hook is probably okay though.

[sgibson91]

Sorry @masonlr, I think I miscommunicated slightly there. I meant that your original idea of using CI to run the prettier CLI was a good one - but it definitely shouldn't block merges! I just wanted to float the pre-commit hook as an alternative implementation. Whichever method you feel most comfortable with I think will be a great addition! 🙂

[kkmann]

:+1: to non-forcing pre-commit hook (non-binding warning about 'untidy markdown', autofix y/n option?)

[masonlr]

1. A local pre-commit hook would require contributors to have a local prettier installation available (i.e. access to npm or yarn). Though, this adds an additional barrier to getting started. You could follow a local setup along the lines of https://www.brianhan.co/prettier-with-a-pre-commit-hook (which follows the first option of the official docs https://prettier.io/docs/en/precommit.html).

2. CI running prettier would be less of a burden to new contributors, but would be harder to trace, i.e. the autofix y/n option wouldn't be possible -- it would be an all-or-nothing situation.

I would lean towards option 2 to make it easier for new contributors. @sgibson91, @kkmann what do you think? Are there more options that I'm missing here?

[sgibson91]

Let's go with option 2 to make contributing as simple as possible 🙂

[masonlr]

@sgibson91 what about this https://www.prettierci.com/?

[sgibson91]

Ooooh, I like it!

[masonlr]

I feel like this service is broken, at least I can't get it to work...

[masonlr]

This just hangs:

[sgibson91]

In which case, keep it simple. It's only going to be difficult for others to debug 😕

[masonlr]

This might work https://pre-commit.com/

The idea is that you place a .pre-commit-config.yaml file in the top-level of the repo:

- repo: https://github.com/prettier/prettier
  rev: "1.17.1"
  hooks:
    - id: prettier

You then go down this route:

pip install pre-commit
pre-commit install

This will install prettier and all the required node dependencies behind the scenes. Should be cross-platform.

Then whenever you commit locally, you get feedback along the lines off:

prettier.................................................................Passed
[test-branch e22f2ed] Example commit
 1 file changed, 4 insertions(+)
 create mode 100644 .pre-commit-config.yaml

[masonlr]

The system is optional: i.e. new contributors don't need to use it (their commit will go unfiltered), experienced/core contributors can run (once)

pip install pre-commit
pre-commit install

then suddenly all their commits will be auto formatted.

If your local commit is rejected, it applies the formatting in place, then the relevant files show up in Changes not staged for commit: when you run git status.

[masonlr]

This is more in line with what @kkmann was suggesting above in https://github.com/alan-turing-institute/the-turing-way/issues/630#issuecomment-497939506

[sgibson91]

Yeah, I think that's the system the Binder/Jupyter teams want to introduce 🙂

[masonlr]

Ah makes sense now.

[masonlr]

Should we go down this route?

[masonlr]

Changes would be:

# .pre-commit-config.yaml
- repo: https://github.com/prettier/prettier
  rev: "1.17.1"
  hooks:
    - id: prettier

Maybe some additions to .prettierignore

# .prettierignore
CONTRIBUTING.md

[sgibson91]

I like the idea of a non-blocking test in Travis, simply because new contributors may be the ones who need their commits checking since there's a possibility that they'll be unfamiliar with git/GitHub/markdown/... At least if Travis presents a warning, the core team could step in and help out maybe?

[sgibson91]

(Though pre-commit is hella cool!)

[masonlr]

We could do both? (as pre-commit is optional)

[sgibson91]

Sure, I'd prioritise the Travis test though

[masonlr]

For the CI-side, it would be something like:

$ prettier --check book/content/**/*.md || true
Checking formatting...
book/content/Ethical_Decisions/Planning_Research.md
book/content/collaborating_github/1/readme_communication.md
book/content/collaborating_github/2/roadmapping.md
book/content/collaborating_github/3/getting_contributors.md
book/content/collaborating_github/4/checklist_bibliography.md
book/content/collaborating_github/collaborating_github.md
book/content/continuous_integration/continuous_integration.md
book/content/credit/credit.md
book/content/figures/figure_list.md
book/content/introduction/introduction.md
book/content/make/make.md
book/content/open_research/01/opendata.md
book/content/open_research/02/opensourcesoftware.md
book/content/open_research/03/openhardware.md
book/content/open_research/04/openaccess.md
book/content/open_research/06/openscholarship.md
book/content/open_research/07/resources.md
book/content/open_research/open_research.md
book/content/reproducibility/02/whycare.md
book/content/reproducibility/03/definitions.md
book/content/reproducibility/04/resources.md
book/content/reproducibility/reproducibility.md
book/content/reproducible_environments/reproducible_environments.md
book/content/reviewing/01/how_helpful.md
book/content/reviewing/02/best_practice.md
book/content/reviewing/03/typical_workflows.md
book/content/reviewing/04/checklists_bib.md
book/content/reviewing/reviewing.md
book/content/risk_assessment/01/longreadriskassessment.md
book/content/risk_assessment/02/finalsummary.md
book/content/risk_assessment/risk_assessment.md
book/content/testing/testing.md
book/content/version_control/version_control.md
Code style issues found in the above file(s). Forgot to run Prettier?

We'd return a zero exit code through the || so that the ci-tests are non-blocking.

[sgibson91]

LGTM

[masonlr]

Okay @sgibson91, I've added you as a reviewer to https://github.com/alan-turing-institute/the-turing-way/pull/646. Let me know how it looks. Thanks.