radian-software / apheleia

🌷 Run code formatter on buffer contents without moving point, using RCS patches and dynamic programming.
MIT License
517 stars 73 forks source link

Add `docformatter` which formats Python docstrings to PEP 257 #267

Closed meliache closed 6 months ago

meliache commented 7 months ago

Add docformatter for Python docstrings.

By default it outputs diffs but changes in-place with --in-place. On successful change it exits with an error code of 3 (found out by trial), so I had to add a formatter wrapping-script.

Initially I used --in-place with the special in-place symbol in apheleia. But now I tried an approach where I transform the diff into usable stdout using patch instead.

Related to #266 , where I had used the example of docformatter to ask how to add scripts with positive exit codes and @raxod502 showed me the phpcs solution.

meliache commented 7 months ago

I have possible implementations of a docformatter wrapper script and both work interactively but fail the tests. I think I need some help. maybe @raxod502 has some idea?

Implementation 1: Using --in-place

Script:

#!/bin/sh
docformatter --in-place "$@"
if [ "$?" -eq 3 ]; then
    exit 0
fi

Formatter:

    (docformatter . ("apheleia-docformatter" inplace))

Error:

  error("Formatter %s modified original file in place" "docformatter")

In place is exactly what I tried to do here, supposedly apheleia has support for that, but seems it doesn't work for tests.

Implementation 1: Transformind diff to stdout

Script:

#!/bin/sh
output=$(docformatter "$@")
ret=$?
[ $ret -gt 0 ] && [ $ret -ne 3 ] && exit $ret
echo "${output}" | patch --quiet --output=-

Formatter:

    (docformatter . ("apheleia-docformatter" file))

Error:

[Errno 2] No such file or directory: '/tmp/srctestformatterssamplecodedocformatterin.py'

When testing interactively after a while I found the call to patch with standard input is very sensitive to from where it's called. It works when it's called in the same directory as the in.py but when it's in another directory it doesn't work except with a correct --strip argument. Not sure if this is what causes this weird error message.

raxod502 commented 6 months ago

It seems simpler to go with in-place in this case, and the feature is supported for a reason. Let me have a look and see if I can fix the issue with the test framework.

raxod502 commented 6 months ago

Problem 2 is solved by some bug fixes I made last week in https://github.com/radian-software/apheleia/pull/249/commits/74d9a5d201795126e4457ad9c42aa755113e6cd2, problem 1 is solved by https://github.com/radian-software/apheleia/pull/267/commits/d74a848ae5f1d7e1d0a47c785d3af16e7152a444 just now. Sorry for how bad the test framework is.

meliache commented 6 months ago

Thanks for fixing and merging this! I was positively surprised that this package even has a dockerized testing framework in the first place, I have contributed to enough packages with no unit tests and have no personal experience with doing them in Elisp.