WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.56k stars 487 forks source link

CI: add an xml formatting check for docs files #2337

Open jrfnl opened 1 year ago

jrfnl commented 1 year ago

See the conversation here: https://github.com/WordPress/WordPress-Coding-Standards/pull/2334#discussion_r1294579815

Basically we'd need a little (bash or otherwise) script to gather the files in the WordPress/Docs directory and then run the following command against each file with XMLLINT_INDENT set to (four) spaces.

diff -B --tabsize=4 ./WordPress/Docs/Cat/SniffNameStandard.xml <(xmllint --format "./WordPress/Docs/Cat/SniffNameStandard.xml")
viditagrawal56 commented 5 months ago

Hey @jrfnl 👋 I am interested in contributing to this issue. I read through the conversation and I understand the gist of the issue.

I have come up with the following bash script which recursively goes through all the folders in the Wordpress/Docs and runs the command diff -B --tabsize=4 "$file" <(xmllint --format "$file")

#!/bin/bash

ROOT_DIR="./WordPress/Docs"

export XMLLINT_INDENT="    "

process_xml_files() {
    local dir="$1"

    echo "Processing directory: $dir"

    if [ ! -d "$dir" ]; then
        echo "Directory $dir does not exist."
        return
    fi

    # Loop through each file in the directory
    for file in "$dir"/*; do
        if [ -d "$file" ]; then
            # If the file is a directory, then recurse into it
            process_xml_files "$file"
        elif [[ "$file" == *.xml ]]; then
            # If the file is an XML file, run the diff command
            echo "Processing file: $file"
            diff -B --tabsize=4 "$file" <(xmllint --format "$file")
        else
            echo "Skipping non-XML file: $file"
        fi
    done
}

process_xml_files "$ROOT_DIR"

Here's a pastebin link for easy viewing https://pastebin.com/VxKB5krb

I tested this script locally by mimicking the Wordpress/Docs folder structure and it successfully shows the diff between the formatted and unformatted file.

This script just shows the diff and does not update the xml file. Let me know if that is needed to be implemented! I will create a PR if everything is correct.

jrfnl commented 5 months ago

Hi @viditagrawal56, thank you for your interest in helping us figure this out.

Would the above script work cross-platform ? I.e. would it be able to run on both *nix systems as well as Windows ? that would be helpful to allow contributors to run the script locally.

Another question I'm wondering about: How difficult would it be to create this as a bash script with a parameter for the path(s) to check ? If that would be an option, it would be great if this could be turned into a script which would be re-usable, not just by WPCS, but also by something like PHPCSExtra, PHPCompatibility and even PHP_CodeSniffer itself.

If so, it might be best to create this as a new feature in PHPCSDevTools. What do you think ?

viditagrawal56 commented 5 months ago

Would the above script work cross-platform ? I.e. would it be able to run on both *nix systems as well as Windows ? that would be helpful to allow contributors to run the script locally.

The script that I wrote works only on Unix-like systems because of the following reasons:-

  1. The script is a Bash script and Windows does not natively support Bash scripts.

  2. xmllint is part of libxml2 and is not included by default on Windows and needs to be installed separately.

Although WSL can be used for these problems.

Another question I'm wondering about: How difficult would it be to create this as a bash script with a parameter for the path(s) to check ?

It's actually pretty straightforward, Here`s the code that for bash script that takes path(s) as parameter.

#!/bin/bash

export XMLLINT_INDENT="    "

process_xml_files() {
    local dir="$1"

    echo "Processing directory: $dir"

    if [ ! -d "$dir" ]; then
        echo "Directory $dir does not exist."
        return
    fi

    # Loop through each file in the directory
    for file in "$dir"/*; do
        if [ -d "$file" ]; then
            # If the file is a directory, then recurse into it
            process_xml_files "$file"
        elif [[ "$file" == *.xml ]]; then
            # If the file is an XML file, run the diff command
            echo "Processing file: $file"
            diff -B --tabsize=4 "$file" <(xmllint --format "$file")
        else
            echo "Skipping non-XML file: $file"
        fi
    done
}

# Check if at least one argument is passed
if [ $# -eq 0 ]; then
    echo "Please provide path(s) to the directory"
    echo "Usage: $0 path1 [path2 ... pathN]"
    exit 1
fi

# Loop through all the arguments (paths)
for path in "$@"; do
    process_xml_files "$path"
done

Here's pasetbin link for easy viewing https://pastebin.com/EmpErJX5

It's all same at the start with just an addition for checking if args are provided or not and then calling the function for each path at the bottom.

If that would be an option, it would be great if this could be turned into a script which would be re-usable, not just by WPCS, but also by something like PHPCSExtra, PHPCompatibility and even PHP_CodeSniffer itself.

If so, it might be best to create this as a new feature in PHPCSDevTools. What do you think ?

Yeah, I think that would be a great idea!

The solution for the cross platform problem of this script is to use wsl, even then libxml2 would be needed to be installed manually if its not already installed. Although I think that we can include a step to install libxml2 automatically in the script.

What do you think @jrfnl?

viditagrawal56 commented 5 months ago

@jrfnl thoughts?

jrfnl commented 5 months ago

@viditagrawal56 Sorry, I was out of the office for few days. I do have some more points/questions/suggestions, but am happy to proceed with this. Would you like to open a PR in the PHPCSDevTools repo ? We can talk through the rest of the implementation details there.

jrfnl commented 5 months ago

@viditagrawal56 I've opened a ticket in the DevTools repo to continue this discussion: https://github.com/PHPCSStandards/PHPCSDevTools/issues/145