Merge pull request #16 from cs50/develop

Update to v.2.1.2

README.md

@@ -37,7 +37,7 @@ optional arguments:
 
 ### Adding a new language
 
-Adding a new language is very simple. Language checks are encoded as classes which inherit from the `StyleCheck` base class (see `style50/checks.py` for more real-world examples). The following is a template for style checks which allows style50 to check the imaginary FooBar language for style.
+Adding a new language is very simple. Language checks are encoded as classes which inherit from the `StyleCheck` base class (see `style50/languages.py` for more real-world examples). The following is a template for style checks which allows style50 to check the imaginary FooBar language for style.
 
 ```python
 import re

setup.py

@@ -20,5 +20,5 @@
         "console_scripts": ["style50=style50.__main__:main"],
     },
     url="https://github.com/cs50/style50",
-    version="2.1.1"
+    version="2.1.2"
 )

style50/__init__.py

@@ -1,6 +1,6 @@
-__all__ = ["Style50", "checks", "StyleCheck", "Error"]
+__all__ = ["Style50", "languages", "StyleCheck", "Error"]
 
-from .core import Style50, StyleCheck, Error
+from .style50 import Style50, StyleCheck, Error
 
-# Ensure that checks are registered
-from . import checks
+# Ensure that all language checks are registered
+from . import languages

style50/__main__.py

@@ -47,11 +47,11 @@ def main():
     signal.signal(signal.SIGINT, handler)
 
     # Define command-line arguments.
-    parser = argparse.ArgumentParser()
-    parser.add_argument("files", nargs="*", help="files/directories to lint", default=["."])
+    parser = argparse.ArgumentParser(prog="style50")
+    parser.add_argument("files", nargs="+", help="files/directories to lint")
     parser.add_argument("-o", "--output", action="store", default="character",
                         choices=["character", "split", "unified", "score", "json"], metavar="MODE",
-                        help="specify output mode")
+                        help="specify output mode. Valid modes are character (default), split, unified, score, and json")
     parser.add_argument("-v", "--verbose", action="store_true",
                         help="print full tracebacks of errors")
 

style50/checks.py → style50/languages.py

@@ -6,7 +6,7 @@
 import autopep8
 import jsbeautifier
 
-from . import StyleCheck
+from . import StyleCheck, Error
 
 
 class C(StyleCheck):
@@ -27,6 +27,22 @@ class C(StyleCheck):
     # Matches string literals.
     match_literals = re.compile(r'"(?:\\.|[^"\\])*"', re.DOTALL)
 
+    def __init__(self, code):
+
+        version_text = self.run(["astyle", "--version"])
+        try:
+            # Match astyle version via regex.
+            version = re.match("Artistic Style Version (\d.+)", version_text).groups()[0]
+        except IndexError:
+            raise Error("could not determine astyle version")
+
+        if tuple(map(int, version.split("."))) < (3, 0, 1):
+            raise Error("style50 requires astyle version 3.0.1 or greater, "
+                        "but version {} was found".format(version))
+
+        # Call parent init.
+        StyleCheck.__init__(self, code)
+
     def count_comments(self, code):
         # Remove all string literals.
         stripped = self.match_literals.sub("", code)
@@ -74,6 +90,9 @@ class Js(C):
          ((?<![\*\/])\/(?![\/\*]).*?(?<![\\])\/) # JS regexes, trying hard not to be tripped up by comments
          """, re.VERBOSE)
 
+    # C.__init__ checks for astyle but we don't need this for Js
+    __init__ = StyleCheck.__init__
+
     # TODO: Determine which options, if any should be passed here
     def style(self, code):
         return jsbeautifier.beautify(code)

style50/core.py → style50/style50.py

@@ -107,15 +107,15 @@ def run_diff(self):
                 termcolor.cprint(e.msg, "yellow", file=sys.stderr)
                 continue
 
-            # Display results.
+            # Display results
             if results.diffs:
-                print(*self.diff(results.original, results.styled), sep="",  end="")
+                print(*self.diff(results.original, results.styled), sep="\n")
+                if results.comment_ratio < results.COMMENT_MIN:
+                    termcolor.cprint("And consider adding more comments!", "yellow")
             else:
-                termcolor.cprint("no style errors found", "green")
-
-            if results.comment_ratio < results.COMMENT_MIN:
-                termcolor.cprint("Warning: It looks like you don't have very many comments; "
-                                 "this may bring down your final score.", "yellow")
+                termcolor.cprint("Looks good!", "green")
+                if results.comment_ratio < results.COMMENT_MIN:
+                    termcolor.cprint("But consider adding more comments!", "yellow")
 
     def run_json(self):
         """
@@ -132,7 +132,7 @@ def run_json(self):
 
             checks[file] = {
                 "comments": results.comment_ratio >= results.COMMENT_MIN,
-                "diff": "<pre>{}</pre>".format("".join(self.html_diff(results.original, results.styled))),
+                "diff": "<pre>{}</pre>".format("\n".join(self.html_diff(results.original, results.styled))),
             }
 
         json.dump(checks, sys.stdout)
@@ -176,32 +176,25 @@ def _check(self, file):
                 raise Error("file \"{}\" not found".format(file))
             else:
                 raise
-        except (KeyError, IndexError):
+        except KeyError:
             raise Error("unknown file type \"{}\", skipping...".format(file))
-
-        # Ensure file ends in a trailing newline for consistency
-        try:
-            if code[-1] != "\n":
-                code += "\n"
-        except IndexError:
-            raise Error("file is empty")
         else:
             return check(code)
 
-
     @staticmethod
     def split_diff(old, new):
         """
         Returns a generator yielding the side-by-side diff of `old` and `new`).
         """
-        return (line + "\n" for line in icdiff.ConsoleDiff(cols=COLUMNS).make_table(old.splitlines(), new.splitlines()))
+        return map(lambda l: l.rstrip(),
+                   icdiff.ConsoleDiff(cols=COLUMNS).make_table(old.splitlines(), new.splitlines()))
 
     @staticmethod
     def unified(old, new):
         """
         Returns a generator yielding a unified diff between `old` and `new`.
         """
-        for diff in difflib.ndiff(old.splitlines(True), new.splitlines(True)):
+        for diff in difflib.ndiff(old.splitlines(), new.splitlines()):
             if diff[0] == " ":
                 yield diff
             elif diff[0] == "?":
@@ -213,46 +206,63 @@ def html_diff(self, old, new):
         """
         Return HTML formatted character-based diff between old and new (used for CS50 IDE).
         """
-        def fmt_html(content, dtype):
-            content = cgi.escape(content, quote=True)
-            return content if dtype == " " else "<{1}>{0}</{1}>".format(content, "ins" if dtype == "+" else "del")
+        def html_transition(old_type, new_type):
+            tags = []
+            for tag in [("/", old_type), ("", new_type)]:
+                if tag[1] not in ["+", "-"]:
+                    continue
+                tags.append("<{}{}>".format(tag[0], "ins" if tag[1] == "+" else "del"))
+            return "".join(tags)
 
-        return self._char_diff(old, new, fmt_html)
+        return self._char_diff(old, new, html_transition, fmt=cgi.escape)
 
     def char_diff(self, old, new):
         """
         Return color-coded character-based diff between `old` and `new`.
         """
-        def fmt_color(content, dtype):
-            return termcolor.colored(content, None, "on_green" if dtype == "+" else "on_red" if dtype == "-" else None)
+        def color_transition(old_type, new_type):
+            new_color = termcolor.colored("", None, "on_red" if new_type ==
+                                          "-" else "on_green" if new_type == "+" else None)
+            return "{}{}".format(termcolor.RESET, new_color[:-len(termcolor.RESET)])
 
-        return self._char_diff(old, new, fmt_color)
+        return self._char_diff(old, new, color_transition)
 
     @staticmethod
-    def _char_diff(old, new, fmt):
+    def _char_diff(old, new, transition, fmt=lambda c: c):
         """
-        Returns a char-based diff between `old` and `new` where blocks are
-        formatted by `fmt`.
+        Returns a char-based diff between `old` and `new` where each character
+        is formatted by `fmt` and transitions between blocks are determined by `transition`.
         """
-        differ = difflib.ndiff(old, new)
+
+        differ = difflib.ndiff(old.rstrip("\n"), new.rstrip("\n"))
+
         # Type of difference.
         dtype = None
-        # List diffs of same type.
-        buffer = []
+
+        # Buffer for current line
+        line = []
         while True:
             # Get next diff or None if we're at the end
             d = next(differ, (None,))
             if d[0] != dtype:
-                yield fmt("".join(buffer), dtype)
+                line += transition(dtype, d[0])
                 dtype = d[0]
-                buffer = []
 
             if dtype is None:
                 break
 
-            # Show insertions/deletions of whitespace clearly
-            ch = d[2] if dtype == " " else d[2].replace("\n", "\\n\n").replace("\t", "\\t")
-            buffer.append(ch)
+            if d[2] == "\n":
+                if dtype != " ":
+                    # Show added/removed newlines
+                    line += [fmt(r"\n"), transition(dtype, " ")]
+                yield "".join(line)
+                line = [transition(" ", dtype)]
+            else:
+                # Show added/removed tabs
+                line.append(fmt(d[2] if dtype == " " else d[2].replace("\t", r"\t")))
+
+        # Flush buffer before quitting
+        yield "".join(line)
 
 
 class StyleMeta(ABCMeta):