From eeee488dde41d09459bba42ac73dd9d1febb2ff3 Mon Sep 17 00:00:00 2001 From: Michael Wolf Date: Sat, 30 Sep 2023 20:38:00 -0600 Subject: [PATCH] Add/update a lot of documentation --- CUSTOMIZATION.md | 206 +++++++++++++++++++++++++++++++++++++++++++++++ DEVELOPMENT.md | 66 +++++++++++++++ README.md | 65 ++++++++++----- examples/php | 36 +++++++++ 4 files changed, 353 insertions(+), 20 deletions(-) create mode 100644 CUSTOMIZATION.md create mode 100644 DEVELOPMENT.md create mode 100644 examples/php diff --git a/CUSTOMIZATION.md b/CUSTOMIZATION.md new file mode 100644 index 0000000..ee800ee --- /dev/null +++ b/CUSTOMIZATION.md @@ -0,0 +1,206 @@ +There are two main areas of per-project customization: how `would-format` +determines a given file's type, and how a file is checked or reformatted +after its type has been determined. + +# Conventions + +`$YOUR_PROJECT_ROOT` is the root where your project is checked out. + +# File type sniffing + +To change a file's mapping, create a script at +`$YOUR_PROJECT_ROOT/.would-reformat/custom-sniffer`. It should take one +argument, the fully qualified path of the file whose type is to be sniffed. +It should emit the type of file it thinks its argument is. + +Here's an example: + +```shell-script +#!/usr/bin/env bash + +file="$1" + +if [[ $file == *.pl ]] ; then + # the default is that .pl corresponds to perl, but not here! + echo -n "prolog" +fi + +if [[ $file == *.ts ]] ; then + # qt translations, who knew + echo -n "qt-translation" +fi + +# We're fine with the defaults for other types of files so we don't +# print anything else. + +exit 0 +``` + +(This file, possibly with updates, is also available +[here](./examples/custom-sniffer).) + +Now, if you run this script against a file that ends in `.pl` or `.ts`, +`would-reformat` will not treat the file as perl or typescript as usual, but +instead as prolog or a qt-translation file. But if you run this script +against a file that ends in `.py` then it'll continue to be treated as python +as usual. + +You can perform arbitrarily sophisticated checks here. They're not limited to +file names or extensions. You can take paths into account. You can can even +take files' contents into account. But do keep in mind that this program +will be run often. If it's slow you'll be frustrated by it. + +Don't forget that this program can be written in something other than shell, +in case that's easier for you or if performance is a serious concern. It +doesn't actually need to be a "script". + +Finally, keep in mind that if `would-reformat`'s defaults work for you, there +is no need to have this file at all. + +## Acceptable output + + +In general, you would do well to limit it to `/[0-9a-z]+/`. It must not +include the `/` character. `would-reformat` doesn't care if you use +uppercase, but some case-preserving filesystems make things difficult. + +It must not output `custom-sniffer`. If there is a hitherto unknown +programming language named "custom sniffer" you'll have to output something else. + +## Return values + +`would-reformat` expects `custom-sniffer` to return one of the following +values: + +- 0: success +- 254: file can't be read due to permissions, etc ("etc" is as of + 2023-Q3 broad in scope; it could include trying and failing to read a file + on a NFS mount) +- 255: some sort of internal error + +# Custom reformatter wrappers + +Your custom wrappers live in `$YOUR_PROJECT_ROOT/.would-reformat`. Its name +should be the file type as identified by the sniffer. + +It takes one command line argument, the fully qualified path of the file to be +checked or reformatted. It also receives input via three environment +variables: + +- `WOULD_REFORMAT` can be set to either `would_reformat` or `do_reformat`. If + the former, it should run in non-destructive mode. If the latter, it should + perform changes on file whose path was passed as the main argument. +- `PROJECT_ROOT` has the value of `$YOUR_PROJECT_ROOT`. +- `WF_ROOT` holds the path where `would-format` is checked out. + +Often, these scripts won't need to check `$PROJECT_ROOT` or `$WF_ROOT`. +`$WOULD_REFORMAT` is indepensable. + +Here is an example: + +```shell-script +#!/usr/bin/env bash + +set -euo pipefail +IFS=$'\n\t' + +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +## This usually isn't necessary, but if you need to use wrflog this +## is how you get it: +# source "$WF_ROOT/_reformat-common.bash" + + +if [[ "$WOULD_REFORMAT" = "would_reformat" ]] ; then + set +e + out=$(php $DIR/.././vendor/bin/pint --test "$1") + retval="$?" + set -e + + # unfortunately, pint doesn't distinguish between files with + # syntax errors and files that are merely misformatted + + echo "$out" + exit "$retval" +fi + +if [[ "$WOULD_REFORMAT" = "do_reformat" ]] ; then + set +e + out=$(php $DIR/.././vendor/bin/pint "$1") + retval="$?" + set -e + + echo "$out" + exit "$retval" +fi + +exit 255 +``` + +(This file, possibly with updates, is also available +[here](./examples/php).) + +As with `custom-sniffer`, a reformatter doesn't have to be written as a shell +script. It just needs to be properly named and executable. + +## Acceptable output + +As output, this script should emit whatever the programs it calls emit. + +## Return values + +### In `would_format` mode + +`would-format.sh` interprets return values to mean the following: + +- 0: file wouldn't be reformatted +- 1: file would be reformatted +- 2: file has at least one syntax error +- 252: Unexpected return value from the tool; that is, the value of `$?` is + unexpected and thus unhandled +- 253: Unexpected output from the tool; that is, the output emitted by the + tool is unexpected and thus unhandled +- 254: file can't be read due to permissions, etc ("etc" is as of + 2023-Q3 broad in scope; it could include trying and failing to read a file + on an NFS mount) +- 255: error internal to the script in question + +(FIXME: Unsurprisingly, `gofmt` is the only tool I've tested so far that gets +this right. That a file should be reformatted isn't an exceptional condition, +so there's no good reason to signal failure here. + +Even less unsurprisingly, Laravel pint gets it worse than all the others I +checked; it doesn't distinguish misformatted files from files with actual +syntax errors. + +Anyway, `gofmt` is the obviously the model to follow. So we should +probably drop `1`.) + + +### in `do_format` mode + +`do-reformat` expects a wrapper to return one of the following +values: + +- 0: file was successfully reformatted +- 1: file was not successfully reformatted, presumably due to a syntax error +- 254: file can't be rewritten due to permissions, etc ("etc" is as of + 2023-Q3 broad in scope; it could include trying and failing to read or write + a file on an NFS mount) +- 255: error internal to the script in question + +# What goes into version control and what does not? + +FIXME: Write this section + +# What files can I modify? + +You should not modify any of the files in `.would-reformat/bin` or the files +that they link to. Everything else in `.would-reformat` is fair game. + +Of course, `would-reformat` is FOSS. Within the limitations set by its +license, you can do what you like with it. But if you want to make deeper +changes, you're probably better off forking and going from there. + diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md new file mode 100644 index 0000000..5d55b1a --- /dev/null +++ b/DEVELOPMENT.md @@ -0,0 +1,66 @@ +# Dependencies + +In addition to a fairly recent version of bash, it's helpful to have the +following tools installed: + +- [shunit2](https://github.com/kward/shunit2) +- [shellcheck](https://www.shellcheck.net/) +- [shfmt](https://github.com/mvdan/sh) + +In general, code should pass shellcheck without warnings, be formatted with +shfmt (please use it with `-i 4`), and come with new tests and pass them when +relevant. + +If any of this causes you trouble, don't worry. Please submit your pull +request anyway and we can work on getting it up to snuff. + +# General goals and non-goals + +## Goals + +- Be as batteries-included as possible. Go out of our way to just work, even + if not entirely optimally, in as many situations as possible. +- Support "relatively isolated" projects. +- Be easy to configure. +- Be clearly documented. Remember that many programmers are working in + high-stress situations. Documentation that is incomplete, unclear, or cute + will make things even worse for them. Use English correctly and, to the + extent possible, unambiguously. + +## Non-goals + + +# Wishlist + +- Generalize this beyond emacs +- In emacs, provide a minor mode or something to use instead of + `.dir-locals.el`, which I've always found to be very fragile +- Add support for more programming languages and file formats +- Clean up shell script code +- Be better at "just working" +- Make it easy to add "private" and alternate formatters +- It should signal more clearly when the file has a syntax error, subject to + the limitations set by third party tools. +- Would global (per-user) customization be useful? My guess is that it'd + probably be more confusing than helpful but I'm not wedded to this + conclusion. +- Write a doctor command to identify potential issues (you'd probably run it + similarly to how you run the install script, but `s/install/doctor`). + +# Problems + +- It would probably be better if we wrote reformatted code to temp files and + atomically renamed them. This might mean that the tool-specific programs + should emit diffs rather than working silently as they currently do. +- It probably doesn't work on Windows as-is. +- There are probably some Linuxisms and maybe even some recentbashisms. +- It doesn't work with tramp. +- As long as it's written in shell, it's always going to be a little bit slow + and, of course, fairly difficult to hack on. If the bulk of it were + rewritten in, say, go, then some problems would go away and some new ones + would be created. On balance I'm not sure if it's a good idea or not, but + I'm tempted. +- Stop writing files to `$PROJECT_ROOT/bin`; use + `$PROJECT_ROOT/.would-reformat/bin` instead. +- `PROJECT_ROOT` and `WF_ROOT` are probably too generic. Consider prefixing + them, maybe with `WOULD_FORMAT_`. diff --git a/README.md b/README.md index da23792..1f3ba93 100644 --- a/README.md +++ b/README.md @@ -38,16 +38,16 @@ saving a file and to run `do-reformat.sh` when you hit ``. - typescript - vue -Python uses `black`; go uses `gofmt`; dart uses `dart format`. The rest use -`prettier`. Adding new programming languages is easy, assuming they have a -formatter with a dry run mode. +Out of the box, python uses `isort` and `black`, go uses `gofmt`, and dart +uses `dart format`. The rest use `prettier`. Adding new programming +languages is easy, assuming they have a formatter with a dry run mode. # Installation -- Change to the directory where you want to use **would-reformat**: +- Change to the directory where you want to use `would-reformat`: `you@host:~ $ cd ~/devel/my-project` - From that directory, call the script `install.sh` in the directory where - you have **would-reformat** checked out: + you have `would-reformat` checked out: `you@host:~/devel/my-project $ ~/src/would-reformat/install.sh` Be sure to run the installation script from the root of your project's @@ -65,7 +65,8 @@ This will do the following: This should work from a checkout wherever you happen to have it. It doesn't need to be in `~/src`. However, if you remove the checkout, then the symlinks -will break, so don't do that. If you rename the checkout, the symlinks will break. +will break, so don't do that. If you rename the checkout, the symlinks will +break. The directory `bin` with respect to `my-project` however is hardcoded (enhancements here are welcome). @@ -76,6 +77,20 @@ checks whether it is a symlink to `~/src/would-reformat/do-reformat.sh`. If it is not, then it just prints a warning rather than attempting to correct the situation. +FIXME: Document how to reinstall if the checkout was moved, incompatible +changes were made, etc. + +# Customization + +For information on changing how `would-format` handles different files, see +[CUSTOMIZATION.md](CUSTOMIZATION.md). + + +# Development + +If you want to hack on `would-format`, see [DEVELOPMENT.md](DEVELOPMENT.md) +for more info. + # Troubleshooting ## nothing works from emacs @@ -106,21 +121,31 @@ cargoculty. There should be a more solid basis for this answer.) The solution seems to be to run `pip install black pipx` somewhere. -# Development +## I need to use one version of `would-reformat` with one of my projects and another with another -# Wishlist +With the per-project configuration [see below] this should rarely be +necessary, but in case it is, one thing you can try is creating different +`would-reformat` checkouts for different projects. For example, something +like this might work: -- Generalize this beyond emacs -- In emacs, provide a minor mode or something to use instead of - `.dir-locals.el`, which I've always found to be very fragile -- Add support for more programming languages and file formats -- Clean up shell script code -- Be better at "just working" -- Make it easy to add "private" and alternate formatters +``` +$ ls ~/devel +new-project1 new-project2 old-and-weird-project1 +$ git clone https://github.com/maw/would-reformat +$ cd new-project1; ../would-reformat/install.sh +$ cd ../new-project2; ../would-reformat/install.sh +$ cd .. +$ git clone https://github.com/maw/would-reformat would-reformat-for-old-and-weird-project1 +$ cd would-reformat-for-old-and-weird-project1; git checkout vSome-old-tag +$ cd ../old-and-weird-project1; ../would-reformat-for-old-and-weird-project1/install.sh +``` -# Problems + +# Similar projects -- It would probably be better if we wrote reformatted code to temp files and - atomically renamed them -- This probably doesn't work on Windows as-is. -- It doesn't work with tramp. +If you don't like `would-reformat`'s approach, maybe one or more of these +would be more suitable. + +- https://github.com/purcell/emacs-shfmt/ +- https://github.com/pythonic-emacs/blacken +- https://github.com/dominikh/go-mode.el diff --git a/examples/php b/examples/php new file mode 100644 index 0000000..04caf57 --- /dev/null +++ b/examples/php @@ -0,0 +1,36 @@ +#!/usr/bin/env bash + +set -euo pipefail +IFS=$'\n\t' + +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +## This usually isn't necessary, but if you need to use wrflog this +## is how you get it: +# source "$WF_ROOT/_reformat-common.bash" + + +if [[ "$WOULD_REFORMAT" = "would_reformat" ]] ; then + set +e + out=$(php $DIR/.././vendor/bin/pint --test "$1") + retval="$?" + set -e + + # unfortunately, pint doesn't distinguish between files with + # syntax errors and files that are merely misformatted + + echo "$out" + exit "$retval" +fi + +if [[ "$WOULD_REFORMAT" = "do_reformat" ]] ; then + set +e + out=$(php $DIR/.././vendor/bin/pint "$1") + retval="$?" + set -e + + echo "$out" + exit "$retval" +fi + +exit 255