Replace str by Path + better arguments consistency checks. (#59)

* Replace str by Path + better arguments consistency checks.

* Fix misleading error message + minor simplification.

potodo/__init__.py

@@ -2,4 +2,4 @@
 
 __author__ = """Jules Lasne"""
 __email__ = "jules.lasne@gmail.com"
-__version__ = "0.6.0"
+__version__ = "0.6.1"

potodo/_github.py

@@ -1,18 +1,19 @@
 import re
 import subprocess
+from pathlib import Path
 from typing import Mapping, List, Dict, Any
 
 import requests
 
 
-def get_repo_url(repo_path: str) -> str:
+def get_repo_url(repo_path: Path) -> str:
     """Tries to get the repository url from git commands
     """
     try:
         url = subprocess.check_output(
             "git remote get-url --all upstream".split(),
             universal_newlines=True,
-            cwd=repo_path,
+            cwd=str(repo_path),
             stderr=subprocess.STDOUT,
         )
     except subprocess.CalledProcessError:
@@ -20,7 +21,7 @@ def get_repo_url(repo_path: str) -> str:
             url = subprocess.check_output(
                 "git remote get-url --all origin".split(),
                 universal_newlines=True,
-                cwd=repo_path,
+                cwd=str(repo_path),
                 stderr=subprocess.STDOUT,
             )
         except subprocess.CalledProcessError as e:
@@ -30,7 +31,7 @@ def get_repo_url(repo_path: str) -> str:
     return url
 
 
-def get_repo_name(repo_path: str) -> str:
+def get_repo_name(repo_path: Path) -> str:
     """Will get the repository url from git commands then remove useless
     stuff to get ORG/NAME.
     """
@@ -45,7 +46,7 @@ def get_repo_name(repo_path: str) -> str:
     return repo_name
 
 
-def get_reservation_list(repo_path: str) -> Mapping[str, str]:
+def get_reservation_list(repo_path: Path) -> Mapping[str, str]:
     """Will get the repository name then request all the issues and put them in a dict
     """
 

potodo/_po_file.py

@@ -57,14 +57,11 @@ class PoFileStats:
 def is_within(file: Path, excluded: Path) -> bool:
     """Check if `file` is `excluded` or within `excluded`'s tree.
     """
-    excluded = excluded.resolve()
-    file = file.resolve()
-
     return excluded in file.parents or file == excluded
 
 
-def get_po_files_from_repo(
-    repo_path: str, exclude: Iterable[str]
+def get_po_stats_from_repo(
+    repo_path: Path, exclude: Iterable[Path]
 ) -> Mapping[str, Sequence[PoFileStats]]:
     """Gets all the po files recursively from 'repo_path', excluding those in
     'exclude'. Return a dict with all directories and PoFile instances of
@@ -76,8 +73,8 @@ def get_po_files_from_repo(
     # any (sub)folder from the exclusion list
     all_po_files: Sequence[Path] = [
         file
-        for file in Path(repo_path).rglob("*.po")
-        if not any(is_within(file, Path(excluded)) for excluded in exclude)
+        for file in repo_path.rglob("*.po")
+        if not any(is_within(file, excluded) for excluded in exclude)
     ]
 
     # Group files by directory
@@ -90,8 +87,10 @@ def get_po_files_from_repo(
         )
     }
 
-    end_dict: Dict[str, Sequence[PoFileStats]] = {}
-    for directory, po_files in sorted(po_files_per_directory.items()):
-        # For each file in each directory, gets a PoFile instance then add it to a dict
-        end_dict[directory] = [PoFileStats(po_file) for po_file in po_files]
-    return end_dict
+    # Turn paths into stat objects
+    po_stats_per_directory: Dict[str, Sequence[PoFileStats]] = {
+        directory: [PoFileStats(po_file) for po_file in po_files]
+        for directory, po_files in po_files_per_directory.items()
+    }
+
+    return po_stats_per_directory

potodo/potodo.py

@@ -5,32 +5,22 @@ import os
 import json
 import statistics
 
-from typing import Any, Dict, List, Mapping, Sequence, Tuple
+from pathlib import Path
+from typing import Any, Dict, List, Mapping, Sequence
 
 from potodo import __version__
 from potodo._github import get_reservation_list
-from potodo._po_file import PoFileStats, get_po_files_from_repo
+from potodo._po_file import PoFileStats, get_po_stats_from_repo
+
 
 # TODO: Sort the functions (maybe in different files ?
 
 
-def initialize_arguments(
-    above: int, below: int, offline: bool, hide_reserved: bool, repo_path: str
-) -> Tuple[int, int, Mapping[str, str]]:
-    """Will initialize the arguments as necessary
+def get_issue_reservations(
+    offline: bool, hide_reserved: bool, repo_path: Path
+) -> Mapping[str, str]:
+    """Retrieve info about reservation if needed.
     """
-    if not above:
-        # If above isn't specified, then print all files above 0% (all of them)
-        above = 0
-    if not below:
-        # If below isn't specified, then print all files below 100% (all of them)
-        below = 100
-
-    if above and below:
-        if below < above:
-            # If above and below are specified and that below is superior to above,
-            # raise an error
-            raise ValueError("Below must be inferior to above")
 
     if not offline and not hide_reserved:
         # If the reservations are to be displayed, then get them
@@ -38,7 +28,25 @@ def initialize_arguments(
     else:
         # Otherwise, an empty list will do the trick
         issue_reservations = {}
-    return above, below, issue_reservations
+    return issue_reservations
+
+
+def check_args(
+    path: str, exclude: List[str], below: int, above: int, **kwargs: Any
+) -> Mapping[str, Any]:
+    # If below is lower than above, raise an error
+    if below < above:
+        raise ValueError("'below' must be greater than 'above'.")
+
+    # If no path is specified, use current directory
+    if not path:
+        path = os.getcwd()
+
+    # Convert strings to `Path` objects and make them absolute
+    return {
+        "path": Path(path).resolve(),
+        "exclude": [Path(path).resolve() for path in exclude],
+    }
 
 
 def print_dir_stats(
@@ -79,8 +87,8 @@ def add_dir_stats(
 
 
 def exec_potodo(
-    path: str,
-    exclude: List[str],
+    path: Path,
+    exclude: List[Path],
     above: int,
     below: int,
     fuzzy: bool,
@@ -104,12 +112,11 @@ def exec_potodo(
     """
 
     # Initialize the arguments
-    above, below, issue_reservations = initialize_arguments(
-        above, below, offline, hide_reserved, path
-    )
+    issue_reservations = get_issue_reservations(offline, hide_reserved, path)
 
-    # Get a dict with the directory name and all po files.
-    po_files_and_dirs = get_po_files_from_repo(path, exclude)
+    # Dict whose keys are directory names and values are
+    # stats for each file within the directory.
+    po_files_and_dirs = get_po_stats_from_repo(path, exclude)
 
     dir_stats: List[Any] = []
     for directory_name, po_files in sorted(po_files_and_dirs.items()):
@@ -257,6 +264,7 @@ def main() -> None:
     parser.add_argument(
         "-a",
         "--above",
+        default=0,
         metavar="X",
         type=int,
         help="list all TODOs above given X%% completion",
@@ -265,6 +273,7 @@ def main() -> None:
     parser.add_argument(
         "-b",
         "--below",
+        default=100,
         metavar="X",
         type=int,
         help="list all TODOs below given X%% completion",
@@ -309,10 +318,9 @@ def main() -> None:
         "--version", action="version", version="%(prog)s " + __version__
     )
 
-    args = parser.parse_args()
+    # Initialize args and check consistency
+    args = vars(parser.parse_args())
+    args.update(check_args(**args))
 
-    # If no path is specified, use current directory
-    if not args.path:
-        args.path = os.getcwd()
-
-    exec_potodo(**vars(args))
+    # Launch the processing itself
+    exec_potodo(**args)

tests/test_potodo.py

@@ -7,8 +7,8 @@ REPO_DIR = "repository"
 ABS_REPO_DIR = Path(__file__).resolve().parent / "fixtures" / REPO_DIR
 
 config = {
-    "path": f"{ABS_REPO_DIR}",
-    "exclude": [f"{ABS_REPO_DIR}/excluded", f"{ABS_REPO_DIR}/folder/excluded.po"],
+    "path": ABS_REPO_DIR,
+    "exclude": [ABS_REPO_DIR / "excluded", ABS_REPO_DIR / "folder" / "excluded.po"],
     "above": 0,
     "below": 100,
     "fuzzy": False,