bug fixes

Author: rongxin-liu

README.md

@@ -40,6 +40,8 @@ options:
                       default CS50 config)
 ```
 
+`STYLE50_IGNORE` is also supported as a comma-separated environment variable fallback for ignore patterns.
+
 `character`, `split`, and `unified` modes output character-based, side-by-side, and unified (respectively) diffs between the inputted file and the correctly styled version. `score` outputs the raw percentage of correct (unchanged) lines, `json` outputs a JSON object containing structured results, `html` outputs browser-readable results, and `format` outputs only the formatted code.
 
 ### Common examples

style50/__main__.py

@@ -31,7 +31,7 @@ def main():
     # Define command-line arguments.
     parser = argparse.ArgumentParser(prog="style50")
     parser.add_argument("file", metavar="FILE", nargs="+", help="file or directory to lint")
-    parser.add_argument("-o", "--output", action="store", default="character",
+    parser.add_argument("-o", "--output", action="store", default=None,
                         choices=["character", "split", "unified", "score", "json", "html", "format"], metavar="MODE",
                         help="output mode, which can be character (default), split, unified, score, json, html, or format")
     parser.add_argument("-y", "--side-by-side", action="store_true",
@@ -56,22 +56,27 @@ def main():
     if args.in_place and args.side_by_side:
         sys.exit("--in-place cannot be combined with --side-by-side")
 
+    if args.in_place and args.output is not None:
+        sys.exit("--in-place cannot be combined with --output")
+
+    output = args.output or "character"
+
     if args.side_by_side:
-        args.output = "split"
+        output = "split"
 
     if args.in_place:
-        if args.output != "character":
-            sys.exit("--in-place cannot be combined with --output")
-        Style50("format", clang_format_style=args.clang_format_style).format_files_in_place(args.file, ignore=ignore)
+        _, failures = Style50("format", clang_format_style=args.clang_format_style).format_files_in_place(args.file, ignore=ignore)
+        if failures:
+            sys.exit(1)
         return
 
-    if args.output == "format":
+    if output == "format":
         if len(args.file) != 1:
             sys.exit("format mode requires exactly one file")
         sys.stdout.write(Style50("format", clang_format_style=args.clang_format_style).format_file(args.file[0]))
         return
 
-    Style50(args.output, clang_format_style=args.clang_format_style).run(args.file, ignore=ignore)
+    Style50(output, clang_format_style=args.clang_format_style).run(args.file, ignore=ignore)
 
 
 

style50/_api.py

@@ -38,7 +38,7 @@ def __init__(self, output="character", clang_format_style=None):
         self._warn_chars = set()
         self.clang_format_style = clang_format_style
 
-        # Set run function as apropriate for output mode.
+        # Set run function as appropriate for output mode.
         if output in ["score", "format"]:
             self.diff = lambda old, new: ""
         elif output in ["json", "html"]:
@@ -78,16 +78,21 @@ def _files_for_paths(self, paths, ignore=None):
 
     def format_files_in_place(self, paths, ignore=None):
         """Format files and rewrite each path in place."""
+        formatted_count = 0
+        failed_count = 0
         for path in self._files_for_paths(paths, ignore=ignore):
             try:
                 results = self._check(path)
             except Error as e:
                 termcolor.cprint(f"{path}: {e.msg}", "red", file=sys.stderr)
+                failed_count += 1
                 continue
 
             if results.original != results.styled:
-                with open(path, "w") as file:
+                with open(path, "w", encoding="utf-8", newline="") as file:
                     file.write(results.styled)
+                formatted_count += 1
+        return formatted_count, failed_count
 
     def run(self, *args, **kwargs):
         """Wraps Style50.check and renders the results using the renderer determined by self.output"""
@@ -114,9 +119,9 @@ def run(self, *args, **kwargs):
             print(render(**results))
 
 
-    def check(self, paths, ignore=[]):
+    def check(self, paths, ignore=None):
         """
-        Run checks on paths recursively, ignoring pataterns in ignore, returning a dict of results
+        Run checks on paths recursively, ignoring patterns in ignore, returning a dict of results
         """
         files = self._files_for_paths(paths, ignore=ignore)
 
@@ -158,7 +163,7 @@ def check(self, paths, ignore=[]):
 
     def _check(self, file):
         """
-        Run apropriate check based on `file`'s extension and return it,
+        Run appropriate check based on `file`'s extension and return it,
         otherwise raise an Error
         """
 
@@ -178,7 +183,7 @@ def _check(self, file):
                 raise Error("unknown file type \"{}\", skipping...".format(file))
 
         try:
-            with open(file) as f:
+            with open(file, encoding="utf-8", newline="") as f:
                 code = "\n".join(line.rstrip() for line in f)
         except UnicodeDecodeError:
             raise Error("file does not seem to contain text, skipping...")
@@ -322,8 +327,12 @@ class StyleCheck(metaclass=StyleMeta):
 
     # Contains substrings to be matched against libmagic's output if file extension not recognized
     magic_names = []
+    CONFIG_KEYS = {"clang_format_style"}
 
     def __init__(self, code, **kwargs):
+        unknown = set(kwargs) - self.CONFIG_KEYS
+        if unknown:
+            raise Error("unknown configuration option(s): {}".format(", ".join(sorted(unknown))))
         self._config = kwargs
         self.original = code
 
@@ -379,7 +388,7 @@ def run(command, input=None, exit=0, shell=False):
 
     def count_comments(self, code):
         """
-        Returns number of coments in `code`. If not implemented by child, will not warn about comments.
+        Returns number of comments in `code`. If not implemented by child, will not warn about comments.
         """
 
     @abstractmethod

tests/test_cli.py

@@ -1,12 +1,14 @@
+import os
 import subprocess
 import sys
 
+import pytest
+
 
 def run_style50(args, *, cwd=None, env=None):
     merged_env = None
     if env is not None:
         merged_env = dict(**env)
-        import os
         baseline = os.environ.copy()
         baseline.update(merged_env)
         merged_env = baseline
@@ -131,6 +133,15 @@ def test_in_place_and_output_conflict(tmp_path):
     assert "--in-place cannot be combined with --output" in (proc.stdout + proc.stderr)
 
 
+def test_in_place_and_output_character_conflict(tmp_path):
+    f = tmp_path / "t.py"
+    f.write_text("x=1\nprint(  x)\n", encoding="utf-8")
+
+    proc = run_style50(["-i", "-o", "character", str(f)])
+    assert proc.returncode != 0
+    assert "--in-place cannot be combined with --output" in (proc.stdout + proc.stderr)
+
+
 def test_in_place_directory_input_and_ignore(tmp_path):
     src = tmp_path / "src"
     src.mkdir()
@@ -158,3 +169,38 @@ def test_in_place_respects_style50_ignore_env(tmp_path):
     assert "print(x)" in keep.read_text(encoding="utf-8")
     assert "print(  y)" in skip.read_text(encoding="utf-8")
 
+
+def test_in_place_returns_nonzero_when_any_file_fails(tmp_path):
+    existing = tmp_path / "good.py"
+    missing = tmp_path / "missing.py"
+    existing.write_text("x=1\nprint(  x)\n", encoding="utf-8")
+
+    proc = run_style50(["-i", str(existing), str(missing)])
+    assert proc.returncode != 0
+    # Existing file should still be processed.
+    assert "print(x)" in existing.read_text(encoding="utf-8")
+
+
+def test_format_mode_html_css_sql(tmp_path):
+    html = tmp_path / "t.html"
+    css = tmp_path / "t.css"
+    sql = tmp_path / "t.sql"
+    html.write_text("<html><body><h1>x</h1></body></html>\n", encoding="utf-8")
+    css.write_text("body{color:red;}\n", encoding="utf-8")
+    sql.write_text("select * from users where id=1;\n", encoding="utf-8")
+
+    proc_html = run_style50(["-o", "format", str(html)])
+    # Skip only if djhtml is unavailable in this environment.
+    if proc_html.returncode != 0 and "requires djhtml" in (proc_html.stdout + proc_html.stderr):
+        pytest.skip("djhtml not installed in test environment")
+    assert proc_html.returncode == 0
+    assert proc_html.stdout.strip() != ""
+
+    proc_css = run_style50(["-o", "format", str(css)])
+    assert proc_css.returncode == 0
+    assert proc_css.stdout.strip() != ""
+
+    proc_sql = run_style50(["-o", "format", str(sql)])
+    assert proc_sql.returncode == 0
+    assert "SELECT" in proc_sql.stdout
+