feat: auto-rename icon PRs; icon PRs may only touch their own icon

.github/scripts/build_assets/util.py

@@ -81,28 +81,42 @@ def get_pr_icon_changes(devicon_json: List[dict], base: str="HEAD^1"):
     :param devicon_json, the devicon.json content of the PR.
     :param base, the commit to compare against.
     :return a dict with:
-        "svgs": {icon folder name: [SVG file names added/modified/renamed]}
-        "json": names of devicon.json entries that were added or changed
+        "svgs": {icon folder name: [SVG file names added/modified]}
+        "icon_folders": sorted names of the icons/<name>/ folders with any change
+            (added, modified or deleted files)
+        "other_files": changed paths outside icons/<name>/ and devicon.json
+        "json": names of devicon.json entries that were added, changed or removed
         "existing": names of devicon.json entries that already existed at `base`
     """
-    diff = subprocess.run(
-        ["git", "diff", "--name-only", "--diff-filter=d", base, "HEAD", "--", "icons"],
-        capture_output=True, text=True, check=True).stdout.splitlines()
-    svgs = {}
-    for path in diff:
-        parts = path.split("/")  # icons/<name>/<file>.svg
-        if len(parts) == 3 and parts[2].endswith(".svg"):
-            svgs.setdefault(parts[1], []).append(parts[2])
+    # -z: NUL-separated "status, path" pairs, so unusual file names parse safely.
+    # --no-renames: a rename shows up as a delete + add, so both paths count.
+    out = subprocess.run(
+        ["git", "diff", "--name-status", "-z", "--no-renames", base, "HEAD"],
+        capture_output=True, text=True, check=True).stdout.split("\0")
+    svgs, icon_folders, other_files = {}, set(), []
+    for status, path in zip(out[0::2], out[1::2]):
+        if path == "devicon.json":
+            continue
+        parts = path.split("/")  # icons/<name>/<file>
+        if len(parts) == 3 and parts[0] == "icons":
+            icon_folders.add(parts[1])
+            if status != "D" and parts[2].endswith(".svg"):
+                svgs.setdefault(parts[1], []).append(parts[2])
+        else:
+            other_files.append(path)
 
     try:
         base_json = json.loads(subprocess.run(["git", "show", f"{base}:devicon.json"],
             capture_output=True, text=True, check=True).stdout)
     except subprocess.CalledProcessError:
         base_json = []
     base_icons = {icon["name"]: icon for icon in base_json}
+    current_names = {icon["name"] for icon in devicon_json}
     changed_json = [icon["name"] for icon in devicon_json if base_icons.get(icon["name"]) != icon]
+    changed_json += [name for name in base_icons if name not in current_names]
 
-    return {"svgs": svgs, "json": changed_json, "existing": set(base_icons)}
+    return {"svgs": svgs, "icon_folders": sorted(icon_folders), "other_files": other_files,
+            "json": changed_json, "existing": set(base_icons)}
 
 
 def get_svg_versions(icon: dict, svg_files: List[str]) -> Tuple[List[str], List[str]]:
@@ -136,7 +150,8 @@ def check_pr_title(pr_title: str, icon: dict, is_new: bool, versions: List[str])
     :param icon, the devicon.json object of the icon whose SVGs the PR changes.
     :param is_new, whether the icon is new (not in the target branch's devicon.json).
     :param versions, the SVG versions the PR adds/changes.
-    :return a string with the problems and the expected title, or "" if it's fine.
+    :return (problems, expected title): problems is a string with the issues and
+    the expected title, or "" if the title is fine.
     """
     kind = "new" if is_new else "update"
     expected = f"{kind} icon: {icon['name']} ({', '.join(versions)})"
@@ -158,8 +173,8 @@ def check_pr_title(pr_title: str, icon: dict, is_new: bool, versions: List[str])
             problems.append(f"the versions should be the SVGs this PR {verb}: {', '.join(versions)}")
 
     if problems:
-        return "PR title issues:\n" + "\n".join(f"- {p}" for p in problems) + f"\n\nExpected title:\n{expected}"
-    return ""
+        return "PR title issues:\n" + "\n".join(f"- {p}" for p in problems) + f"\n\nExpected title:\n{expected}", expected
+    return "", expected
 
 
 def is_svg_in_font_attribute(svg_file_path: Path, devicon_object: dict): 

.github/scripts/check_icon_pr.py

@@ -11,44 +11,66 @@
 def main():
     """
     Check the icons touched by this PR, found from the changed files: the
-    `devicon.json` entries and the SVGs. If the PR changes SVGs, also check that
-    it covers a single icon and that the PR title describes it.
+    `devicon.json` entries and the SVGs.
+
+    An icon PR (one that changes anything in icons/) must only change one
+    icons/<name>/ folder and that icon's `devicon.json` entry, and its title must
+    describe it. If the title is the only thing wrong about the PR's scope, the
+    expected title is written to './expected_title.txt' so the bot can rename the
+    PR, instead of reporting it as an error.
+
     Error messages are written to './err_messages.txt' (empty if all is well).
     """
     args = arg_getters.get_check_icon_pr_args()
     try:
         all_icons = filehandler.get_json_file_content(args.devicon_json_path)
         icons_by_name = {icon["name"]: icon for icon in all_icons}
         changes = util.get_pr_icon_changes(all_icons, args.base)
+        print("Changed icon folders:", changes["icon_folders"])
         print("Changed SVGs:", changes["svgs"])
         print("Changed devicon.json entries:", changes["json"])
+        print("Other changed files:", changes["other_files"])
 
         err_msg = []
         #First check if devicon.json is sorted
         if sorted(all_icons, key=lambda d: d['name']) != all_icons:
             err_msg.append(f"devicon.json is not sorted correctly.\nPlease make sure that your icon is added in the `devicon.json` file at the correct alphabetic position\nas seen here: https://github.com/devicons/devicon/wiki/Updating-%60devicon.json%60")
 
-        # An icon PR changes the SVGs of exactly one icon, and its title says which
-        svg_icons = sorted(changes["svgs"])
-        if len(svg_icons) > 1:
-            err_msg.append(f"This PR changes the SVGs of several icons ({', '.join(svg_icons)}).\nPlease open a separate PR for each icon.")
-        for name in svg_icons:
-            if name not in icons_by_name:
+        # An icon PR only touches one icons/<name>/ folder and that icon's devicon.json entry
+        icon_folders = changes["icon_folders"]
+        if icon_folders:
+            if changes["other_files"]:
+                err_msg.append("Icon PRs should only change files in `icons/<icon name>/` and `devicon.json`, but this PR also changes:\n" +
+                    "\n".join(f"- {path}" for path in changes["other_files"]) +
+                    "\nPlease remove these changes from this PR (open a separate PR for them if they're needed).")
+            if len(icon_folders) > 1:
+                err_msg.append(f"This PR changes several icons ({', '.join(icon_folders)}).\nPlease open a separate PR for each icon.")
+
+        expected_title = ""
+        if len(icon_folders) == 1:
+            name = icon_folders[0]
+            other_entries = [n for n in changes["json"] if n != name]
+            if other_entries:
+                err_msg.append(f"Icon PRs should only change the `devicon.json` entry of their own icon (\"{name}\"), but this PR also changes: {', '.join(other_entries)}.")
+
+            if name in changes["svgs"] and name not in icons_by_name:
                 err_msg.append(f"`icons/{name}/` was changed, but `devicon.json` has no entry named \"{name}\".")
-        if len(svg_icons) == 1 and svg_icons[0] in icons_by_name:
-            name = svg_icons[0]
-            icon = icons_by_name[name]
-            versions, invalid_files = util.get_svg_versions(icon, changes["svgs"][name])
-            if invalid_files:
-                err_msg.append("These SVG file names don't match our pattern of `<name>-(original|plain|line)(-wordmark)?.svg`:\n" +
-                    "\n".join(f"- icons/{name}/{file}" for file in invalid_files))
-            if versions:
-                title_err = util.check_pr_title(args.pr_title, icon, name not in changes["existing"], versions)
-                if title_err:
-                    err_msg.append(title_err)
+            elif name in changes["svgs"]:
+                icon = icons_by_name[name]
+                versions, invalid_files = util.get_svg_versions(icon, changes["svgs"][name])
+                if invalid_files:
+                    err_msg.append("These SVG file names don't match our pattern of `<name>-(original|plain|line)(-wordmark)?.svg`:\n" +
+                        "\n".join(f"- icons/{name}/{file}" for file in invalid_files))
+                if versions:
+                    title_err, title = util.check_pr_title(args.pr_title, icon, name not in changes["existing"], versions)
+                    # Only rename automatically when the PR's scope is clean
+                    if title_err and not (changes["other_files"] or other_entries or invalid_files):
+                        expected_title = title
+                    elif title_err:
+                        err_msg.append(title_err)
 
         # Check every icon this PR touches
-        to_check = sorted(set(svg_icons) | set(changes["json"]))
+        to_check = sorted(set(icon_folders) | set(changes["json"]))
         for name in to_check:
             if name not in icons_by_name:
                 continue
@@ -73,6 +95,9 @@ def main():
                     err_msg.append(svg_err)
 
         filehandler.write_to_file("./err_messages.txt", "\n\n".join(err_msg))
+        filehandler.write_to_file("./expected_title.txt", expected_title)
+        if expected_title:
+            print(f"PR title should be: {expected_title}")
         print("Task completed.")
     except Exception as e:
         filehandler.write_to_file("./err_messages.txt", str(e))

.github/workflows/check_icon_pr.yml

@@ -50,22 +50,32 @@ jobs:
           PR_TITLE: ${{ github.event.pull_request.title }}
         run: python ./.github/scripts/check_icon_pr.py "$PR_TITLE" ./icons ./devicon.json
 
-      # Uploaded even if the script crashed: it writes the error to this file
+      # Uploaded even if the script crashed: it writes the error to err_messages.txt.
+      # expected_title.txt is set when the bot should rename the PR.
       - name: Upload the err messages
         uses: actions/upload-artifact@v4
         if: always()
         with:
           name: err_messages
-          path: ./err_messages.txt
+          path: |
+            ./err_messages.txt
+            ./expected_title.txt
           if-no-files-found: ignore
 
-      # Fail the check when issues were found, so the PR shows a red check.
-      # post_check_icon_pr_comment.yml posts the details as a comment.
+      # Fail the check when issues were found or the title is wrong, so the PR
+      # shows a red check. post_check_icon_pr_comment.yml posts the details and
+      # renames the PR; the rename re-runs this check.
       - name: Fail if issues were found
         shell: bash
         run: |
+          FAIL=0
+          if [ -s expected_title.txt ]; then
+            echo "::error::PR title should be: $(cat expected_title.txt) (the check-bot will rename the PR)"
+            FAIL=1
+          fi
           if [ -s err_messages.txt ]; then
             echo "::error::Issues found in this PR (details in the check-bot comment):"
             cat err_messages.txt
-            exit 1
+            FAIL=1
           fi
+          exit $FAIL