diff --git a/.github/scripts/build_assets/arg_getters.py b/.github/scripts/build_assets/arg_getters.py index 71f7d16b8..d086bd331 100644 --- a/.github/scripts/build_assets/arg_getters.py +++ b/.github/scripts/build_assets/arg_getters.py @@ -34,28 +34,32 @@ def get_selenium_runner_args(peek_mode=False): action=PathResolverAction) if peek_mode: - parser.add_argument("--pr_title", + parser.add_argument("pr_title", help="The title of the PR that we are peeking at") else: parser.add_argument("token", - help="The GitHub token to access the GitHub REST API.", - type=str) + help="The GitHub token to access the GitHub REST API.") return parser.parse_args() -def get_check_svgs_on_pr_args(): +def get_check_icon_pr_args(): """ - Get the commandline arguments for the check_svgs_on_pr.py. + Get the commandline arguments for the check_icon_pr.py. """ parser = ArgumentParser(description="Check the SVGs to ensure their attributes are correct. Run whenever a PR is opened") - parser.add_argument("files_added_json_path", - help="The path to the files_added.json created by the gh-action-get-changed-files@2.1.4", + + parser.add_argument("pr_title", + help="The title of the PR that we are peeking at") + + parser.add_argument("icons_folder_path", + help="The path to the icons folder", action=PathResolverAction) - parser.add_argument("files_modified_json_path", - help="The path to the files_modified.json created by the gh-action-get-changed-files@2.1.4", + parser.add_argument("devicon_json_path", + help="The path to the devicon.json", action=PathResolverAction) + return parser.parse_args() @@ -65,8 +69,7 @@ def get_release_message_args(): """ parser = ArgumentParser(description="Create a text containing the icons and features added since last release.") parser.add_argument("token", - help="The GitHub token to access the GitHub REST API.", - type=str) + help="The GitHub token to access the GitHub REST API.") return parser.parse_args() diff --git a/.github/scripts/build_assets/filehandler.py b/.github/scripts/build_assets/filehandler.py index 054431cd2..29e095928 100644 --- a/.github/scripts/build_assets/filehandler.py +++ b/.github/scripts/build_assets/filehandler.py @@ -68,7 +68,7 @@ def get_svgs_paths(new_icons: List[dict], icons_folder_path: str, folder_path = Path(icons_folder_path, icon_info['name']) if not folder_path.is_dir(): - raise ValueError(f"Invalid path. This is not a directory: {folder_path}.") + raise ValueError(f"Invalid path. This is not a directory: '{folder_path}'.") if icon_versions_only: get_icon_svgs_paths(folder_path, icon_info, file_paths, as_str) @@ -100,7 +100,7 @@ def get_icon_svgs_paths(folder_path: Path, icon_info: dict, if path.exists(): file_paths.append(str(path) if as_str else path) else: - raise ValueError(f"This path doesn't exist: {path}") + raise ValueError(f"This path doesn't exist: '{path}'") def get_all_svgs_paths(folder_path: Path, icon_info: dict, @@ -119,7 +119,7 @@ def get_all_svgs_paths(folder_path: Path, icon_info: dict, if path.exists(): file_paths.append(str(path) if as_str else path) else: - raise ValueError(f"This path doesn't exist: {path}") + raise ValueError(f"This path doesn't exist: '{path}'") def is_alias(font_version: str, aliases: List[dict]): @@ -207,6 +207,14 @@ def create_screenshot_folder(dir, screenshot_name: str="screenshots/"): finally: return str(screenshot_folder) +def write_to_file(path: str, value: any): + """ + Write the value to a file. + """ + with open(path, "w") as file: + file.write(value) + +# --- NOT USED CURRENTLY --- def get_added_modified_svgs(files_added_json_path: str, files_modified_json_path: str): """ @@ -230,11 +238,3 @@ def get_added_modified_svgs(files_added_json_path: str, svgs.append(path) return svgs - - -def write_to_file(path: str, value: any): - """ - Write the value to a file. - """ - with open(path, "w") as file: - file.write(value) diff --git a/.github/scripts/build_assets/util.py b/.github/scripts/build_assets/util.py index 9ecbc8075..55ebb3219 100644 --- a/.github/scripts/build_assets/util.py +++ b/.github/scripts/build_assets/util.py @@ -5,7 +5,6 @@ import sys import traceback - def exit_with_err(err: Exception): """ Exit the current step and display the err. @@ -66,3 +65,9 @@ def find_object_added_in_pr(icons: List[dict], pr_title: str): message = "util.find_object_added_in_pr: Couldn't find an icon matching the name in the PR title.\n" \ f"PR title is: '{pr_title}'" raise Exception(message) + + +valid_svg_filename_pattern = re.compile(r"-(original|plain|line)(-wordmark)?\.svg$") +def is_svg_name_valid(filename: str): + return valid_svg_filename_pattern.search(filename) is not None + diff --git a/.github/scripts/check_icon_pr.py b/.github/scripts/check_icon_pr.py new file mode 100644 index 000000000..5116986fb --- /dev/null +++ b/.github/scripts/check_icon_pr.py @@ -0,0 +1,166 @@ +from typing import List +import xml.etree.ElementTree as et +from pathlib import Path + + +# pycharm complains that build_assets is an unresolved ref +# don't worry about it, the script still runs +from build_assets import filehandler, arg_getters, util + + +def main(): + """ + Check the quality of the svgs IF this is an icon PR. Else, does nothing. + If any svg error is found, create a json file called 'svg_err_messages.json' + in the root folder that will contains the error messages. + """ + args = arg_getters.get_check_icon_pr_args() + try: + all_icons = filehandler.get_json_file_content(args.devicon_json_path) + + # get only the icon object that has the name matching the pr title + filtered_icon = util.find_object_added_in_pr(all_icons, args.pr_title) + print("Checking devicon.json object: " + str(filtered_icon)) + devicon_err_msg = check_devicon_object(filtered_icon) + + # check the file names + filename_err_msg = "" + svgs = None + try: + svgs = filehandler.get_svgs_paths([filtered_icon], args.icons_folder_path, as_str=False) + print("SVGs to check: ", *svgs, sep='\n') + except ValueError as e: + filename_err_msg = "Error found regarding filenames:\n- " + e.args[0] + + # check the svgs + if svgs is None or len(svgs) == 0: + print("No SVGs to check, ending script.") + svg_err_msg = "Error checking SVGs: no SVGs to check. Might be caused by above issues." + else: + svg_err_msg = check_svgs(svgs) + + err_msg = [] + if devicon_err_msg != "": + err_msg.append(devicon_err_msg) + + if filename_err_msg != "": + err_msg.append(filename_err_msg) + + if svg_err_msg != "": + err_msg.append(svg_err_msg) + + filehandler.write_to_file("./err_messages.txt", "\n\n".join(err_msg)) + print("Task completed.") + except Exception as e: + filehandler.write_to_file("./err_messages.txt", str(e)) + util.exit_with_err(e) + + +def check_devicon_object(icon: dict): + """ + Check that the devicon object added is up to standard. + :return a string containing the error messages if any. + """ + err_msgs = [] + try: + for tag in icon["tags"]: + if type(tag) != str: + raise TypeError() + except TypeError: + err_msgs.append("- 'tags' must be an array of strings, not: " + str(icon["tags"])) + except KeyError: + err_msgs.append("- missing key: 'tags'.") + + try: + if type(icon["versions"]) != dict: + err_msgs.append("- 'versions' must be an object.") + except KeyError: + err_msgs.append("- missing key: 'versions'.") + + try: + if type(icon["versions"]["svg"]) != list or len(icon["versions"]["svg"]) == 0: + err_msgs.append("- must contain at least 1 svg version in a list.") + + for version in icon["versions"]["svg"]: + if not util.is_version_name_valid(version): + err_msgs.append(f"- Invalid version name in versions['svg']: '{version}'. Must match regexp: (original|plain|line)(-wordmark)?") + except KeyError: + err_msgs.append("- missing key: 'svg' in 'versions'.") + + try: + if type(icon["versions"]["font"]) != list or len(icon["versions"]["svg"]) == 0: + err_msgs.append("- must contain at least 1 font version in a list.") + + for version in icon["versions"]["font"]: + if not util.is_version_name_valid(version): + err_msgs.append(f"- Invalid version name in versions['font']: '{version}'. Must match regexp: (original|plain|line)(-wordmark)?") + except KeyError: + err_msgs.append("- missing key: 'font' in 'versions'.") + + try: + if type(icon["color"]) != str or "#" not in icon["color"]: + err_msgs.append("- 'color' must be a string in the format '#abcdef'") + except KeyError: + err_msgs.append("- missing key: 'color'.") + + try: + if type(icon["aliases"]) != list: + err_msgs.append("- 'aliases' must be an array.") + except KeyError: + err_msgs.append("- missing key: 'aliases'.") + + if len(err_msgs) > 0: + message = "Error found in 'devicon.json' for '{}' entry: \n{}".format(icon["name"], "\n".join(err_msgs)) + return message + return "" + + +def check_svgs(svg_file_paths: List[Path]): + """ + Check the width, height, viewBox and style of each svgs passed in. + The viewBox must be '0 0 128 128'. + The style must not contain any 'stroke' declarations. + If any error is found, they will be thrown. + :param: svg_file_paths, the file paths to the svg to check for. + :return: None if there no errors. If there is, return a JSON.stringified + list with the error messages in it. + """ + # batch err messages together so user can fix everything at once + err_msgs = [] + for svg_path in svg_file_paths: + try: + err_msg = [f"SVG Error in '{svg_path.name}':"] + + # name check + if not util.is_svg_name_valid(svg_path.name): + err_msg.append("- SVG file name didn't match our pattern of `name-(original|plain|line)(-wordmark)?.svg`") + + # svg check + tree = et.parse(svg_path) + root = tree.getroot() + namespace = "{http://www.w3.org/2000/svg}" + + if root.tag != f"{namespace}svg": + err_msg.append(f"- root is '{root.tag}'. Root must be an 'svg' element") + + if root.get("viewBox") != "0 0 128 128": + err_msg.append("- 'viewBox' is not '0 0 128 128' -> Set it or scale the file using https://www.iloveimg.com/resize-image/resize-svg.") + + # goes through all elems and check for strokes + for child in tree.iter(): + if child.get("stroke") != None: + err_msg.append("- SVG contains `stroke` property. This will get ignored by Icomoon. Please convert them to fills.") + break + + if len(err_msg) > 1: + err_msgs.append("\n".join(err_msg)) + except et.ParseError as e: + raise Exception(f"SVG Error in file: {svg_path}. Full Error: \n" + str(e)) + + if len(err_msgs) > 0: + return "\n\n".join(err_msgs) + return "" + + +if __name__ == "__main__": + main() diff --git a/.github/scripts/check_svgs_on_pr.py b/.github/scripts/check_svgs_on_pr.py deleted file mode 100644 index d6a725654..000000000 --- a/.github/scripts/check_svgs_on_pr.py +++ /dev/null @@ -1,89 +0,0 @@ -from enum import Enum -from typing import List -import xml.etree.ElementTree as et -from pathlib import Path - - -# pycharm complains that build_assets is an unresolved ref -# don't worry about it, the script still runs -from build_assets import filehandler, arg_getters -from build_assets import util - - -class SVG_STATUS_CODE(Enum): - """ - The status codes to check for in post_check_svgs_comment.yml - """ - NO_SVG = 0 # action: do nothing - SVG_OK = 1 # action: let user know their svgs are fine - - -def main(): - """ - Check the quality of the svgs. - If any svg error is found, create a json file called 'svg_err_messages.json' - in the root folder that will contains the error messages. - """ - args = arg_getters.get_check_svgs_on_pr_args() - try: - # check the svgs - svgs = filehandler.get_added_modified_svgs(args.files_added_json_path, - args.files_modified_json_path) - print("SVGs to check: ", *svgs, sep='\n') - - if len(svgs) == 0: - print("No SVGs to check, ending script.") - err_messages = SVG_STATUS_CODE.NO_SVG.value - else: - err_messages = check_svgs(svgs) - - filehandler.write_to_file("./svg_err_messages.txt", str(err_messages)) - print("Task completed.") - except Exception as e: - util.exit_with_err(e) - - -def check_svgs(svg_file_paths: List[Path]): - """ - Check the width, height, viewBox and style of each svgs passed in. - The viewBox must be '0 0 128 128'. - If the svg has a width and height attr, ensure it's '128px'. - The style must not contain any 'fill' declarations. - If any error is found, they will be thrown. - :param: svg_file_paths, the file paths to the svg to check for. - :return: None if there no errors. If there is, return a JSON.stringified - list with the error messages in it. - """ - # batch err messages together so user can fix everything at once - err_msgs = [] - for svg_path in svg_file_paths: - try: - tree = et.parse(svg_path) - root = tree.getroot() - namespace = "{http://www.w3.org/2000/svg}" - err_msg = [f"{svg_path}:"] - - if root.tag != f"{namespace}svg": - err_msg.append(f"-root is '{root.tag}'. Root must be an 'svg' element") - - if root.get("viewBox") != "0 0 128 128": - err_msg.append("-'viewBox' is not '0 0 128 128' -> Set it or scale the file using https://www.iloveimg.com/resize-image/resize-svg.") - - if root.get("x") is not None: - err_msg.append("-unneccessary 'x' attribute in svg root element -> Remove it") - - if root.get("y") is not None: - err_msg.append("-unneccessary 'y' attribute in svg root element -> Remove it") - - if len(err_msg) > 1: - err_msgs.append("\n".join(err_msg)) - except et.ParseError as e: - raise Exception(f"SVG Error in file: {svg_path}. Full Error: \n" + str(e)) - - if len(err_msgs) > 0: - return "\n\n".join(err_msgs) - return SVG_STATUS_CODE.SVG_OK.value - - -if __name__ == "__main__": - main() diff --git a/.github/scripts/icomoon_peek.py b/.github/scripts/icomoon_peek.py index 900f1aae4..80a6a6ca3 100644 --- a/.github/scripts/icomoon_peek.py +++ b/.github/scripts/icomoon_peek.py @@ -1,6 +1,5 @@ from build_assets.selenium_runner.PeekSeleniumRunner import PeekSeleniumRunner -from build_assets import filehandler, arg_getters -from build_assets import util +from build_assets import filehandler, arg_getters, util def main(): @@ -11,12 +10,10 @@ def main(): # get only the icon object that has the name matching the pr title filtered_icon = util.find_object_added_in_pr(all_icons, args.pr_title) - check_devicon_object(filtered_icon) - print("Icon being checked:", filtered_icon, sep = "\n", end='\n\n') - - runner = PeekSeleniumRunner(args.download_path, args.geckodriver_path, args.headless) svgs = filehandler.get_svgs_paths([filtered_icon], args.icons_folder_path, True) screenshot_folder = filehandler.create_screenshot_folder("./") + + runner = PeekSeleniumRunner(args.download_path, args.geckodriver_path, args.headless) svgs_with_strokes = runner.peek(svgs, screenshot_folder, filtered_icon) print("Task completed.") @@ -33,57 +30,5 @@ def main(): runner.close() -def check_devicon_object(icon: dict): - """ - Check that the devicon object added is up to standard. - :param icon: a dictionary containing info on an icon. Taken from the devicon.json. - :return a string containing the error messages if any. - """ - err_msgs = [] - try: - for tag in icon["tags"]: - if type(tag) != str: - raise TypeError() - except TypeError: - err_msgs.append("- 'tags' must be an array of strings, not: " + str(icon["tags"])) - except KeyError: - err_msgs.append("- missing key: 'tags'.") - - try: - if type(icon["versions"]) != dict: - err_msgs.append("- 'versions' must be an object.") - except KeyError: - err_msgs.append("- missing key: 'versions'.") - - try: - if type(icon["versions"]["svg"]) != list or len(icon["versions"]["svg"]) == 0: - err_msgs.append("- must contain at least 1 svg version in a list.") - except KeyError: - err_msgs.append("- missing key: 'svg' in 'versions'.") - - try: - if type(icon["versions"]["font"]) != list or len(icon["versions"]["svg"]) == 0: - err_msgs.append("- must contain at least 1 font version in a list.") - except KeyError: - err_msgs.append("- missing key: 'font' in 'versions'.") - - try: - if type(icon["color"]) != str or "#" not in icon["color"]: - err_msgs.append("- 'color' must be a string in the format '#abcdef'") - except KeyError: - err_msgs.append("- missing key: 'color'.") - - try: - if type(icon["aliases"]) != list: - err_msgs.append("- 'aliases' must be an array.") - except KeyError: - err_msgs.append("- missing key: 'aliases'.") - - if len(err_msgs) > 0: - message = "Error found in 'devicon.json' for '{}' entry: \n{}".format(icon["name"], "\n".join(err_msgs)) - raise ValueError(message) - return "" - - if __name__ == "__main__": main() diff --git a/.github/workflows/check_svgs_on_pr.yml b/.github/workflows/check_icon_pr.yml similarity index 56% rename from .github/workflows/check_svgs_on_pr.yml rename to .github/workflows/check_icon_pr.yml index 70dd42446..1247ed8ae 100644 --- a/.github/workflows/check_svgs_on_pr.yml +++ b/.github/workflows/check_icon_pr.yml @@ -1,9 +1,10 @@ -name: Check SVGs On PR +name: Check Icon PR on: pull_request jobs: check: - name: Check the SVGs' quality + name: Check the `devicon.json` and the SVGs' quality runs-on: ubuntu-18.04 + if: startsWith(github.event.pull_request.title, 'new icon') # only checks icon PR steps: - uses: actions/checkout@v2 @@ -11,24 +12,17 @@ jobs: with: python-version: 3.8 - - name: Install dependencies - run: python -m pip install --upgrade pip - - - name: Get Changed Files and generate files_added.json & files_modified.json - uses: lots0logs/gh-action-get-changed-files@2.1.4 - with: - token: ${{ secrets.GITHUB_TOKEN }} - - name: Run the check_svg script - run: > - python ./.github/scripts/check_svgs_on_pr.py $HOME/files_added.json $HOME/files_modified.json + env: + PR_TITLE: ${{ github.event.pull_request.title }} + run: python ./.github/scripts/check_icon_pr.py "$PR_TITLE" ./icons ./devicon.json - name: Upload the err messages uses: actions/upload-artifact@v2 if: success() with: - name: svg_err_messages - path: ./svg_err_messages.txt + name: err_messages + path: ./err_messages.txt - name: Save the pr num in an artifact shell: bash diff --git a/.github/workflows/post_check_svgs_comment.yml b/.github/workflows/post_check_icon_pr_comment.yml similarity index 78% rename from .github/workflows/post_check_svgs_comment.yml rename to .github/workflows/post_check_icon_pr_comment.yml index 5ac89d3aa..716ca752b 100644 --- a/.github/workflows/post_check_svgs_comment.yml +++ b/.github/workflows/post_check_icon_pr_comment.yml @@ -1,7 +1,7 @@ -name: Post the result of a SVG Check into its PR. +name: Post the result of the check_icon_pr workflow into its PR. on: workflow_run: - workflows: ['Check SVGs On PR'] + workflows: ['Check Icon PR'] types: - completed jobs: @@ -18,7 +18,7 @@ jobs: if: success() with: github_token: ${{ secrets.GITHUB_TOKEN }} - workflow: peek_icons.yml + workflow: check_icon_pr.yml run_id: ${{ github.event.workflow_run.id }} - name: Read the pr_num file @@ -33,30 +33,27 @@ jobs: id: err_message_reader uses: juliangruber/read-file-action@v1.0.0 with: - path: ./svg_err_messages/svg_err_messages.txt + path: ./err_messages/err_messages.txt - name: Comment on the PR about the result - SVG Error uses: jungwinter/comment@v1 # let us comment on a specific PR - if: success() && (steps.err_message_reader.outputs.content != '0' && steps.err_message_reader.outputs.content != '1') + if: success() && (steps.err_message_reader.outputs.content != '') env: MESSAGE: | Hi! - I'm Devicons' SVG-Checker Bot and it seems we have some issues with your SVGs. + I'm the `check-bot` and we have some issues with your PR: - Here is what went wrong: ``` {0} ``` - For more reference on why these are errors, check out our [CONTRIBUTING guide](https://github.com/devicons/devicon/blob/develop/CONTRIBUTING.md#svgStandards) + Check our [CONTRIBUTING guide](https://github.com/devicons/devicon/blob/develop/CONTRIBUTING.md#svgStandards) for more details regarding these errors. Please address these issues. When you update this PR, I will check your SVGs again. Thanks for your help, SVG-Checker Bot :smile: - - PS. One day, I will be smart enough to fix these errors for you :persevere:. Until then, I can only point them out. with: type: create issue_number: ${{ steps.pr_num_reader.outputs.content }} diff --git a/gulpfile.js b/gulpfile.js index 9a7c74344..f6cc2fae5 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -187,6 +187,12 @@ function configOptionCallback(file) { }, { removeDimensions: true // remove height and width + }, + { + name: "removeAttrs", + params: { + attrs: "svg:(x|y)" + } } ] };