Added check script for the icons and fix icons with fill or viewBox issues (#460)

* Added script to check svg fill and viewBox

* Fix cucumber-plain-wordmark

* Fix various fill and viewBox issues in svgs

* Added check for height and width attr

* Added check_svgs workflow

* Fix an issue where the error is not log properly

* Added on push for testing

* Updated trigger so it now runs whenever PR is update

* Added sleep to script to make logs nicer

* Added script that create env var

* Updated the github_env to accomodate ubuntu

* Change format of log and allow filehandler to return Path

* Updated logging messages

* Updated refs for the checkout action

* Make logging nicer

* Updated fix messaging so it's more clear

* fix icons: icons/cucumber/cucumber-plain-wordmark.svg, icons/intellij/intellij-plain-wordmark.svg, icons/jenkins/jenkins-plain.svg, icons/twitter/twitter-original.svg, icons/yunohost/yunohost-plain.svg

.github/scripts/build_assets/arg_getters.py

@@ -3,6 +3,10 @@ from build_assets.PathResolverAction import PathResolverAction
 
 
 def get_selenium_runner_args(peek_mode=False):
+    """
+    Get the commandline arguments for the icomoon_peek.py and 
+    icomoon_build.py.
+    """
     parser = ArgumentParser(description="Upload svgs to Icomoon to create icon files.")
 
     parser.add_argument("--headless",
@@ -33,4 +37,23 @@ def get_selenium_runner_args(peek_mode=False):
         parser.add_argument("--pr_title",
                             help="The title of the PR that we are peeking at")
 
+    return parser.parse_args()
+
+
+def get_check_svgs_args():
+    """
+    Get the commandline arguments for the chec_svgs.py.
+    """
+    parser = ArgumentParser(description="Check the SVGs to ensure their attributes are correct.")
+    parser.add_argument("icomoon_json_path",
+                        help="The path to the icomoon.json aka the selection.json created by Icomoon",
+                        action=PathResolverAction)
+
+    parser.add_argument("devicon_json_path",
+                        help="The path to the devicon.json",
+                        action=PathResolverAction)
+
+    parser.add_argument("icons_folder_path",
+                        help="The path to the icons folder",
+                        action=PathResolverAction)
     return parser.parse_args()

.github/scripts/build_assets/drafts/check_devicon_object.py

@@ -0,0 +1,37 @@
+from typing import List
+
+# abandoned since it's not too hard to check devicon objects using our eyes
+# however, still keep in case we need it in the future
+
+def check_devicon_objects(icons: List[dict]):
+    """
+    Check that the devicon objects added is up to standard.
+    """
+    err_msgs = []
+    for icon in icons:
+        if type(icon["name"]) != str:
+            err_msgs.append("'name' must be a string, not: " + str(icon["name"]))
+
+        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"]))
+            break
+
+
+        if type(icon["versions"]["svg"]) != list or len(icon["versions"]["svg"]) == 0:
+            err_msgs.append("Icon name must be a string")
+        
+        if type(icon["versions"]["font"]) != list or len(icon["versions"]["svg"]) == 0:
+            err_msgs.append("Icon name must be a string")
+
+        if type(icon["color"]) != str or "#" not in icon["color"]:
+            err_msgs.append("'color' must be a string in the format '#abcdef'")
+
+        if type(icon["aliases"]) != list:
+            err_msgs.append("'aliases' must be an array of dicts")
+    
+    if len(err_msgs) > 0:
+        raise Exception("Error found in devicon.json: \n" + "\n".join(err_msgs))

.github/scripts/build_assets/filehandler.py

@@ -44,12 +44,16 @@ def is_not_in_icomoon_json(icon, icomoon_json) -> bool:
     return True
 
 
-def get_svgs_paths(new_icons: List[dict], icons_folder_path: str) -> List[str]:
+def get_svgs_paths(new_icons: List[dict], icons_folder_path: str, 
+    icon_versions_only: bool=False, as_str: bool=True) -> List[str] or List[Path]:
     """
     Get all the suitable svgs file path listed in the devicon.json.
     :param new_icons, a list containing the info on the new icons.
     :param icons_folder_path, the path where the function can find the
     listed folders.
+    :param icon_versions_only, whether to only get the svgs that can be 
+    made into an icon. 
+    :param: as_str, whether to add the path as a string or as a Path.
     :return: a list of svg file paths that can be uploaded to Icomoon.
     """
     file_paths = []
@@ -59,29 +63,58 @@ def get_svgs_paths(new_icons: List[dict], icons_folder_path: str) -> List[str]:
         if not folder_path.is_dir():
             raise ValueError(f"Invalid path. This is not a directory: {folder_path}.")
 
-        # TODO: remove the try-except when the devicon.json is upgraded
-        try:
-            aliases = icon_info["aliases"]
-        except KeyError:
-            aliases = [] # create empty list of aliases if not provided in devicon.json
-
-        for font_version in icon_info["versions"]["font"]:
-            # if it's an alias, we don't want to make it into an icon
-            if is_alias(font_version, aliases):
-                print(f"Not exist {icon_info['name']}-{font_version}.svg")
-                continue
-
-            file_name = f"{icon_info['name']}-{font_version}.svg"
-            path = Path(folder_path, file_name)
-
-            if path.exists():
-                file_paths.append(str(path))
-            else:
-                raise ValueError(f"This path doesn't exist: {path}")
-
+        if icon_versions_only:
+            get_icon_svgs_paths(folder_path, icon_info, file_paths, as_str)
+        else:
+            get_all_svgs_paths(folder_path, icon_info, file_paths, as_str)
     return file_paths
 
 
+def get_icon_svgs_paths(folder_path: Path, icon_info: dict,
+    file_paths: List[str], as_str: bool):
+    """
+    Get only the svg file paths that can be made into an icon from the icon_info.
+    :param: folder_path, the folder where we can find the icons.
+    :param: icon_info, an icon object in the devicon.json.
+    :param: file_paths, an array containing all the file paths found.
+    :param: as_str, whether to add the path as a string or as a Path.
+    """
+    aliases = icon_info["aliases"]
+
+    for font_version in icon_info["versions"]["font"]:
+        # if it's an alias, we don't want to make it into an icon
+        if is_alias(font_version, aliases):
+            print(f"Skipping this font since it's an alias: {icon_info['name']}-{font_version}.svg")
+            continue
+
+        file_name = f"{icon_info['name']}-{font_version}.svg"
+        path = Path(folder_path, file_name)
+
+        if path.exists():
+            file_paths.append(str(path) if as_str else path)
+        else:
+            raise ValueError(f"This path doesn't exist: {path}")
+
+
+def get_all_svgs_paths(folder_path: Path, icon_info: dict,
+    file_paths: List[str], as_str: bool):
+    """
+    Get all the svg file paths of an icon.
+    :param: folder_path, the folder where we can find the icons.
+    :param: icon_info, an icon object in the devicon.json.
+    :param: file_paths, an array containing all the file paths found.
+    :param: as_str, whether to add the path as a string or as a Path.
+    """
+    for font_version in icon_info["versions"]["svg"]:
+        file_name = f"{icon_info['name']}-{font_version}.svg"
+        path = Path(folder_path, file_name)
+
+        if path.exists():
+            file_paths.append(str(path) if as_str else path)
+        else:
+            raise ValueError(f"This path doesn't exist: {path}")
+
+
 def is_alias(font_version: str, aliases: List[dict]):
     """
     Check whether the font version is an alias of another version.

.github/scripts/build_assets/github_env.py

@@ -0,0 +1,33 @@
+import os
+import platform
+
+
+def set_env_var(key: str, value: str, delimiter: str='~'):
+    """
+    Set the GitHub env variable of 'key' to 'value' using
+    the method specified here:
+    https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-environment-variable
+    Support both Windows and Ubuntu machines provided by GitHub Actions.
+
+    :param: key, the name of the env variable.
+    :param: value, the value of the env variable.
+    :param: delimiter, the delimiter that you want to use
+    to write to the file. Only applicable if the 'value' contains
+    '\n' character aka a multiline string.
+    """
+    if platform.system() == "Windows":
+        if "\n" in value:
+            os.system(f'echo "{key}<<{delimiter}" >> %GITHUB_ENV%')
+            os.system(f'echo "{value}" >> %GITHUB_ENV%')
+            os.system(f'echo "{delimiter}" >> %GITHUB_ENV%')
+        else:
+            os.system(f'echo "{key}={value}" >> %GITHUB_ENV%')
+    elif platform.system() == "Linux":
+        if "\n" in value:
+            os.system(f'echo "{key}<<{delimiter}" >> $GITHUB_ENV')
+            os.system(f'echo "{value}" >> $GITHUB_ENV')
+            os.system(f'echo "{delimiter}" >> $GITHUB_ENV')
+        else:
+            os.system(f'echo "{key}={value}" >> $GITHUB_ENV')
+    else:
+        raise Exception("This function doesn't support this platform: " + platform.system())

.github/scripts/check_all_icons.py

@@ -0,0 +1,18 @@
+from pathlib import Path
+import json
+
+
+# pycharm complains that build_assets is an unresolved ref
+# don't worry about it, the script still runs
+from build_assets import filehandler
+
+
+if __name__ == "__main__":
+    """
+    Use as a cmd line script to check all the icons of the devicon.json.
+    """
+    devicon_json_path = str(Path("./devicon.json").resolve())
+    icons_folder_path = str(Path("./icons").resolve())
+    with open(devicon_json_path) as json_file:
+        devicon_json = json.load(json_file)
+        svgs = filehandler.get_svgs_paths(devicon_json, icons_folder_path)

.github/scripts/check_svgs.py

@@ -0,0 +1,91 @@
+from typing import List
+import sys
+import xml.etree.ElementTree as et
+import time
+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 github_env
+
+
+def main():
+    """
+    Check the quality of the svgs.
+    If any error is found, set an environmental variable called ERR_MSGS
+    that will contains the error messages.
+    """
+    args = arg_getters.get_check_svgs_args()
+    new_icons = filehandler.find_new_icons(args.devicon_json_path, args.icomoon_json_path)
+
+    if len(new_icons) == 0:
+        sys.exit("No files need to be uploaded. Ending script...")
+
+    # print list of new icons
+    print("SVGs being checked:", *new_icons, sep = "\n", end='\n\n')
+
+    time.sleep(1)  # do this so the logs stay clean
+    try:
+        # check the svgs
+        svgs = filehandler.get_svgs_paths(new_icons, args.icons_folder_path, as_str=False)
+        check_svgs(svgs)
+        print("All SVGs found were good.\nTask completed.")
+    except Exception as e:
+        github_env.set_env_var("ERR_MSGS", str(e))
+        sys.exit(str(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.
+    """
+    # batch err messages together so user can fix everything at once
+    err_msgs = []
+    for svg_path in svg_file_paths:
+        tree = et.parse(svg_path)
+        root = tree.getroot()
+        namespace = "{http://www.w3.org/2000/svg}"
+        err_msg = [f"{svg_path.name}:"]
+
+        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")
+
+        acceptable_size = [None, "128px", "128"]
+        if root.get("height") not in acceptable_size:
+            err_msg.append("-'height' is present in svg element but is not '128' or '128px' -> Remove it or set it to '128' or '128px'")
+
+        if root.get("width") not in acceptable_size:
+            err_msg.append("-'width' is present in svg element but is not '128' or '128px' -> Remove it or set it to '128' or '128px'")
+
+        if root.get("style") is not None and "enable-background" in root.get("style"):
+            err_msg.append("-deprecated 'enable-background' in style attribute -> Remove it")
+
+        if root.get("x") is not None:
+            err_msg.append("-unneccessary 'x' attribute in svg element -> Remove it")
+
+        if root.get("y") is not None:
+            err_msg.append("-unneccessary 'y' attribute in svg element -> Remove it")
+
+        style = root.findtext(f".//{namespace}style")
+        if style != None and "fill" in style:
+            err_msg.append("-contains style declaration using 'fill' -> Replace classes with the 'fill' attribute instead")
+
+        if len(err_msg) > 1:
+            err_msgs.append("\n".join(err_msg))
+
+    if len(err_msgs) > 0:
+        raise Exception("Errors found in these files:\n" + "\n\n".join(err_msgs))
+
+
+if __name__ == "__main__":
+    main()

.github/scripts/icomoon_build.py

@@ -22,7 +22,7 @@ def main():
         runner = SeleniumRunner(args.download_path,
                                 args.geckodriver_path, args.headless)
         runner.upload_icomoon(args.icomoon_json_path)
-        svgs = filehandler.get_svgs_paths(new_icons, args.icons_folder_path)
+        svgs = filehandler.get_svgs_paths(new_icons, args.icons_folder_path, True)
         runner.upload_svgs(svgs)
 
         zip_name = "devicon-v1.0.zip"
@@ -34,7 +34,7 @@ def main():
     except TimeoutException as e:
         sys.exit("Selenium Time Out Error: \n" + str(e))
     except Exception as e:
-        sys.exit(e)
+        sys.exit(str(e))
     finally:
         runner.close() 
 

.github/scripts/icomoon_peek.py

@@ -2,6 +2,7 @@ from typing import List
 import re
 import sys
 from selenium.common.exceptions import TimeoutException
+import xml.etree.ElementTree as et
 
 # pycharm complains that build_assets is an unresolved ref
 # don't worry about it, the script still runs
@@ -21,7 +22,7 @@ def main():
 
     if len(filtered_icons) == 0:
         message = "No icons found matching the icon name in the PR's title.\n" \
-        "Ensure that the PR title matches the convention here: \n" \
+        "Ensure that a new icon entry is added in the devicon.json and the PR title matches the convention here: \n" \
         "https://github.com/devicons/devicon/blob/master/CONTRIBUTING.md#overview.\n" \
         "Ending script...\n"
         sys.exit(message)
@@ -33,14 +34,14 @@ def main():
     runner = None
     try:
         runner = SeleniumRunner(args.download_path, args.geckodriver_path, args.headless)
-        svgs = filehandler.get_svgs_paths(filtered_icons, args.icons_folder_path)
+        svgs = filehandler.get_svgs_paths(filtered_icons, args.icons_folder_path, True)
         screenshot_folder = filehandler.create_screenshot_folder("./") 
         runner.upload_svgs(svgs, screenshot_folder)
         print("Task completed.")
     except TimeoutException as e:
         sys.exit("Selenium Time Out Error: \n" + str(e))
     except Exception as e:
-        sys.exit(e)
+        sys.exit(str(e))
     finally:
         runner.close() 
 
@@ -62,5 +63,7 @@ def find_object_added_in_this_pr(icons: List[dict], pr_title: str):
         return []  
 
 
+
+
 if __name__ == "__main__":
     main()