This repo is using some systems that we've since moved away from:

• setup.py
• versioneer
• maybe others

We should reorganize to make it more like the other repositories we manage.

Issue

This hook: https://github.com/pcdshub/pre-commit-hooks/blob/master/pre_commit_hooks/xml_format.py#L1
sometimes requires multiple tries to properly reformat:

• First try: reformats partially
• Second try: reformats fully
• Third try: allows you to commit

We should double-check the code - potentially reformatting with multiple iterations or increasing some parameter to the existing call to dive deeper into the XML.

We had a discussion here TcOpenGroup/TcOpen#731 about the mixed TwinCAT versions and pinned ones in the tsproj files. Would it be an idea to add another pre-commit to check for this? I can add it.

Something like check-twincat-versions --target-version 4024.22 --pinned

Different cases:

1. In case of a single project, you can select if it should be pinned and add an optional target version.
2. For two or more, it can check if all target versions match the targeted one and if they are pinned. If no target version is given, it checks whether more than one version is used.

Optionally, I can also add an --fix option, that would fix --pinned and if a --target-version is given.

I wrote a small post regarding using pre-commits with TwinCAT files. I used your pre-commit as and example as well. https://cookncode.com/twincat/2022/04/14/pre-commit.html

Thanks for making it available!

Maybe an idea to add the .pre-commit-config.yaml files to the README.md? Then more experienced people can immediately copy paste the relevant stuff. For example how it's done here: https://github.com/pre-commit/pre-commit-hooks.

• Can include actual check scripts here in this repository, or refer to pcds-ci-helpers scripts?

https://pre-commit.com/#usage-in-continuous-integration

My original thought, not necessarily how we should do it, given the above documentation:

matrix:
  include:
    name: pre-commit checks
    python: 3.7
    env:
      PRE_COMMIT=1


script:
    - |
      if [ "$PRE_COMMIT" ]; then
        pip install pre-commit
        pre-commit install
        pre-commit run --all-files  # or whatever it is
        exit $?  #  exit with pre-commits return code
      fi

Likely this should be part of pcds-ci-helpers. x-ref an issue there: pcdshub/pcds-ci-helpers#12

Saw this today: https://pre-commit.com/#automatically-enabling-pre-commit-on-repositories

If it works, it'd be a lot easier than trying to get everyone to run pre-commit install every time they clone a repo.

Add pytmc pragma linting as a pre-commit hook.

It's only sensible that pre-commit checks for pragma lint so that another step in the Tools menu is unnecessary.

Investigate the built-in git from the TwinCAT XAE shell and figure out how to configure it to run the pre-commit hooks properly

I started investigating this a little bit using the TwinCAT installed on the programming node. In the TwinCAT docs, the interface appears to have more configuration options than mine does in Tools > Options > Source Control, where I only have the option of changing my source control plug-in. I assume this means that the git plugin doesn't have any configuration options.

I peeked at the pre-commit hook script generated on Windows and the shebang looks like:
#!/usr/bin/env python.exe
I'm not sure if this leads to any clues, or if modifying this default script will be the path forward.

As an aside, there will need to be extra effort to get this working at all on the programming node due to the airgap

I'm considering making a pre-commit hook that inserts newlines at the start and end of TwinCAT POUs (and some of the other files?). When a TwinCAT POU starts and ends with a newline, it is much easier to review in github. Snippit from a current PR of mine that does not do this:

    <Implementation>
      <ST><![CDATA[fbNCErrorFFO(
    stMotionStage := stMotionStage,
    fbFFHWO := fbFFHWO,
    bReset := bReset,
    sName := sName,
    bTripped => bTripped);]]></ST>
    </Implementation>

With pre and post newlines:

    <Implementation>
      <ST><![CDATA[
fbNCErrorFFO(
    stMotionStage := stMotionStage,
    fbFFHWO := fbFFHWO,
    bReset := bReset,
    sName := sName,
    bTripped => bTripped);
]]></ST>
    </Implementation>

With two pre and two post newlines:

    <Implementation>
      <ST><![CDATA[

fbNCErrorFFO(
    stMotionStage := stMotionStage,
    fbFFHWO := fbFFHWO,
    bReset := bReset,
    sName := sName,
    bTripped => bTripped);

]]></ST>
    </Implementation>

Current test one is:

$ cat .pre-commit-config.yaml

# See https://pre-commit.com for more information
# See https://pre-commit.com/hooks.html for more hooks
repos:
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v2.5.0
    hooks:
    -   id: trailing-whitespace
    -   id: end-of-file-fixer

-   repo: https://gitlab.com/pycqa/flake8
    rev: 3.7.9
    hooks:
    -   id: flake8

Add:

• Import sorting via isort?

Thinking doc8 could be useful for checking our static .rst files. Thoughts?

-   repo: https://github.com/PyCQA/doc8
    rev: 0.8.1rc3
    hooks:
    - id: doc8