Python code is not consistently formatted

[louismerlin]

This issue could be solved by using a code formatter like black.

Then, if we start using an automated testing environment, it could be integrated into that.

If the team agrees on this, I can quickly submit a PR.

[diagprov]

This is a nice idea, but it will be quite a big diff all in one go so I think we should wait until we have tests to check that things are sane.

Q1 Does this tool have a compliance checker we can use, i.e. a command you can run that does not format anything but reports inconsistencies? If so, one thing we can do is add a githook. It looks like this:

.githooks/pre-commit:

#!/bin/sh

echo
count=0
replacedirective=0

echo "Checking for merge conflicts"
for f in `git diff HEAD --name-only --diff-filter=M`; do
    if [[ -f $f ]] && [[ $f != .githooks/* ]]; then
        if [[ $(grep -c "<<<<<<" $f) -gt 0 ]] || [[ $(grep -c ">>>>>>" $f) -gt 0 ]]; then
            echo "$(tput setaf 1)! $f$(tput sgr0)"
            ((count += 1))
        fi
    fi
done

if [ $count != 0 ]; then
    echo "$(tput setaf 1)$count$(tput sgr0) files conflict, please resolve conflicts."
    exit 1;
else
    echo "$(tput setaf 2)No conflicts, continuing with commit.$(tput sgr0)"
fi

This is an example from my personal code that will prevent anyone from committing with <<< unmerged code from a git merge. It can be installed with this script:

#!/bin/sh

GIT_VERSION=$(git --version | cut -d" " -f3)
GIT_REQUIRED_VERSION=2.9.0

if [ "$(printf '%s\n' "$GIT_REQUIRED_VERSION" "$GIT_VERSION" | sort -V | head -n1)" = "$GIT_REQUIRED_VERSION" ]; then 
    echo "GIT >= 2.9, installing .githooks directory"
    git config core.hooksPath .githooks
else
    echo "Copying githooks to .git"
    #find .git/hooks -type l -exec rm {} \; && find .githooks -type f -exec ln -sf ../../{} .git/hooks/ \;
fi

Everyone will need to run that locally, since git is distributed, but the result is that it will stop you committing unfinished merges. I'm wondering if we can adjust it to warn when people commit code that is out of style.

Q2 Is there integration into editors for this tool? I generally use editorconfig, but it is "light" on what it does i.e. focuses on tabs/spaces and so on. Can we configure a style, e.g. pep8, and just format the code like that?

Q3 is this significantly different to a PEP8 formatter?

[louismerlin]

Q1 Yes, you can only report inconsistencies with the --diff flag, and only return the status with the --check flag.

A githook souds nice ! Also, black --check can be the first step of the automated testing. You quickly learn that if you don't want your build to fail, you should run the linter before you commit :)

Q2 About integration : Black is shipped with many default editor integrations : for example, it is in Microsoft's default python VS Code extension.

Q3 From Black's main README :

Black is a PEP 8 compliant opinionated formatter.

[cyanpencil]

I agree, the git precommit looks like a good solution. The main takeaway is that this will ruin our diff history. But refactoring will bring heavy changes too, so maybe we can introduce this when we begin refactoring?

[diagprov]

I think this should be a separate commit and not done as part of other work. If necessary we can go submodule by submodule and run this tool, so split it over several commits if needed, so we can eyeball things. You can still do git history path/to/file.py to see what changes were done to the file.

I'm all for it!

[diagprov]

Still a problem. We'll get there :) but in the public repo https://github.com/HexHive/retrowrite.