12
\$\begingroup\$

Here's the first functional version of my Python 2 script for processing comments in C++ source files. It's a personal project, I expect to expand it later with more advanced options (mainly about replacing comments with whitespace or marking their original positions in the comment-only output).

It's also intended as a learning excercise. I am self-learned in Python, my primary language is C++. So the core of my question is whether the code is "Pythonic" and if not, how to improve on that. I don't want to "write C++ with a different syntax," I want to (learn to) write proper Python.

I will of course also welcome any other comments (general style, efficiency, safety).

#! /usr/bin/env python 

# Copyright Petr Kmoch 2014

"""Script for processing comments and non-comment code in C++ files.

The goal of this script to extract comments from C++ files and output only the comments, only the
non-comment code, or both.

For a quick usage summary, pass '-h' or '--help' as a command-line argument.
"""

import argparse
import os
import re
import sys


class Progress(object):
  """This class stores intermediary data when processing a single line of input.

  It is used as a means of communicating data between the processFile() function and State_*
  objects.

  It contains the following members:
    line: The tail part of the input line which has not been processed yet.
    finished: Boolean flag indicating that the entire line has been processed.
    stateChange:
      If not None, this member holds a callable which takes the state stack as argument
      and will modify it according to the results of processing the line.
    noncomments: String of non-comments extracted from the line during processing.
    comments: String of comments extracted from the line during processing.
  """

  def __init__(self, line):
    object.__init__(self)
    self.line = line
    self.resetProcessing()

  def resetProcessing(self):
    """Clear the results of previously processing a piece of the line.
    """
    self.finished = False
    self.stateChange = None
    self.noncomments = ''
    self.comments = ''

  def extractNoncomment(self, length = None):
    """Move a part of the current line to the `noncomments` member.

    Removes the first `length` characters from `line` and appends them to `noncomments`. If `length`
    is not specified, the entire line is moved.
    """
    self.noncomments += self.extract(length)

  def extractComment(self, length = None):
    """Move a part of the current line to the `comments` member.

    Removes the first `length` characters from `line` and appends them to `noncomments`. If `length`
    is not specified, the entire line is moved.
    """
    self.comments += self.extract(length)

  def extract(self, length):
    """Extract a part of the current line and return it.

    Removes the first `length` characters from `line` and returns them. If `length` is not
    specified, the entire line is used.
    """
    if length is None:
      val = self.line
      self.line = ''
      return val
    else:
      val = self.line[0 : length]
      self.line = self.line[length:]
      return val


class State_Normal(object):
  """State class representing normal C++ code.

  This state represents processing normal C++ code. It is also the initial state in which a file
  starts.
  """

  regex = re.compile('["\'/]')

  def process(self, progress):
    """Process `progress.line` as normal C++ code.

    When this funciton encounters a start of a character literal, string literal or comment, it
    switches state accordingly. Otherwise, it just treats the text it processes as non-comments.
    """
    progress.resetProcessing()
    while progress.line:
      match = self.regex.search(progress.line)
      if not match:
        progress.extractNoncomment()
      elif match.group() == '/':
        nextChar = progress.line[match.start() + 1]
        if nextChar == '/' or nextChar == '*':
          progress.extractNoncomment(match.start())
          progress.extractComment(2)
          if nextChar == '/':
            progress.stateChange = lambda stack: stack.append(State_LineComment())
          else:
            progress.stateChange = lambda stack: stack.append(State_BlockComment())
          return
        else:
          progress.extractNoncomment(match.end())
      else:
        progress.extractNoncomment(match.end())
        progress.stateChange = lambda stack: stack.append(State_Literal(match.group()))
        return
    progress.finished = True


class State_Literal(object):
  """State class representing a character or string literal.

  This state represents processing a character or string literal.
  """

  regexes = {
    '"' : re.compile('["\\\\]')
    , "'" : re.compile("['\\\\]")
  }

  def __init__(self, delim):
    """Initialise for a particular literal type (string or character).

    delim:
      A string containing just the opening delimiter: either a single quote (') or double quote (")
    """
    object.__init__(self)
    self.delim = delim
    self.regex = self.regexes[delim]

  def process(self, progress):
    """Process `progress.line` as the contents of a character or string literal.

    This function processes the line until a non-escaped literal-closing character (' or ") is
    encoutered, at which point it pops this state from the stack. Text processed is treated as a
    non-comment.
    """
    progress.resetProcessing()
    while progress.line:
      match = self.regex.search(progress.line)
      if not match:
        progress.extractNoncomment()
      elif match.group() == '\\':
        progress.extractNoncomment(match.end() + 1) # Extract the backslash and the following character
      else:
        progress.extractNoncomment(match.end())
        progress.stateChange = lambda stack: stack.pop()
        return
    progress.finished = True


class State_LineComment(object):
  """State class representing a //-style comment.

  This state represents processing a comment introduced by //
  """

  regex = re.compile('(\\\\*)\n$')

  def process(self, progress):
    """Process `progress.line` as the contents of a //-style comment.

    The entire line is processed as a comment. Unless it ends with a backslash-escaped newline, this
    state is popped from the stack.
    """
    progress.resetProcessing()
    match = self.regex.search(progress.line)
    if len(match.group(1)) % 2 == 0:
      progress.extractComment()
      progress.noncomments += '\n'
      progress.stateChange = lambda stack: stack.pop()
    else:
      progress.extractComment()
    progress.finished = True


class State_BlockComment(object):
  """State class representing a /* */-style comment.

  This state represents processing a comment delimited by /* and */
  """

  regex = re.compile('\\*/')

  def process(self, progress):
    """Process `progress.line` as the contents of a //-style comment.

    This function processes the line until */ is encountered, at which point it pops this state from
    the stack. Text processed is treated as a comment.
    """
    progress.resetProcessing()
    match = self.regex.search(progress.line)
    if not match:
      progress.extractComment()
      progress.finished = True
    else:
      progress.extractComment(match.end())
      progress.comments += '\n'
      progress.stateChange = lambda stack: stack.pop()


def createArgParser(progName):
  """Create and initialise an argparse.ArgumentParser object.

  This function returns a argument parsing object initialised for parsing ccp arguments.

  progName: String to use as the program's name in the command-line usage help.
  """
  parser = argparse.ArgumentParser(
    description = 'C and C++ comment processor'
    , prog = progName
  )
  parser.add_argument(
    '-c', '--comments'
    , nargs = '?'
    , type = argparse.FileType('w')
    , const = sys.stdout
    , help = '\
Output comments, optionally specifying a destination file. \
If -c is used and -o is not, non-comment output will be suppressed. \
When COMMENT_FILE is not specified, standard output is used.\
'
    , metavar = 'COMMENT_FILE'
    , dest = 'commentFile'
  )
  parser.add_argument(
    '-o', '--output'
    , nargs = '?'
    , type = argparse.FileType('w')
    , const = sys.stdout
    , help = '\
Output non-comments, optionally specifying a destination file. \
This is the default action if none of -o and -c is specified. \
When OUTPUT_FILE is not specified, standard output is used.\
'
    , metavar = 'OUTPUT_FILE'
    , dest = 'outputFile'
  )
  parser.add_argument(
    'inputFile'
    , nargs = '?'
    , type = argparse.FileType('r')
    , default = sys.stdin
    , help = '\
Input file to process. If not specified, standard input is used.\
'
    , metavar = 'INPUT_FILE'
  )
  return parser


def parseArguments(args):
  """Parse the command line `args` and return the resulting options.

  args:
    Sequence of command-line arguments, including the program being invoked. Typical use is passing
    `sys.argv`.
  """
  parser = createArgParser(os.path.basename(args[0]))
  return parser.parse_args(args[1:])


def validate(options):
  """Validate options retrieved from commandline.

  This function detects invalid settings in `options` (which are assumed to come from parsing
  command-line arguments), if any. It also does general post-processing of the options beyond what
  can be expressed in `argparse`.
  """
  if not options.outputFile and not options.commentFile:
    options.outputFile = sys.stdout


def process(options):
  """Act on valid `options`.

  This function processes the inputs specified in `options`. Each input file is fed to
  processFile().
  """
  processFile(options.inputFile, options)


def processFile(inputFile, options):
  """Process `inputFile` according to `options`.

  This function parses `inputFile`, spits it into the comment and non-comment parts, and outputs
  one or both of them based on the settings in `options`.
  """
  stateStack = [State_Normal()]
  for line in inputFile:
    progress = Progress(line)
    while not progress.finished:
      stateStack[-1].process(progress)
      if progress.stateChange:
        progress.stateChange(stateStack)
      if options.outputFile and progress.noncomments:
        options.outputFile.write(progress.noncomments)
      if options.commentFile and progress.comments:
        options.commentFile.write(progress.comments)


def main(args):
  options = parseArguments(args)
  validate(options)
  process(options)


if __name__ == '__main__':
  main(sys.argv)

One thing I am aware of is that the code does not handle trigraphs; they are somewhere near the end of the to-do list.

\$\endgroup\$
  • \$\begingroup\$ Did you take '\' and //? into account? :) \$\endgroup\$ – Morwenn Mar 27 '14 at 14:00
  • \$\begingroup\$ @Morwenn How are these special? I don't handle trigraphs, though, and I should have mentioned that in the question. \$\endgroup\$ – Angew is no longer proud of SO Mar 27 '14 at 14:02
  • \$\begingroup\$ These are special for EOL comments since they can "invalidate" a line break. \$\endgroup\$ – Morwenn Mar 27 '14 at 14:39
  • \$\begingroup\$ @Morwenn A // comment continuation caused by a backslash is accounted for. With //?, did you perhaps mean the ??/ trigraph? \$\endgroup\$ – Angew is no longer proud of SO Mar 27 '14 at 14:43
  • 1
    \$\begingroup\$ Yes, totally ??/. My bad. \$\endgroup\$ – Morwenn Mar 27 '14 at 14:47
4
\$\begingroup\$

A few brief comments from an initial read through:

  • A lot of your method names are mixed case, but the Python convention is lowercase with underscores. The Python style guide is PEP 8:

    Function names should be lowercase, with words separated by underscores as necessary to improve readability.

    mixedCase is allowed only in contexts where that's already the prevailing style (e.g. threading.py), to retain backwards compatibility.

    PEP 8 also recommends 4 spaces for indentation, rather than the 2 space which you’ve used, but that’s not worth getting too worked up about.

  • Why do the docstrings for extractNonComment and extractComment both say they append to noncomments, when this doesn’t seem to match what they’re actually doing?

  • Within the extract method: if in an initial bound in a string slice isn’t set, it defaults to 0, so you can replace val = self.line[0 : length] by val=self.line[:length].

    Next, string slices that start or end in None return the whole string. For example:

    >>> my_string = "12345\n"
    >>> my_string[:None]
    "12345\n"
    

    So you don’t need to check for length is None explicitly: just set val = self.line[:length]. Then you could just trim self.line by the length of val. Something like:

    def extract(self, length = None):
        val = self.line[:length]
        self.line = self.line[len(val):]
        return val
    
\$\endgroup\$
  • \$\begingroup\$ Thanks for the review. lowercase_underscores will be painful for me, but I guess I'll have to adapt :-( The noncomments is just silly C&P. Regarding the None indexing: I didn't know that, great tip. But is it known well enough not to induce a "wtf" from someone reading the code? I wouldn't like to sacrifice clarity to tersity with this. \$\endgroup\$ – Angew is no longer proud of SO Mar 27 '14 at 13:56
  • \$\begingroup\$ @Angew when you're using slice notation, you're calling the __getitem__(self, key) operator.. Key can either be an int, or a slice containing key.start, key.stop and key.step. If you use slice notation and leave any of them out they get sent as None. if start is None: start = 0, if stop is None: stop = len(self), if step is None: step = 1 \$\endgroup\$ – flakes Mar 27 '14 at 19:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.